migrate unit tests from test-unit to mini-test #256
Conversation
|
👍 ++ on MiniTest for unit testing! |
did nothing but test whether git gem was working as expected. will need a real unit test suite for our GitInfo class instead.
ruby-lint wants assert-match to be more specific
these should be gone once we do #99 and these will move into each respective gem test suite
unfortunately aruba specifically uses rspec-assertions, which conflict with the way cucumber loads minitest that it did not with testunit. to get around this, i rewrote our steps that use assertions to use rspec expectations.
|
This was relatively straightforward, with the exception of me banging my head against some weird incompatibilities between cucumber/aruba/minitest (see 6bc799a), but I think this is working now (pending CI approval) |
|
BTW, current state of the world: we have a few unit tests that are actually for plugins (now in their own file), a single solitary unit test that is actually testing for a pre-build error condition but not validating our code, and zero current unit tests for actual lolcommits classes. 😲 (our cuke integration tests are nice though!) |
|
Okay, not sure what's going on... The twitter existing plugin tests seem to randomly fail. Just rerunning repeatedly sometimes they work, sometimes they don't. (Sheesh, I will be so happy when plugins can be out of the codebase!) @matthutchinson any idea what might be going on here? |
|
Okay, better visibility into whats going on with change in 926cb3d. Looks like the lolcommits plugin tests are actually order dependent (ugh), even though they certainly shouldn't be. Since minitest randomizes test order, when they execute out of order they have preserved state. |
|
Okay, fixed for now via 918c845 by force defining configuration on each test. The way the tests for the Twitter plugin are stubbing is really bad practice since it affects global state. It should be possible to restructure these tests to not do so if someone wants to take it on, but in my opinion this can hold us over until we get the plugins out of the main codebase entirely. |
| @@ -73,7 +73,7 @@ | |||
| end | |||
|
|
|||
| Then /^I should be (prompted for|presented) "(.*?)"$/ do |_, prompt| | |||
| assert @stdout.read.to_s.include?(prompt) | |||
| expect(@stdout.read.to_s).to include(prompt) | |||
There was a problem hiding this comment.
Shouldn't we stick with the assert syntax in this file? Given we're using that style in the refactored plugin tests in this PR? Personally I prefer the assert MiniTest style over the 'expect to' grammar but this is just mho 😄
There was a problem hiding this comment.
Yeah, I would have preferred that, but there is an 3-way incompatibility with minitest/cucumber/aruba because aruba depends on rspec-expectations (see 6bc799a), and none of the workarounds functioned in our case that I could get to work. Thankfully it's only for the few assertions in the cuke tests. From looking through the issues boards it sounds like this may be addressed by the next major version of cucumber.
There was a problem hiding this comment.
Ah I think I see why.. this is so we can remove the dependency on TestUnit assertions within step definitions? And use the rspec-expectations available through aruba..
It kinda sucks a bit that rspec leaks in here tho. I wonder if Aruba can work with MiniTest like assertions somehow.
There was a problem hiding this comment.
Yeah, exactly. In theory we should be able to mixin MiniTest assertions the same way (although it requires a bit of boilerplate: https://github.com/cucumber/cucumber/wiki/Using-MiniTest) but that doesn't actually function if rspec functions are also loaded due to E_TOOMUCHMAGIC of different frameworks trying to do the right thing automatically.
That said even if it did work it introduces a larger amount of boilerplate than the 3-4 one line assertions.
I think its unlikely to be able to strip rspec-expectations out of Aruba, I think it uses a lot of its advanced functionality to write its ruleset. It may be possible to rewrite them all with minitest 5, which would be a pretty epic pull request for Aruba, but I suspect it would be quite an endeavor.
|
Just caught up on your work here now, added a few comments, let me know what you think 💭 |
|
OK, overall a good start and good to merge I think 👍 |
migrate unit tests from test-unit to mini-test
mini-test has been the stdlib standard since ruby 1.9. since it functions on ruby 1.8.7 as well (and probably as far back as ruby 1.6 which we don't support), we might as well switch to it at some point prior to creating a bunch of unit tests when refactoring classes.