Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failing tests from minitest-around #1

Closed
wants to merge 2 commits into from

Conversation

grosser
Copy link
Contributor

@grosser grosser commented May 12, 2015

@jeremyevans pulling in the tests from minitest-around it looks like the inner around is being executed before the outer around maybe some other before/around ordering issues, but they are not that bad since treating around as it's own special beats should be fine

@grosser
Copy link
Contributor Author

grosser commented May 12, 2015

also added a before :all spec, it should only be run once per superclass, not once per sub-class ?

@jeremyevans
Copy link
Owner

The before(:all) running before each subclass is expected. before(:all) defines a before_all instance method which is evaluated once per subclass with a subclass instance, and can call other instance methods which could be overridden per subclass.

The way minitest is written, all spec subclasses are independent and executed in a random order (by default), there is no hierarchical execution of spec classes, and therefore it is not really feasible to run the before(:all) only once instead of once per subclass, unless you want to turn off randomization of specs (a bad idea) and try to change minitest to run spec classes in a hierarchical fashion. Even if it was possible, it wouldn't work correctly if before(:all) called instance methods that were overridden in subclasses.

For inner versus outer execution order in around/around(:all), that depends on how you call super in your specs. You control when the inner passes control to the outer based on how you call super, following normal super behavior in ruby. Minitest-hooks reflects minitest's philosophy of "it's just ruby", so around/around(:all) just define methods on classes, and in ruby, method lookup starts at the subclass, and you call super to get the superclass's implementation. Did you look at the existing specs (https://github.com/jeremyevans/minitest-hooks/blob/1.1.0/spec/minitest_hooks_spec.rb#L72-L94)?

To make the order clear, before(:all) and after(:all) are called inside around(:all), around is called after before(:all) and before after(:all), and before/after are called inside around.

Please don't include unrelated files in pull requests. I'm OK with shipping a Gemfile, but not a Gemfile.lock, but that should really be a separate pull request.

In short, I think the fact that minitest-around specs fail is just due to different semantics between the two libraries, not due to bugs in minitest-hooks. After reading the above, what are your thoughts on this?

@grosser
Copy link
Contributor Author

grosser commented May 13, 2015

sounds like an ok tradeoff and the after-nesting makes sense :)

On Tue, May 12, 2015 at 9:22 AM, Jeremy Evans [email protected]
wrote:

Closed #1 #1 via
5482db2
5482db2
.


Reply to this email directly or view it on GitHub
#1 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants