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

Fix tests for newer Rubies/Rails. #219

Closed

Conversation

stevenharman
Copy link
Contributor

@stevenharman stevenharman commented Mar 24, 2025

This add Rails 7.2 and 8.0 to the test matrix. Should we also drop EoL'd Rails/Rubies? For now I've excluded combinations which won't work due to minimum version constraints.

Even then, as of Rail 7.2 the tests for this library are broken. It seems something changed with how ActionDispatch::Assertions::RoutingAssertions work as the reset_routes method, called by with_routes, starts to break because the expected app is nil.

I've stepped through the code and I'm unsure where that was supposed to be set up, or how it worked back in Rails 7.1. If anyone has pointers, I'm happy to go spelunking and try to get the tests passing so we can consider some of the other PRs to fix various bugs/improvements.

@stevenharman stevenharman force-pushed the update_supported_rails_rubies branch 4 times, most recently from bb8554f to 12ff474 Compare March 24, 2025 19:06
@stevenharman stevenharman changed the title Update the Ruby and Rails test matrix Fix tests for newer Rubies/Rails. Mar 24, 2025
@stevenharman
Copy link
Contributor Author

stevenharman commented Mar 26, 2025

@byroot @gmcgibbon I was able to get the tests all passing, on both older Rubies/Rails, and current. I leaned on Jean's work to get build_app and with_routing working without blowing up. And the tests all pass, most of the time. But there seems to be some test pollution happening in some cases.

I've been able to get a failure on Ruby 3.2 and 3.3, using Rails edge, and seed 60717. It looks like the LoggerSilencerTest::TestController is being routed to, despite the test being ActionControllerTest#test_getting_session_value_after_session_reset. You should be able to pull my branch, update the Gemfile to point at the Rails edge (like in gemfiles/rails_edge.gemfile), and reproduce with the following:

bundle exec rake test SEED=60717

[… snip …]
Finished in 0.211093s, 246.3369 runs/s, 776.9088 assertions/s.

1) Error:
ActionControllerTest#test_getting_session_value_after_session_reset:
AbstractController::ActionNotFound: The action 'call_reset_session' could not be found for LoggerSilencerTest::TestController
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/abstract_controller/base.rb:162:in `process'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionview/lib/action_view/rendering.rb:40:in `process'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_controller/metal.rb:252:in `dispatch'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_controller/metal.rb:335:in `dispatch'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/routing/route_set.rb:67:in `dispatch'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/routing/route_set.rb:50:in `serve'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/journey/router.rb:34:in `block in serve'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/journey/router.rb:85:in `block in recognize'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/journey/router.rb:65:in `each'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/journey/router.rb:65:in `recognize'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/journey/router.rb:30:in `serve'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/routing/route_set.rb:908:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-2.2.13/lib/rack/session/abstract/id.rb:266:in `context'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-2.2.13/lib/rack/session/abstract/id.rb:260:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-2.2.13/lib/rack/head.rb:12:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-2.2.13/lib/rack/method_override.rb:24:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/middleware/cookies.rb:706:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/middleware/callbacks.rb:31:in `block in call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/activesupport/lib/active_support/callbacks.rb:100:in `run_callbacks'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/middleware/callbacks.rb:30:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb:31:in `call'
    test/helper.rb:37:in `call'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-test-2.2.0/lib/rack/test.rb:360:in `process_request'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/gems/rack-test-2.2.0/lib/rack/test.rb:153:in `request'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/testing/integration.rb:297:in `process'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/testing/integration.rb:19:in `get'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/testing/integration.rb:388:in `get'
    test/action_controller_test.rb:132:in `block in test_getting_session_value_after_session_reset'
    test/helper.rb:79:in `block in with_test_route_set'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/testing/assertions/routing.rb:77:in `create_routes'
    ~/.rbenv/versions/3.3.7/lib/ruby/gems/3.3.0/bundler/gems/rails-76efa0ff4b1e/actionpack/lib/action_dispatch/testing/assertions/routing.rb:44:in `with_routing'
    test/helper.rb:72:in `with_test_route_set'
    test/action_controller_test.rb:126:in `test_getting_session_value_after_session_reset'

As of rails/rails#49819 the internals of
`ActionDispatch::Assertions::RoutingAssertions` changed and the way we
were building our app instance was no compatible with it. This gets us
back to passing tests for Rails 7.2+. I hope 🤞
@stevenharman stevenharman force-pushed the update_supported_rails_rubies branch from 7818343 to 88d8171 Compare March 26, 2025 02:16
@byroot
Copy link
Member

byroot commented Mar 26, 2025

There is some sort of state leak in the test suite indeed. I'll see if I can figure it out, but in the meantime using your changes combined with mine, I could get a green build with a couple retries, so that is enough for cutting a release (not ideal I know)

@byroot byroot closed this in #220 Mar 26, 2025
@stevenharman stevenharman deleted the update_supported_rails_rubies branch March 26, 2025 12:16
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.

2 participants