Minitest plugin's before_teardown can have unexpected interactions with tests #131
Description
What?
We tried using buildkite-test_collector and noticed it broke some expectations in our test suite, so we had to temporarily turn it off until we can figure out a workaround or a fix.
How?
In our test suite, some tests expect a call to be made to SecureRandom.uuid, like this:
SecureRandom.expects(:uuid).returns("123")(We're using the Mocha mocking/stubbing library and its #expects method.)
The tests used to work but as soon as we started using buildkite-test_collector, the tests kept failing, announcing that the expectation was not fulfilled anymore:
unexpected invocation: SecureRandom.uuid()
unsatisfied expectations:
- expected exactly once, invoked twice: SecureRandom.uuid(any_parameters)
(We also have another one of these unexpected invocations on Process.clock_gettime(1, :float_second).)
Now I know that this kind of expectation is not testing best practice and we have some work to do to educate and figure out better ways, but I think buildkite-test_collector is introducing unexpected behaviour, and it would be nice if we could find a way to fix it.
Taking a quick look at the MiniTest plugin implementation, I noticed the use of before_teardown:
This gets this to run before all our teardowns, and likely before Mocha's teardown as well.
I'm not sure whether it's possible, but would like to ask: would it still work if this was an after_teardown (instead of a before_teardown)?
It would kinda make more sense to me too, as running the test collector before teardown means we'll ignore all things that happen in teardown, even at a test file level.
class SampleTest < Minitest::Test
def setup
puts "I'm doing something in #setup, and this will be collected for test analytics."
end
def teardown
puts "I'm doing something in #teardown, and this will NOT be collected for test analytics."
end
def sample_test
puts "This is a sample test"
assert true
end
endOne workaround that might fix our Mocha expectation problem (but not the current setup/teardown asymmetry) could be to change the order of the Mocha and buildkite-test_collector gems in our Gemfile, but order of gems in the Gemfile is not something I'd necessarily want to rely on...
If using after_teardown instead of before_teardown sounds reasonable, I'd be happy to open a PR (though I'm sure that'd be a very quick fix you'd manage to put out in no time too 🙂 ). Please let me know!