From 330becbdd098faba6275b468e7ccae1b5717432a Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
Date: Wed, 5 Jan 2022 15:33:18 -0600
Subject: [PATCH] Fix asset pipeline errors for plugin dummy apps

To fix #43920, f292daad7051275f7b557b352876cfeb3988f639 added
`sprockets-rails` to the generated `Gemfile` for engine plugins because
their dummy apps use Sprockets.  However, non-engine plugins exhibit the
same issue because their dummy apps also use Sprockets.

This commit forces `skip_asset_pipeline` to be true when a plugin is not
an engine, and fixes several tests that failed to detect these issues
because they were accidentally using the `rails/rails` `Gemfile` instead
of the generated plugin `Gemfile`.
---
 .../rails/plugin/plugin_generator.rb          |  4 +
 .../test/generators/app_generator_test.rb     | 39 +++-----
 .../test/generators/plugin_generator_test.rb  | 88 +++++++++----------
 .../test/generators/plugin_test_helper.rb     | 11 +++
 .../generators/plugin_test_runner_test.rb     | 16 +++-
 .../test/generators/shared_generator_tests.rb | 11 +++
 .../generators/test_runner_in_engine_test.rb  | 14 ++-
 7 files changed, 104 insertions(+), 79 deletions(-)

diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
index 1a056c288e..8065df10ab 100644
--- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
+++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
@@ -226,6 +226,10 @@ module Rails
       def initialize(*args)
         @dummy_path = nil
         super
+
+        if !engine? || !with_dummy_app?
+          self.options = options.merge(skip_asset_pipeline: true).freeze
+        end
       end
 
       public_task :set_default_accessors!
diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb
index 3b47748259..3566278dbb 100644
--- a/railties/test/generators/app_generator_test.rb
+++ b/railties/test/generators/app_generator_test.rb
@@ -944,26 +944,25 @@ class AppGeneratorTest < Rails::Generators::TestCase
   end
 
   def test_minimal_rails_app
-    app_root = File.join(destination_root, "myapp")
-    run_generator [app_root, "--minimal"]
+    run_generator [destination_root, "--minimal"]
 
-    assert_no_file "#{app_root}/config/storage.yml"
-    assert_no_file "#{app_root}/config/cable.yml"
-    assert_no_file "#{app_root}/views/layouts/mailer.html.erb"
-    assert_no_file "#{app_root}/app/jobs/application.rb"
-    assert_file "#{app_root}/app/views/layouts/application.html.erb" do |content|
+    assert_no_file "config/storage.yml"
+    assert_no_file "config/cable.yml"
+    assert_no_file "views/layouts/mailer.html.erb"
+    assert_no_file "app/jobs/application.rb"
+    assert_file "app/views/layouts/application.html.erb" do |content|
       assert_no_match(/data-turbo-track/, content)
     end
-    assert_file "#{app_root}/config/environments/development.rb" do |content|
+    assert_file "config/environments/development.rb" do |content|
       assert_no_match(/config\.active_storage/, content)
     end
-    assert_file "#{app_root}/config/environments/production.rb" do |content|
+    assert_file "config/environments/production.rb" do |content|
       assert_no_match(/config\.active_job/, content)
     end
-    assert_file "#{app_root}/config/boot.rb" do |content|
+    assert_file "config/boot.rb" do |content|
       assert_no_match(/require "bootsnap\/setup"/, content)
     end
