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 skip_system_test not being preserved in update #45648

Merged
merged 1 commit into from Jul 24, 2022

Conversation

skipkayhil
Copy link
Member

Summary

Previously running app:update always would template an application.rb
file without the config.generators.system_tests = nil set, even for
applications that previously had it set.

Previously running app:update always would template an application.rb
file without the `config.generators.system_tests = nil` set, even for
applications that previously had it set.
@rails-bot rails-bot bot added the railties label Jul 23, 2022
@@ -29,6 +29,7 @@ def generator_options
options[:skip_action_text] = !defined?(ActionText::Engine)
options[:skip_action_cable] = !defined?(ActionCable::Engine)
options[:skip_test] = !defined?(Rails::TestUnitRailtie)
options[:skip_system_test] = Rails.application.config.generators.system_tests.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, options[:skip_system_test] triggers !depends_on_system_test?:

def depends_on_system_test?
!(options[:skip_system_test] || options[:skip_test] || options[:api])
end

which triggers config.generators.system_tests = nil:

<%- elsif !depends_on_system_test? -%>
# Don't generate system test files.
config.generators.system_tests = nil
<%- end -%>

@jonathanhefner jonathanhefner merged commit 7548d76 into rails:main Jul 24, 2022
@jonathanhefner
Copy link
Member

Thank you, @skipkayhil! ✔️ ✔️

@skipkayhil
Copy link
Member Author

Would this be worth a backport? Asking because I saw 018fa5a and 6cda769 were and this is a similar fix.

@skipkayhil skipkayhil deleted the fix-skip-system-test-update branch July 24, 2022 03:05
jonathanhefner added a commit that referenced this pull request Jul 24, 2022
Fix skip_system_test not being preserved in update

(cherry picked from commit 7548d76)
@jonathanhefner
Copy link
Member

Backported to 7-0-stable in 53334c2.

@skipkayhil By the way, should we add a regression test, similar to this one?

@skipkayhil
Copy link
Member Author

By the way, should we add a regression test, similar to this one?

Definitely, thanks for the pointer as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants