assertion_error.rb forces load of minitest for ruby 1.9 #88

Closed
wr0ngway opened this Issue Apr 2, 2012 · 7 comments

Projects

None yet

3 participants

@wr0ngway
wr0ngway commented Apr 2, 2012

Any reason for this? It breaks RubyMine when running tests because RM doesn't activate testunit support when minitest is detected.

https://github.com/thoughtbot/shoulda-matchers/blob/master/lib/shoulda/matchers/assertion_error.rb

@gabebw
Member
gabebw commented Apr 2, 2012

Hi,

I'm not familiar with RubyMine - could you explain a little bit more how this breaks, or (even better) what the desired behavior would be?

-- Gabe

@wr0ngway
wr0ngway commented Apr 2, 2012

RM is detecting the presence of minitest, I assume by seeing if the class constant is defined, and when it does, it disables the TestUnit integration. Since assertion_error is requiring minitest/unit, it causes the constant to be defined. Ideally, shoulda should not require a dependency unless it uses it. In assertion_error, its checking for ruby 1.9 to determine if it should use minitest over testunit, which is a flawed assumption (people can and do use testunit with 1.9). It seems dangerous that one could end up with both minitest and testunit loaded, especially since they play in the same namespace.

Perhaps you could test for presence of testunit as the conditional? E.g.

module Shoulda
  module Matchers
    if defined?(Test::Unit::AssertionFailedError)
      AssertionError = Test::Unit::AssertionFailedError
    elsif Gem.ruby_version >= Gem::Version.new("1.9")
      require 'minitest/unit'
      AssertionError = MiniTest::Assertion
    else
      raise "No unit test library available"
    end
  end
end
@gabebw
Member
gabebw commented Apr 3, 2012

Ah, I see. Thanks for the explanation. That looks like a lovely fix.

-- Gabe

@pupeno
Contributor
pupeno commented Jun 15, 2012

I run into this issue which caused our team to waste a lot of time by not being able to run individual tests (test-unit can run individual test with RubyMine, minitest can't) as well as me making the guys at JetBrains to waste time tracking why that was happening when I reported it (http://youtrack.jetbrains.com/issue/RUBY-10602?replyTo=27-343557). I also re-reported the issue here: #118.

Will that patch be included? Do you need a clean pull request to do it? (I can give it a try)

@pupeno
Contributor
pupeno commented Jun 15, 2012

I created a fork with a branch with @wr0ngway's code... so far everything seems to be working fine for me: https://github.com/watu/shoulda-matchers/tree/do_not_load_minitest

@gabebw
Member
gabebw commented Jun 15, 2012

@pupeno - I'd be glad to merge this in if you create a pull.

@pupeno
Contributor
pupeno commented Jun 15, 2012

@gabebw I'm glad you are, here it is: #119 :)

@gabebw gabebw closed this in c75c833 Jun 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment