jumph4x/canonical-rails

Test suite fails with rack 3

Closed this issue · 2 comments

While running the test suite with sprockets 4 (and adding the missing manifest file, see #85), I found that there are two failures with rack 3.

Sprockets < 4.2.0 (September 5 2023) will run without error, since they have a constraint to rack 2, but newer versions of sprockets support rack 3.

The errors are:

1) CanonicalRails::TagHelper w/ custom config with parameters should escape allowed params properly
   Failure/Error: expect(helper.allowed_query_string).to eq '?page=5&keywords=%22here+be+dragons%22&search[super]=special'
   
     expected: "?page=5&keywords=%22here+be+dragons%22&search[super]=special"
          got: "?page=5&keywords=%22here+be+dragons%22&search%5Bsuper%5D=special"
   
     (compared using ==)
   # ./spec/helpers/canonical_rails/tag_helper_spec.rb:207:in `block (4 levels) in <top (required)>'

2) CanonicalRails::TagHelper w/ custom config with parameters should output allowed params using proper syntax (?key=value&key=value)
   Failure/Error: expect(helper.canonical_tag).to eq '<link href="http://www.mywebstore.com/our_resources/?page=5&keywords=%22here+be+dragons%22&search[super]=special" rel="canonical" />'
   
     expected: "<link href=\"http://www.mywebstore.com/our_resources/?page=5&keywords=%22here+be+dragons%22&search[super]=special\" rel=\"canonical\" />"
          got: "<link href=\"http://www.mywebstore.com/our_resources/?page=5&keywords=%22here+be+dragons%22&search%5Bsuper%5D=special\" rel=\"canonical\" />"
   
     (compared using ==)
   # ./spec/helpers/canonical_rails/tag_helper_spec.rb:211:in `block (4 levels) in <top (required)>'

I haven't investigated what the root cause of these failures are, but they can be avoided by adding s.add_dependency "rack", '< 3' to the gemspec.

I avoided the Rack dep spec, trying to keep this as widely usable as possible.

I included a comment right in the test that will accept either result, it links to Aaron Patterson publicly stating Rack does not conform to the RFC and that the web as we know it seems to work just fine.

Closing. @gravitystorm

Seems like a reasonable fix to me 👍