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

Mocha is incompatible with minitest v5.19 and later #614

Closed
kyrofa opened this issue Jul 26, 2023 · 16 comments · Fixed by dodona-edu/dolos#1197
Closed

Mocha is incompatible with minitest v5.19 and later #614

kyrofa opened this issue Jul 26, 2023 · 16 comments · Fixed by dodona-edu/dolos#1197

Comments

@kyrofa
Copy link

kyrofa commented Jul 26, 2023

Minitest has removed the ancient MiniTest compatibility layer that Mocha has been relying on unless the user defines the MT_COMPAT environment variable. See the commit for more details.

Seems that Mocha should move to using Minitest instead?

@floehopper
Copy link
Member

@kyrofa:

Thanks for the heads up, although I'm a bit confused about what's broken. I just manually ran the weekly build which tries to pick up on incompatibilities like this, but nothing failed even though the job using the latest Minitest gem seems to have used v5.19.0 which is the version you mention.

If you understand more about exactly what the problem is, it would be great if you can save me some time and tell me what's actually broken and ideally what I need to change!

@floehopper
Copy link
Member

And do you know if there was a Minitest deprecation warning for this? If so, I missed it!

@manewitz
Copy link

here's a backtrace if that's helpful:

/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/mocha-2.0.4/lib/mocha/integration/mini_test/adapter.rb:26:in `included': uninitialized constant MiniTest (NameError)

          Mocha::ExpectationErrorFactory.exception_class = ::MiniTest::Assertion
                                                                     ^^^^^^^^^^^
Did you mean?  Minitest
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/mocha-2.0.4/lib/mocha/integration/mini_test.rb:21:in `include'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/mocha-2.0.4/lib/mocha/integration/mini_test.rb:21:in `activate'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/mocha-2.0.4/lib/mocha/minitest.rb:4:in `<main>'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.9/lib/zeitwerk/kernel.rb:38:in `require'
	from /Users/mikemanewitz/work/coupons/test/test_helper.rb:10:in `<main>'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.9/lib/zeitwerk/kernel.rb:38:in `require'
	from /Users/mikemanewitz/work/coupons/test/controllers/bulk_uploads_controller_test.rb:3:in `<main>'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.9/lib/zeitwerk/kernel.rb:38:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/test_unit/runner.rb:47:in `block in load_tests'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/test_unit/runner.rb:47:in `each'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/test_unit/runner.rb:47:in `load_tests'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/test_unit/runner.rb:40:in `run'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/commands/test/test_command.rb:33:in `perform'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/command.rb:27:in `run'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/invocation.rb:127:in `invoke_command'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/honeybadger-5.2.1/lib/honeybadger/plugins/thor.rb:17:in `invoke_command_with_honeybadger'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor.rb:392:in `dispatch'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/command/base.rb:87:in `perform'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/command.rb:48:in `invoke'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.0.6/lib/rails/commands.rb:18:in `<main>'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
	from /Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.9/lib/zeitwerk/kernel.rb:38:in `require'
	from /Users/mikemanewitz/work/coupons/bin/rails:5:in `<main>'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
	from <internal:/Users/mikemanewitz/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
	from -e:1:in `<main>'

@kyrofa
Copy link
Author

kyrofa commented Jul 26, 2023

And do you know if there was a Minitest deprecation warning for this? If so, I missed it!

You and I both 😛 . Not that I know of, no. I logged an issue asking for confirmation that minitest actually uses semver, because I've been operating under the assumption that it does. Hopefully this was just an oversight, but it's a good reminder that we're using pretty old API, here.

I just manually ran the weekly build which tries to pick up on incompatibilities like this, but nothing failed [...].

I'll see if I can work up a minimal reproducer for you.

@manewitz
Copy link

When I edited the gem locally to use the Minitest casing it fixed my test suite. Might roll back to the last minitest for now.

@kyrofa
Copy link
Author

kyrofa commented Jul 26, 2023

Yeah I just pinned minitest to < 5.19 for now.

@kyrofa
Copy link
Author

kyrofa commented Jul 26, 2023

@floehopper run this:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "minitest", "~> 5.19"
  gem "mocha", "~> 2.0"
end

require "minitest/autorun"
require 'mocha/minitest'

I see this:

