fix-finding-extension #2325
fix-finding-extension #2325
Conversation
|
👋 Thanks for contributing this! Seems like we need an integration test to check this as the current spec is obviously not enough to catch this, I believe 5.2 does string interpolation so this might be ok for all versions we support but I'm not sure. |
oh... render_options[:variants] = [match[:variant], match[:variant].to_sym] if match[:variant] |
|
We still soft-support 4.2, so intentionally breaking it would be irresponsible. Your code seems to be quite ok to support both. Can you please check if your fix works for your project? May I ask you to add a note to remove stringified parts once we completely drop 4.2? |
|
updated. |
|
some error occurred in travis, but I can't detect what's wrong from error message. |
|
That's weird, I can't reproduce locally, can you by running |
|
all passes on my machine. |
|
The CI error happens in another build too. #2328 (comment) I am gonna try to look at it, this afternoon. |
|
Ok I don't know who is giving a status 9. I wrote a quick and dirty script bin/rake clobber:app
bin/rake generate:app
bin/rake generate:stuff
mkdir -p tmp/example_app/spec/system/
tee -a tmp/example_app/spec/system/widget_system_spec.rb << END
require "rails_helper"
RSpec.describe "Widget management", :type => :system do
before do
driven_by(:selenium_chrome_headless)
end
it "enables me to create widgets" do
visit "/widgets/new"
fill_in "Name", :with => "My Widget"
click_button "Create Widget"
expect(page).to have_text("Widget was successfully created.")
end
end
END
cd tmp/example_app/
bundle install
bundle exec rspec --backtrace spec/system/widget_system_spec.rb
echo $?
cd ../../
bin/cucumber features/system_specs/system_specs.feature
The highlighted line show a status 0... then we a have a status 9... I don't get it. I am in favor of removing this status expectation. IMHO we should care only if the status is 0 or 1 but not something else. |
|
@mvz Do you have an idea for my last comment? |
|
You can also rebase this off master now |
If aruba is doing its job properly this means rspec exits with status 9. I'm attempting to reproduce this locally to see if I can debug this. |
https://github.com/rails/rails/blob/6-0-3/actionview/lib/action_view/template/resolver.rb#L349-L351 Resolver search files and resolve most matched file with **symbolized** string, so `_default_render_options` must return symbolized values.
|
Rebased on master. |
|
@masarakki Sorry for the trouble with CI. |
| render_options[:handlers] = [match[:handler]] if match[:handler] | ||
|
|
||
| # remove stringified parts when dropping rails-4.x support | ||
| render_options[:handlers] = [match[:handler], match[:handler].to_sym] if match[:handler] |
There was a problem hiding this comment.
We don't want to match both, we just want to match the right type for each rails version.
| @@ -148,19 +148,19 @@ def _default_file_to_render; end # Stub method | |||
| it "converts the filename components into render options" do | |||
| allow(view_spec).to receive(:_default_file_to_render) { "widgets/new.en.html.erb" } | |||
| view_spec.render | |||
| expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], formats: [:html], handlers: ['erb']}, {}, nil]) | |||
| expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en', :en], formats: [:html], handlers: ['erb', :erb]}, {}, nil]) | |||
There was a problem hiding this comment.
The problem with these specs are they are making exact expectations on value we know we're sending, we need an integration test for this that shows its a problem that they are not symbols.
|
@masarakki Sorry for pushing. Will you have a moment to wrap this up? |
|
It is easy to match only right type for rails version, but it is very hard to test it. |
|
FYI, this PR is moot as of 3ccd1f, 1/20/2022. |
|
Should we close this, @masarakki ? Does it resolve your case? |
|
this problem fixed by #2521 . |
6.x: https://github.com/rails/rails/blob/v6.0.3/actionview/lib/action_view/template/resolver.rb#L349-L351
5.x: https://github.com/rails/rails/blob/v5.2.4.3/actionview/lib/action_view/template/resolver.rb#L368-L374
4.x: https://github.com/rails/rails/blob/v4.2.11.3/actionview/lib/action_view/template/resolver.rb#L347-L349
Resolver search files and resolve most matched file with symbolized string,
so
_default_render_optionsmust return symbolized values.