Fix Propshaft tests - Take 2#51913
Merged
dhh merged 1 commit intorails:propshaft-by-defaultfrom May 26, 2024
Merged
Conversation
be4684c to
474a2eb
Compare
e6a2650 to
aa9284b
Compare
d21813a to
42697c4
Compare
42697c4 to
ec9197e
Compare
Contributor
Author
Member
|
Excellent work, @lazaronixon!! |
dhh
added a commit
that referenced
this pull request
May 26, 2024
* Change asset pipeline default to Propshaft * Use :all for stylesheets when propshaft is active * Switch to using propshaft as the default (still need to find a way to tests against sprockets too) * Fix tests that rely on sprockets being used * Fix Propshaft tests (#51913) * Update railties/test/generators/shared_generator_tests.rb Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com> --------- Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com>
| PROJECT_ROOT = File.expand_path("../../../../", __dir__) | ||
| ALLOWED_WARNINGS = Regexp.union( | ||
| /circular require considered harmful.*delayed_job/, # Bug in delayed job. | ||
| /circular require considered harmful.*rails-html-sanitizer/, # Bug when sprockets-rails is not required. |
Member
There was a problem hiding this comment.
If there is a bug in one of Rails' gems, why aren't fixing instead of delaying the problem? Who is expected to fix this?
| FileUtils.mkdir_p(app_template_path) | ||
|
|
||
| sh "#{Gem.ruby} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --skip-bundle --no-rc --quiet" | ||
| sh "#{Gem.ruby} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --asset-pipeline=sprockets --skip-bundle --no-rc --quiet" |
Member
There was a problem hiding this comment.
Isn't this just delaying the problem? The test app is still generating using sprockets. Who is supposed to fix this?
| # app-specific one: we don't want to require every gem that lists. | ||
| contents = File.read("#{app_template_path}/config/application.rb") | ||
| contents.sub!(/^Bundler\.require.*/, "%w(importmap-rails).each { |r| require r }") | ||
| contents.sub!(/^Bundler\.require.*/, "%w(sprockets/railtie importmap-rails).each { |r| require r }") |
Member
There was a problem hiding this comment.
Do we need to do this? sprockets-rails already require this file, so Bundler.require should just work.
xjunior
pushed a commit
to xjunior/rails
that referenced
this pull request
Jun 9, 2024
* Change asset pipeline default to Propshaft * Use :all for stylesheets when propshaft is active * Switch to using propshaft as the default (still need to find a way to tests against sprockets too) * Fix tests that rely on sprockets being used * Fix Propshaft tests (rails#51913) * Update railties/test/generators/shared_generator_tests.rb Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com> --------- Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com>
Set2005
pushed a commit
to Set2005/fix-association-initialize-order
that referenced
this pull request
Jul 8, 2024
* Change asset pipeline default to Propshaft * Use :all for stylesheets when propshaft is active * Switch to using propshaft as the default (still need to find a way to tests against sprockets too) * Fix tests that rely on sprockets being used * Fix Propshaft tests (rails#51913) * Update railties/test/generators/shared_generator_tests.rb Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com> --------- Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com>
DanielaVelasquez
pushed a commit
to DanielaVelasquez/rails
that referenced
this pull request
Oct 3, 2024
* Change asset pipeline default to Propshaft * Use :all for stylesheets when propshaft is active * Switch to using propshaft as the default (still need to find a way to tests against sprockets too) * Fix tests that rely on sprockets being used * Fix Propshaft tests (rails#51913) * Update railties/test/generators/shared_generator_tests.rb Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com> --------- Co-authored-by: Lázaro Nixon <lazaronixon@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes tests blocking Propshaft from being merged to rails.