$ ruby reproducer.rb 
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using bundler 2.4.12
Using minitest 5.19.0
Using ruby2_keywords 0.0.5
Fetching mocha 2.0.4
Installing mocha 2.0.4
Traceback (most recent call last):
	5: from reproducer.rb:11:in `<main>'
	4: from reproducer.rb:11:in `require'
	3: from /home/kyrofa/.rvm/gems/ruby-2.7.5/gems/mocha-2.0.4/lib/mocha/minitest.rb:4:in `<top (required)>'
	2: from /home/kyrofa/.rvm/gems/ruby-2.7.5/gems/mocha-2.0.4/lib/mocha/integration/mini_test.rb:21:in `activate'
	1: from /home/kyrofa/.rvm/gems/ruby-2.7.5/gems/mocha-2.0.4/lib/mocha/integration/mini_test.rb:21:in `include'
/home/kyrofa/.rvm/gems/ruby-2.7.5/gems/mocha-2.0.4/lib/mocha/integration/mini_test/adapter.rb:26:in `included': uninitialized constant MiniTest (NameError)
Did you mean?  Minitest

Works fine with minitest < 5.19.

@floehopper
Copy link
Member

@manewitz:

Thanks for the backtrace - it certainly gives me a bit more to go on!

When I edited the gem locally to use the Minitest casing it fixed my test suite. Might roll back to the last minitest for now.

Did you rename all occurrences of MiniTest -> Minitest throughout the Mocha codebase to fix it? That seems a bit challenging if we need to do that, because it will mean Mocha is no longer compatible with older versions of Minitest!

@floehopper
Copy link
Member

@kyrofa Many thanks for the code to reproduce the problem 👍

@kyrofa
Copy link
Author

kyrofa commented Jul 26, 2023

That seems a bit challenging if we need to do that, because it will mean Mocha is no longer compatible with older versions of Minitest!

I definitely appreciate that mindset, but it's worth a look at minitest. I'd be willing to bet the Minitest module has been "the way" for a very long time. The versions of minitest that only have MiniTest are probably extremely old.

@manewitz
Copy link

@manewitz:

Thanks for the backtrace - it certainly gives me a bit more to go on!

When I edited the gem locally to use the Minitest casing it fixed my test suite. Might roll back to the last minitest for now.

Did you rename all occurrences of MiniTest -> Minitest throughout the Mocha codebase to fix it? That seems a bit challenging if we need to do that, because it will mean Mocha is no longer compatible with older versions of Minitest!

I just updated the one call in lib/mocha/integration/mini_test/adapter.rb:26 and it worked for me

floehopper added a commit that referenced this issue Jul 26, 2023
TODO:

- [ ] Ideally I'd like to add a failing test to force me to make this change
- [ ] Check that other occurrences of MiniTest aren't breaking things
@floehopper
Copy link
Member

@manewitz

I just updated the one call in lib/mocha/integration/mini_test/adapter.rb:26 and it worked for me