-    assert_file "#{app_root}/config/application.rb" do |content|
+    assert_file "config/application.rb" do |content|
       assert_match(/#\s+require\s+["']active_job\/railtie["']/, content)
       assert_match(/#\s+require\s+["']active_storage\/engine["']/, content)
       assert_match(/#\s+require\s+["']action_mailer\/railtie["']/, content)
@@ -972,8 +971,8 @@ class AppGeneratorTest < Rails::Generators::TestCase
       assert_match(/#\s+require\s+["']action_cable\/engine["']/, content)
     end
 
-    assert_no_gem "jbuilder", app_root
-    assert_no_gem "web-console", app_root
+    assert_no_gem "jbuilder"
+    assert_no_gem "web-console"
   end
 
   private
@@ -985,18 +984,4 @@ class AppGeneratorTest < Rails::Generators::TestCase
     def action(*args, &block)
       capture(:stdout) { generator.send(*args, &block) }
     end
-
-    def assert_gem(gem, constraint = nil, app_path = ".")
-      if constraint
-        assert_file File.join(app_path, "Gemfile"), /^\s*gem\s+["']#{gem}["'], #{constraint}$*/
-      else
-        assert_file File.join(app_path, "Gemfile"), /^\s*gem\s+["']#{gem}["']$*/
-      end
-    end
-
-    def assert_no_gem(gem, app_path = ".")
-      assert_file File.join(app_path, "Gemfile") do |content|
-        assert_no_match(gem, content)
-      end
-    end
 end
diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb
index 81f50c9593..3e0fe7f78d 100644
--- a/railties/test/generators/plugin_generator_test.rb
+++ b/railties/test/generators/plugin_generator_test.rb
@@ -269,20 +269,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase
     assert_match(/It works from file!/, run_generator([destination_root, "-m", "lib/template.rb"]))
   end
 
-  def test_ensure_that_tests_work
-    run_generator
-    FileUtils.cd destination_root
-    quietly { system "bundle install" }
-    assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bin/test 2>&1`)
-  end
-
-  def test_ensure_that_tests_works_in_full_mode
-    run_generator [destination_root, "--full", "--skip_active_record"]
-    FileUtils.cd destination_root
-    quietly { system "bundle install" }
-    assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`)
-  end
-
   def test_ensure_that_migration_tasks_work_with_mountable_option
     run_generator [destination_root, "--mountable"]
     FileUtils.cd destination_root
@@ -291,39 +277,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase
     assert $?.success?, "Command failed: #{output}"
   end
 
-  def test_ensure_that_sprokets_is_required_when_mountable
-    run_generator [destination_root, "--mountable"]
-    assert_file "Gemfile", /^gem "sprockets-rails"/
-  end
-
-  def test_ensure_that_sprokets_is_required_when_full
-    run_generator [destination_root, "--full"]
-    assert_file "Gemfile", /^gem "sprockets-rails"/
-  end
-
-  def test_ensure_that_sprokets_is_not_required_when_not_mountable_or_full
-    run_generator
-    assert_file "Gemfile" do |content|
-      assert_no_match(/sprockets-rails/, content)
-    end
-  end
-
-  def test_ensure_that_sprokets_is_not_required_when_assets_pipeline_is_skipped
-    run_generator [destination_root, "--skip-asset-pipeline", "--mountable"]
-
-    assert_file "Gemfile" do |contents|
-      assert_no_match(/sprockets-rails/, contents)
-    end
-  end
-
-  def test_ensure_that_sprokets_is_not_required_when_assets_pipeline_is_not_sprockets
-    run_generator [destination_root, "--asset-pipeline=propshaft", "--mountable"]
-
-    assert_file "Gemfile" do |contents|
-      assert_no_match(/sprockets-rails/, contents)
-    end
-  end
-
   def test_creating_engine_in_full_mode
     run_generator [destination_root, "--full"]
     assert_file "app/assets/stylesheets/bukkits"
@@ -664,6 +617,47 @@ class PluginGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_dummy_application_skips_asset_pipeline_when_simple_railtie
+    run_generator
+
+    assert_no_gem "sprockets-rails"
+    assert_no_file "test/dummy/config/initializers/assets.rb"
+    assert_file "test/dummy/config/environments/development.rb" do |content|
+      assert_no_match "config.assets", content
+    end
+  end
+
+  def test_dummy_application_configures_asset_pipeline_when_mountable
+    run_generator [destination_root, "--mountable"]
+
+    assert_gem "sprockets-rails"
+    assert_file "test/dummy/app/assets/config/manifest.js"
+  end
+
+  def test_dummy_application_configures_asset_pipeline_when_full
+    run_generator [destination_root, "--full"]
+
+    assert_gem "sprockets-rails"
+    assert_file "test/dummy/app/assets/config/manifest.js"
+  end
+
+  def test_dummy_application_skips_asset_pipeline_when_flag_skip_asset_pipeline
+    run_generator [destination_root, "--mountable", "--skip-asset-pipeline"]
+
+    assert_no_gem "sprockets-rails"
+    assert_no_file "test/dummy/config/initializers/assets.rb"
+    assert_file "test/dummy/config/environments/development.rb" do |content|
+      assert_no_match "config.assets", content
+    end
+  end
+
+  def test_no_asset_pipeline_gem_when_no_dummy_application
+    run_generator [destination_root, "--mountable", "--skip-test"]
+
+    assert_no_gem "sprockets-rails"
+    assert_no_directory "test/dummy"
+  end
+
   def test_skipping_gemspec
     run_generator [destination_root, "--skip-gemspec"]
     assert_no_file "bukkits.gemspec"
diff --git a/railties/test/generators/plugin_test_helper.rb b/railties/test/generators/plugin_test_helper.rb
index 1357d042cb..ca38f2c972 100644
--- a/railties/test/generators/plugin_test_helper.rb
+++ b/railties/test/generators/plugin_test_helper.rb
@@ -23,4 +23,15 @@ module PluginTestHelper
       f.puts contents
     end
   end
+
+  def fill_in_gemspec_fields(gemspec_path = "#{plugin_path}/#{File.basename plugin_path}.gemspec")
+    # Some fields must be a valid URL.
+    filled_in = File.read(gemspec_path).gsub(/"TODO.*"/, "http://example.com".inspect)
+    File.write(gemspec_path, filled_in)
+  end
+
+  def resolve_rails_gem_to_repository(gemfile_path = "#{plugin_path}/Gemfile")
+    repository_path = File.expand_path("../../..", __dir__)
+    File.write(gemfile_path, "gem 'rails', path: #{repository_path.inspect}\n", mode: "a")
+  end
 end
diff --git a/railties/test/generators/plugin_test_runner_test.rb b/railties/test/generators/plugin_test_runner_test.rb
index cef53f9105..d30c149a94 100644
--- a/railties/test/generators/plugin_test_runner_test.rb
+++ b/railties/test/generators/plugin_test_runner_test.rb
@@ -1,13 +1,17 @@
 # frozen_string_literal: true
 
 require "generators/plugin_test_helper"
+require "env_helpers"
 
 class PluginTestRunnerTest < ActiveSupport::TestCase
   include PluginTestHelper
+  include EnvHelpers
 
   def setup
     @destination_root = Dir.mktmpdir("bukkits")
-    Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --skip-bundle --webpack` }
+    Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --skip-bundle` }
+    fill_in_gemspec_fields
+    resolve_rails_gem_to_repository
     plugin_file "test/dummy/db/schema.rb", ""
   end
 
@@ -15,6 +19,10 @@ class PluginTestRunnerTest < ActiveSupport::TestCase
     FileUtils.rm_rf(@destination_root)
   end
 
+  def test_run_default
+    assert_match "0 failures, 0 errors", run_test_command
+  end
+
   def test_run_single_file
     create_test_file "foo"
     create_test_file "bar"
@@ -110,7 +118,9 @@ class PluginTestRunnerTest < ActiveSupport::TestCase
       "#{@destination_root}/bukkits"
     end
 
-    def run_test_command(arguments)
-      Dir.chdir(plugin_path) { `bin/test #{arguments}` }
+    def run_test_command(arguments = "")
+      Dir.chdir(plugin_path) do
+        switch_env("BUNDLE_GEMFILE", "") { `bin/test #{arguments}` }
+      end
     end
 end
diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb
index 0debad7453..b05deb9d68 100644
--- a/railties/test/generators/shared_generator_tests.rb
+++ b/railties/test/generators/shared_generator_tests.rb
@@ -387,4 +387,15 @@ module SharedGeneratorTests
         assert_equal gemfile[rails_gem_pattern], bundle_command_rails_gems[1]
       end
     end
+
+    def assert_gem(name, constraint = nil)
+      constraint_pattern = /, #{Regexp.escape constraint}/ if constraint
+      assert_file "Gemfile", %r/^\s*gem ["']#{name}["']#{constraint_pattern}/
+    end
+
+    def assert_no_gem(name)
+      assert_file "Gemfile" do |content|
+        assert_no_match %r/gem ["']#{name}["']/, content
+      end
+    end
 end
diff --git a/railties/test/generators/test_runner_in_engine_test.rb b/railties/test/generators/test_runner_in_engine_test.rb
index bd102a32b5..896f156954 100644
--- a/railties/test/generators/test_runner_in_engine_test.rb
+++ b/railties/test/generators/test_runner_in_engine_test.rb
@@ -1,13 +1,17 @@
 # frozen_string_literal: true
 
 require "generators/plugin_test_helper"
+require "env_helpers"
 
 class TestRunnerInEngineTest < ActiveSupport::TestCase
   include PluginTestHelper
+  include EnvHelpers
 
   def setup
     @destination_root = Dir.mktmpdir("bukkits")
     Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --full --skip-bundle` }
+    fill_in_gemspec_fields
+    resolve_rails_gem_to_repository
     plugin_file "test/dummy/db/schema.rb", ""
   end
 
@@ -15,6 +19,10 @@ class TestRunnerInEngineTest < ActiveSupport::TestCase
     FileUtils.rm_rf(@destination_root)
   end
 
+  def test_run_default
+    assert_match "0 failures, 0 errors", run_test_command
+  end
+
   def test_rerun_snippet_is_relative_path
     create_test_file "post", pass: false
 
@@ -28,7 +36,9 @@ class TestRunnerInEngineTest < ActiveSupport::TestCase
       "#{@destination_root}/bukkits"
     end
 
-    def run_test_command(arguments)
-      Dir.chdir(plugin_path) { `bin/rails test #{arguments}` }
+    def run_test_command(arguments = "")
+      Dir.chdir(plugin_path) do
+        switch_env("BUNDLE_GEMFILE", "") { `bin/rails test #{arguments}` }
+      end
     end
 end