-
-
Notifications
You must be signed in to change notification settings - Fork 753
WIP Refactor group profile to include beforehook #1942
Conversation
…formatter_spec, remove test_spec and fix profile_groupe.feature
I noticed the to_h function in ruby < 2.1, it's on my todo. @JonRowe you can see the problem with the profile_groupe.feature on the ruby 2.2 build |
@@ -634,6 +631,7 @@ class NullNotification | |||
end | |||
|
|||
# `CustomNotification` is used when sending custom events to formatters / | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is superfluous and should be removed :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, no problem :)
end | ||
""" | ||
When I run `rspec spec --profile` | ||
Then the output should contain "slow before context hook" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the examples in this scenario are kind of noisy since there are so many of them. IMO, it would work better as documentation so that the reader doesn't have to read so much to get the full context. You can use rspec spec --profile <number>
to limit it to the top <number>
slowest examples.
I also think we can cut the sleep
values further...maybe sleep 0.1
and sleep 0.2
? It's nice to keep the scenario from getting too slow. We can always disable GC in it to make the runtime more deterministic.
next if location_hash.key?(:description) | ||
location_hash[:description] = example.example_group.top_level_description | ||
end | ||
# todo rename this method or move it's code to the json_formater.rb and profile.formater.rb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code shouldn't be moved to the formatters, it should be de-duplicated here.
@denislaliberte -- thanks for taking this PR this far! @JonRowe, do you want to work on taking this across the finish line so it can be in 3.3? |
Yep sure |
@myronmarston no problem, It was fun to give a try on this issue but in the end I was stuck, I can't add example_group_started on the notifications class without a larger refactoring... I will look to other issue, I'll be glad to contribute if I can. |
@JonRowe after your comments of yesterday I have done some cleanup, If you have other comments or hint to fix json_formatter_spec or profile_groupe.feature let me know
The json_formatter_spec change the clock of the example, do you think i should rewrite "ranks the example groups by average time" with it's own 'before' or test this behaviour in the new profile_groupe.feature ?
issue #1878