Minitest cleanup #3445
Minitest cleanup #3445
Conversation
| @@ -11,11 +11,6 @@ | |||
| Gem::Specification.dirs.shift | |||
| end | |||
|
|
|||
| begin | |||
| gem 'minitest', '~> 5.0' | |||
There was a problem hiding this comment.
Are we suppose to be using bundle exec rake test from now on? I'm lost why we don't need this anymore
There was a problem hiding this comment.
I can see how you're lost because removing this is just wrong 😆. Nice catch, removing the commit.
There was a problem hiding this comment.
@colby-swandale I ended up restoring this but removing the exception swallowing code. Since after these changes our test suite requires minitest 5.13 or higher because I removed the compatibility code, I think the exception is useful to tell users not satisfying the requirement to install a matching version. Let me know what you think.
There was a problem hiding this comment.
That sounds reasonable. Would it be worth catching the exception anyway and add a friendly error message to say you need to run bundle install?
There was a problem hiding this comment.
@colby-swandale Since our tests are run under different environments, I think I'd rather leave the raw exception in place? For example, if our tests are run from ruby-core, I don't think a bundle install suggestion would apply there. Plus, gem install minitest will fix the issue anyways without a full bundle install.
Our tests are now guaranteed to not work on older versions of minitest, so I don't think we should swallow this activation error because it will make it very clear for the user what the problem is if she has a minitest version not satisfying the requirement.
deivid-rodriguez
force-pushed
the
minitest_cleanup
branch
from
d2f0c3b
to
a291b65
Compare
These changes completely broke the test suite on ruby core repository. rubygems/rubygems@84b0bea3e1
Description:
Some small code cleanups related to minitest.
Tasks:
I will abide by the code of conduct.