search

rspec / rspec-rails

RSpec for Rails 7+
https://rspec.info
MIT License
5.19k stars 1.04k forks source link

Even if specify the use of TestUnit, it will be overwritten by Minitest and cannot be used. #2717

Closed akicho8 closed 1 year ago

akicho8 commented 1 year ago

What Ruby, Rails and RSpec versions are you using?

ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22] Rails 7.1.2 RSpec 3.12

Observed behaviour

Even if I set spec_helper.rb so that TestUnit can also be used, TestUnit's assert does not work.

require "rails_helper"

RSpec.describe type: :model do
  it "works" do
    pp self.class.ancestors.collect(&:name).grep(/unit|mini/i)
    assert { 1 + 2 == 3 }
  end
end
# >> ["RSpec::Rails::MinitestAssertionAdapter",
# >>  "RSpec::Rails::MinitestLifecycleAdapter",
# >>  "RSpec::Core::TestUnitAssertionsAdapter",
# >>  "Test::Unit::Assertions"]
# >> F
# >>
# >> Failures:
# >>
# >>   1) {:type=>:model} works
# >>      Failure/Error: Unable to find - to read failed line
# >>
# >>      ArgumentError:
# >>        wrong number of arguments (given 0, expected 1..2)
# >>      # -:7:in `block (2 levels) in <main>'
# >>
# >> Finished in 0.01616 seconds (files took 0.91698 seconds to load)
# >> 1 example, 1 failure
# >>
# >> Failed examples:
# >>
# >> rspec -:5 # {:type=>:model} works

Expected behaviour

The assert method of PowerAssert, which is originally installed with TestUnit, is enabled, so assert can be executed with a block. And the test passes without any problems.

Actual behavior

An error message will be displayed stating that the number of arguments does not match.

# >>      ArgumentError:
# >>        wrong number of arguments (given 0, expected 1..2)

Cause

The cause is that TestUnit is overwritten with Minitest. If check ancestors, will see that MinitestAssertionAdapter is included, which is closer than TestUnitAssertionsAdapter.

Specifically, MinitestAssertionAdapter is included in the following two places regardless of the user's intention.

https://github.com/rspec/rspec-rails/blob/b109337c1b60ce1882208b11f887f908b70edac8/lib/rspec/rails/example/rails_example_group.rb#L17

https://github.com/rspec/rspec-rails/blob/b109337c1b60ce1882208b11f887f908b70edac8/lib/rspec/rails/fixture_support.rb#L9

I think this setting allows the user to decide whether to use TestUnit or Minitest.

https://github.com/rspec/rspec-core/blob/1eeadce5aa7137ead054783c31ff35cbfe9d07cc/lib/rspec/core/configuration.rb#L867-L872

If I want to use Minitest, configure it explicitly there.

Would it be possible to consider discontinuing the mandatory use of Minitest in RailsExampleGroup and FixtureSupport? I kindly request your consideration.

Steps to reproduce

https://github.com/akicho8/my_rspec_rails_app/blob/64712fbe557d1580987ef94e9e089503fcf7bea7/spec/spec_helper.rb#L20

https://github.com/akicho8/my_rspec_rails_app/blob/main/spec/models/hello_spec.rb

The entire code is below. Fully reproducible.

https://github.com/akicho8/my_rspec_rails_app

It is also possible to reproduce with just this one file.

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "rspec"
  gem "rails"
  gem "rspec-rails"
  gem "test-unit"
end

require "rspec/autorun"
require "test/unit"

require "rails"
require "action_view"
require "action_controller"
require "rspec/rails"

RSpec.configure do |config|
  config.expect_with :test_unit
end

RSpec.describe do
  include RSpec::Rails::RailsExampleGroup

  it "works" do
    pp self.class.ancestors.collect(&:name).grep(/unit|mini/i)
    assert { 1 + 2 == 3 }
  end
end
# >> ["RSpec::Rails::MinitestAssertionAdapter",
# >>  "RSpec::Rails::MinitestLifecycleAdapter",
# >>  "RSpec::Core::TestUnitAssertionsAdapter",
# >>  "Test::Unit::Assertions"]
# >> F
# >> 
# >> Failures:
# >> 
# >>   1) works
# >>      Failure/Error: Unable to find - to read failed line
# >> 
# >>      ArgumentError:
# >>        wrong number of arguments (given 0, expected 1..2)
# >>      # -:29:in `block (2 levels) in <main>'
# >> 
# >> Finished in 0.005 seconds (files took 0.48997 seconds to load)
# >> 1 example, 1 failure
# >> 
# >> Failed examples:
# >> 
# >> rspec -:27 # works

Possible related issues

2633

pirj commented 1 year ago

Secondary question:

Would it be possible to consider discontinuing the mandatory use of Minitest in RailsExampleGroup and FixtureSupport?

Not easily at least. Our matchers use Rails-provided assertions internally, e.g. https://github.com/rspec/rspec-rails/blob/b109337c1b60ce1882208b11f887f908b70edac8/lib/rspec/rails/matchers/redirect_to.rb#L28

akicho8 commented 1 year ago

@pirj Thanks for your comment

RSpec was designed to be able to switch between TesUnit and Minitest. So we naturally assumed that rspec-rails would be able to do the same.

However, from what you say, is rspec-rails designed with the basic assumption that minitest exists to support the expect notation?

If so, it would be a shame if you could make it clear that TestUnit should not be mixed with rspec-rails, but I'll give up and use minitest.

JonRowe commented 1 year ago

Rails has a dependency on minitest and uses minitest for its test helpers which we bring into RSpec in rspec-rails to enable behaviour similar to those test helpers, so its not possible for rspec-rails to disable minitest entirely, but you should able to preprend the assertion module yourself if you want to use test unit assertions, as this looks to me like an ordering issue, you've enable the config but then rspec-rails comes on top of that...

akicho8 commented 1 year ago

@JonRowe Thanks for your comment Since TestUnit and Minitest have the same interface, is it safe to switch between the two? Also, there may be an ordering issue: Minitest is included more recently than TestUnit in the ancestors results.

akicho8 commented 1 year ago

Looking in ActiveSupport, there are so many places where it directly says Minitest:: that I have a feeling that replacing only the Minitest assertion with the TestUnit assertion may not work.

For now, I would like to proceed by combining the current spec-rails with the minutest-power_assert gem.

JonRowe commented 1 year ago

👍 I'm going to close this as not planned but if someone does find a way to allow this to be better prioritised feel free to open a PR