Yes, that seems to do the trick for me too (I've opened #615 to demo that), although I think there might be a bunch of other stuff to check and definitely some code to clean up. If anyone has a project with a lot of tests that using Mocha, it would be great if you could point your Gemfile at the fix-minitest-compatibility branch and see if you run into any other problems, i.e.

# Gemfile

gem "mocha", github: "freerange/mocha", branch: "fix-minitest-compatibility"

@manewitz
Copy link

Looks like a 10 year old casing change: https://github.com/minitest/minitest/blob/master/History.rdoc#505--2013-06-20-

@zenspider
Copy link
Contributor

zenspider commented Jul 27, 2023

  1. Yes, the name change is over a decade old.
  2. All that has changed is not requiring the cruft file.
  3. You can require "minitest/unit" to avoid changing all instances of MiniTest to Minitest (but... c'mon... a decade).
  4. Literally nothing else has changed.

because it will mean Mocha is no longer compatible with older versions of Minitest

no

floehopper added a commit to freerange/whitehall that referenced this issue Jul 27, 2023
floehopper added a commit to alphagov/whitehall that referenced this issue Jul 27, 2023
floehopper added a commit that referenced this issue Jul 27, 2023
It turns out we were relying on the very old [1] MiniTest module name
rather than the newer Minitest module name. While there are other places
in the code that use the MiniTest form, most (all?) of those are
internal to Mocha. Anyway making this one change seems to fix the
problems people are having.

Ideally I'd like to add a failing test to force me to make this change,
but I don't have time right now and I want to fix the problem for people
as soon as possible.

It would also be good to do a general clean up of all the uses of
MiniTest to make sure I haven't missed anything.

Fixes #614.

[1]: https://github.com/minitest/minitest/blob/master/History.rdoc#505--2013-06-20-
floehopper added a commit to alphagov/whitehall that referenced this issue Jul 27, 2023
floehopper added a commit to alphagov/whitehall that referenced this issue Jul 27, 2023
floehopper added a commit to alphagov/whitehall that referenced this issue Jul 27, 2023
The mocha update is required to retain compatibility with minitest. See
freerange/mocha#614 for more details.
@floehopper
Copy link
Member

Fix released in v2.1.0. Thanks for your help, @kyrofa, @manewitz & @zenspider.

@kyrofa
Copy link
Author

kyrofa commented Jul 27, 2023

Thanks @floehopper, really appreciate it!

steventux added a commit to steventux/omniauth_openid_connect that referenced this issue Sep 20, 2023
Mocha > v2.1.0 is needed to run with modern versions of Minitest.
See freerange/mocha#614
floehopper added a commit that referenced this issue Nov 12, 2023
All of these are either references to constants internal to Mocha or
documentation referencing the Minitest library.

As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

I've also renamed `test/mini_test_result.rb` ->
`test/minitest_result_pre_v5.rb` to better explain its purpose. I did
initially think about removing this, but we're still running pre v5
versions of Minitest in the CI builds for Ruby v2.0 & v2.1, so I'm going
to leave that for now and address it separately.

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
All of these are either references to constants internal to Mocha or
documentation referencing the Minitest library.

As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

I've also renamed `test/mini_test_result.rb` ->
`test/minitest_result_pre_v5.rb` to better explain its purpose. I did
initially think about removing this, but we're still running pre v5
versions of Minitest in the CI builds for Ruby v2.0 & v2.1, so I'm going
to leave that for now and address it separately.

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

I've also renamed `test/mini_test_result.rb` ->
`test/minitest_result_pre_v5.rb` to better explain its purpose. I did
initially think about removing this, but we're still running pre v5
versions of Minitest in the CI builds for Ruby v2.0 & v2.1, so I'm going
to leave that for now and address it separately.

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
All of these are either references to constants internal to Mocha or
documentation referencing the Minitest library.

As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

[1]: #614 (comment)
floehopper added a commit that referenced this issue Nov 12, 2023
As noted by @zenspider in this comment [1], the rename is over 10 years
old, so it seems about time we update it!

I've also renamed `test/mini_test_result.rb` ->
`test/minitest_result_pre_v5.rb` to better explain its purpose. I did
initially think about removing this, but we're still running pre v5
versions of Minitest in the CI builds for Ruby v2.0 & v2.1, so I'm going
to leave that for now and address it separately.

[1]: #614 (comment)
geekq added a commit to geekq/workflow that referenced this issue Apr 17, 2024
* upgrade other dependencies
* use current Ruby version for build
coberlin added a commit to coberlin/omniauth_openid_connect that referenced this issue Oct 18, 2024
The access token request needs the code if we're using code flow.
Some providers require additional parameters such as grant_type
and redirect_uri.

Also, pin the minitest version per a conflict with mocha, as noted here:
freerange/mocha#614
coberlin added a commit to coberlin/omniauth_openid_connect that referenced this issue Oct 18, 2024
The access token request needs the code if we're using code flow.
Some providers require additional parameters such as grant_type
and redirect_uri.

To run tests, pin the minitest version per a conflict with mocha, as noted here:
freerange/mocha#614
But do not leave pinned as the gem fails to install under some rubies that
previously succeeded.

For example:
net-imap-0.5.0 requires ruby version >= 3.1.0, which is incompatible with the
current version, ruby 2.6.8p0 (jruby 9.3.7.0)

Also, Set grant type explicitly when response type is code

Setting the grant_type to :authorization_code in extra_token_params
results in token requests with multiple grant types separated by commas,
with authorization_code appended to the end, which results in invalid
grant type error from the provider.
coberlin added a commit to coberlin/omniauth_openid_connect that referenced this issue Oct 23, 2024
The access token request needs the code if we're using code flow.
For authorization code flow, grant_type and redirect_uri are also required.

To run tests, pin the minitest version per a conflict with mocha, as noted here:
freerange/mocha#614
But do not leave pinned as the gem fails to install under some rubies that
previously succeeded.

For example:
net-imap-0.5.0 requires ruby version >= 3.1.0, which is incompatible with the
current version, ruby 2.6.8p0 (jruby 9.3.7.0)
coberlin added a commit to coberlin/omniauth_openid_connect that referenced this issue Oct 23, 2024
The access token request needs the code if we're using code flow.
For authorization code flow, grant_type and redirect_uri are also required.
See https://docs.duendesoftware.com/identityserver/v7/reference/endpoints/token/

To run tests, pin the minitest version per a conflict with mocha, as noted here:
freerange/mocha#614
But do not leave pinned as the gem fails to install under some rubies that
previously succeeded.

For example:
net-imap-0.5.0 requires ruby version >= 3.1.0, which is incompatible with the
current version, ruby 2.6.8p0 (jruby 9.3.7.0)
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 a pull request may close this issue.

4 participants