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

Make tests for the presence of MiniTest consistent #8933

Merged

Conversation

floehopper
Copy link
Contributor

There is an inconsistency between the conditional logic in
the definition of ActiveSupport::TestCase and the conditional logic in
ActiveSupport::Testing::SetupAndTeardown [1].

In some circumstances, it's possible for MiniTest to be defined, but
for ActiveSupport::TestCase not to have MiniTest::Unit::TestCase or
MiniTest::Assertions in its ancestor chain. e.g. in Ruby 1.8 with the
minitest gem included in the bundle. In this case, the
Test::Unit/MiniTest shim/wrapper is not present and so
Test::Unit::TestCase (and therefore ActiveSupport::TestCase) is
completely independent of MiniTest::Unit::TestCase.

The conditional logic in the definition of ActiveSupport::TestCase does
not take this scenario into account, whereas the logic in
ActiveSupport::Testing::SetupAndTeardown does take it into account.

The changes in this commit are an alternative to the change in [2] which
was reverted in [3].

Similar conditional logic exists in ActiveSupport::Testing::Isolation
[4], ActiveSupport::Testing::Pending [5],
ActiveSupport::Testing::Performance [6], and in their respective tests.
I have not addressed these, because I know less about what's going on
there, but it would be worth bringing them all into line too.

[1]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/setup_and_teardown.rb#L13
[2]
c3e186e
[3]
267fb61
[4]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/isolation.rb#L41
[5]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/pending.rb#L14
[6]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/performance.rb#L17

There is an inconsistency between the conditional logic in
the definition of ActiveSupport::TestCase and the conditional logic in
ActiveSupport::Testing::SetupAndTeardown [1].

In some circumstances, it's possible for MiniTest to be defined, but
for ActiveSupport::TestCase *not* to have MiniTest::Unit::TestCase or
MiniTest::Assertions in its ancestor chain. e.g. in Ruby 1.8 with the
minitest gem included in the bundle. In this case, the
Test::Unit/MiniTest shim/wrapper is not present and so
Test::Unit::TestCase (and therefore ActiveSupport::TestCase) is
completely independent of MiniTest::Unit::TestCase.

The conditional logic in the definition of ActiveSupport::TestCase does
not take this scenario into account, whereas the logic in
ActiveSupport::Testing::SetupAndTeardown does take it into account.

The changes in this commit are an alternative to the change in [2] which
was reverted in [3].

Similar conditional logic exists in ActiveSupport::Testing::Isolation
[4], ActiveSupport::Testing::Pending [5],
ActiveSupport::Testing::Performance [6], and in their respective tests.
I have not addressed these, because I know less about what's going on
there, but it would be worth bringing them all into line too.

[1]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/setup_and_teardown.rb#L13
[2]
rails@c3e186e
[3]
rails@267fb61
[4]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/isolation.rb#L41
[5]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/pending.rb#L14
[6]
https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/testing/performance.rb#L17
@floehopper
Copy link
Contributor Author

Note that this is a fix for 3-2-stable.

@steveklabnik
Copy link
Member

Right, because Ruby 1.8 doesn't count for master.

rafaelfranca added a commit that referenced this pull request Jan 16, 2013
Make tests for the presence of MiniTest consistent
@rafaelfranca rafaelfranca merged commit fd990f2 into rails:3-2-stable Jan 16, 2013
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.

3 participants