Fix plugin generation inside applications #46074
Conversation
|
We have a failing test because of this change. Can you take a look? |
skipkayhil
force-pushed
the
fix-app-plugin-generation-2
branch
2 times, most recently
from
1a3c460
to
499770b
Compare
Previously, generating a plugin inside an application would throw an error when trying to create a dummy_app due to the generator trying to reuse the application's name. This is fixed by extracting the logic used to determine the source of an application's name, and then only using the existing name as the source for the app:update task.
skipkayhil
force-pushed
the
fix-app-plugin-generation-2
branch
from
499770b
to
3dfa9b9
Compare
| @@ -79,7 +79,7 @@ class ChangeGeneratorTest < Rails::Generators::TestCase | |||
|
|
|||
| assert_file("config/database.yml") do |content| | |||
| assert_match "adapter: mysql2", content | |||
| assert_match "database: test_app", content | |||
| assert_match "database: tmp_production", content | |||
There was a problem hiding this comment.
In a real application, Rails.application throws a NoMethodError inside the Change Generator (bin/rails db:system:change). So instead of keeping the Rails.application.class... || File.basename for the Change Generator I think it makes more sense for the tests to reflect the real behavior (always File.basename, which is /tmp for these tests)
There was a problem hiding this comment.
But how was this passing before?
There was a problem hiding this comment.
The Rails.application.class... path was hit before because the generator test requires generators/generators_test_helper -> abstract_unit which requires rails/all and defines TestApp::Application -> sets Rails.application.
In the bin_setup_test/real application case, config/application.rb is never required during bin/rails db:system:change so it never sets Rails.application.
My hunch is that this was fixed in Rails 7.1 by rails/rails#46074, but I'm less concerned here because this test suite uses a Dummy app for all of the tests. This is a problem I've been working on in rails/rails#50427, to remove the dummy applications and replace them with a generated app like we've done here with the installer test. I'm happy to investigate replacing the dummy app here afterwards.
Motivation / Background
Previously, generating a plugin inside an application would throw an error when trying to create a dummy_app due to the generator trying to reuse the application's name.
Detail
This is fixed by extracting the logic used to determine the source of an application's name, and then only using the existing name as the source for the app:update and db:change tasks.
Additional information
Closes #45824
Fixes #18073
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]main(if not - rebase it).