Skip to content

Commit e5e4b80

Browse files
committed
Change skeleton test to assert exact file output
Previously, the skeleton test would assert that every file in the `default_files` list existed. However, since there is no requirement that a file is added to this list when added to the generator the list can (and has) become out of sync with the exact generated output. This commit refactors the skeleton test to gather the full list of files created by the generator so that it can be compared exactly to the list of `default_files`. This ensures the generated file list must be updated when any file is added or removed from the generator. Additionally, by using array equality the sorting of the `default_files` list is enforced (it was previously _mostly_ alphabetical, but not fully). The new skeleton test assertion was written to only assert files instead of folders for a few reasons: - asserting that folders exist is redundant if a file exists within a folder (and the `default_files` list would be significantly longer with all folders) - any folder generated without any files inside it will not be tracked by Git, so other code cannot assume these empty folders exist Because of these reasons, this patch also includes a change to no longer generate the `tmp/cache/assets` folder by default. The folder is an implementation detail of Sprockets and doesn't need any special treatment in Rails. The `Sprockets::Cache::FileStore` will create the directory if it does not exist (which it usually won't since `tmp/cache` is excluded from Git by the `.gitignore`), and the `tmp:cache:clear` task already clears all subdirectories of `tmp/cache`. Finally, some tests making assertions about the default output of the app generator were removed as they have been made redundant by the improvements to the skeleton test.
1 parent 351a8d9 commit e5e4b80

File tree

5 files changed

+65
-68
lines changed

5 files changed

+65
-68
lines changed

railties/lib/rails/generators/rails/app/app_generator.rb

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,6 @@ def system_test
257257
def tmp
258258
empty_directory_with_keep_file "tmp"
259259
empty_directory_with_keep_file "tmp/pids"
260-
empty_directory "tmp/cache"
261-
empty_directory "tmp/cache/assets"
262260
end
263261

264262
def vendor
@@ -461,7 +459,6 @@ def delete_app_assets_if_api_option
461459
if options[:api]
462460
remove_dir "app/assets"
463461
remove_dir "lib/assets"
464-
remove_dir "tmp/cache/assets"
465462
end
466463
end
467464

railties/lib/rails/tasks/tmp.rake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace :tmp do
77
tmp_dirs = [ "tmp/cache",
88
"tmp/sockets",
99
"tmp/pids",
10-
"tmp/cache/assets" ]
10+
]
1111

1212
tmp_dirs.each { |d| directory d }
1313

railties/test/generators/api_app_generator_test.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ def skipped_files
177177
config/initializers/permissions_policy.rb
178178
lib/assets
179179
test/helpers
180-
tmp/cache/assets
181180
public/404.html
182181
public/422.html
183182
public/426.html

railties/test/generators/app_generator_test.rb

Lines changed: 39 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,90 +5,88 @@
55
require "generators/shared_generator_tests"
66

77
DEFAULT_APP_FILES = %w(
8+
.dockerignore
9+
.git
810
.gitattributes
911
.github/dependabot.yml
1012
.github/workflows/ci.yml
1113
.gitignore
12-
.dockerignore
1314
.rubocop.yml
1415
.ruby-version
15-
README.md
16+
Dockerfile
1617
Gemfile
18+
README.md
1719
Rakefile
18-
Dockerfile
19-
config.ru
2020
app/assets/config/manifest.js
21-
app/assets/images
22-
app/assets/stylesheets
21+
app/assets/images/.keep
2322
app/assets/stylesheets/application.css
2423
app/channels/application_cable/channel.rb
2524
app/channels/application_cable/connection.rb
26-
app/controllers
2725
app/controllers/application_controller.rb
28-
app/controllers/concerns
29-
app/helpers
26+
app/controllers/concerns/.keep
3027
app/helpers/application_helper.rb
31-
app/mailers
28+
app/jobs/application_job.rb
3229
app/mailers/application_mailer.rb
33-
app/models
3430
app/models/application_record.rb
35-
app/models/concerns
36-
app/jobs
37-
app/jobs/application_job.rb
38-
app/views/layouts
31+
app/models/concerns/.keep
3932
app/views/layouts/application.html.erb
4033
app/views/layouts/mailer.html.erb
4134
app/views/layouts/mailer.text.erb
42-
bin/docker-entrypoint
35+
app/views/pwa/manifest.json.erb
36+
app/views/pwa/service-worker.js
4337
bin/brakeman
38+
bin/docker-entrypoint
4439
bin/rails
4540
bin/rake
4641
bin/rubocop
4742
bin/setup
43+
config.ru
4844
config/application.rb
4945
config/boot.rb
5046
config/cable.yml
47+
config/credentials.yml.enc
48+
config/database.yml
5149
config/environment.rb
52-
config/environments
5350
config/environments/development.rb
5451
config/environments/production.rb
5552
config/environments/test.rb
56-
config/initializers
5753
config/initializers/assets.rb
5854
config/initializers/content_security_policy.rb
5955
config/initializers/enable_yjit.rb
6056
config/initializers/filter_parameter_logging.rb
6157
config/initializers/inflections.rb
62-
config/locales
58+
config/initializers/permissions_policy.rb
6359
config/locales/en.yml
60+
config/master.key
6461
config/puma.rb
6562
config/routes.rb
66-
config/credentials.yml.enc
6763
config/storage.yml
68-
db
6964
db/seeds.rb
70-
lib
71-
lib/tasks
72-
lib/assets
73-
log
74-
public
75-
storage
65+
lib/assets/.keep
66+
lib/tasks/.keep
67+
log/.keep
68+
public/404.html
69+
public/422.html
70+
public/426.html
71+
public/500.html
72+
public/icon.png
73+
public/icon.svg
74+
public/robots.txt
75+
storage/.keep
7676
test/application_system_test_case.rb
77-
test/test_helper.rb
78-
test/fixtures
79-
test/fixtures/files
8077
test/channels/application_cable/connection_test.rb
81-
test/controllers
82-
test/models
83-
test/helpers
84-
test/mailers
85-
test/integration
86-
test/system
87-
vendor
88-
tmp
89-
tmp/cache
90-
tmp/cache/assets
91-
tmp/storage
78+
test/controllers/.keep
79+
test/fixtures/files/.keep
80+
test/helpers/.keep
81+
test/integration/.keep
82+
test/mailers/.keep
83+
test/models/.keep
84+
test/system/.keep
85+
test/test_helper.rb
86+
tmp/.keep
87+
tmp/pids/.keep
88+
tmp/storage/.keep
89+
vendor/.keep
9290
)
9391

9492
class AppGeneratorTest < Rails::Generators::TestCase
@@ -111,12 +109,6 @@ def test_skip_bundle
111109
assert_file "Gemfile"
112110
end
113111

114-
def test_assets
115-
run_generator
116-
117-
assert_file("app/assets/stylesheets/application.css")
118-
end
119-
120112
def test_invalid_javascript_option_raises_an_error
121113
content = capture(:stderr) { run_generator([destination_root, "-j", "unknown"]) }
122114
assert_match(/Expected '--javascript' to be one of/, content)
@@ -132,11 +124,6 @@ def test_invalid_css_option_raises_an_error
132124
assert_match(/Expected '--css' to be one of/, content)
133125
end
134126

135-
def test_application_job_file_present
136-
run_generator
137-
assert_file("app/jobs/application_job.rb")
138-
end
139-
140127
def test_invalid_application_name_raises_an_error
141128
content = capture(:stderr) { run_generator [File.join(destination_root, "43-things")] }
142129
assert_equal "Invalid application name 43-things. Please give a name which does not start with numbers.\n", content
@@ -183,17 +170,6 @@ def test_application_name_is_detected_if_it_exists_and_app_folder_renamed
183170
assert_file "#{app_moved_root}/config/environment.rb", /Rails\.application\.initialize!/
184171
end
185172

186-
def test_new_application_not_include_api_initializers
187-
run_generator
188-
189-
assert_no_file "config/initializers/cors.rb"
190-
end
191-
192-
def test_new_application_doesnt_need_defaults
193-
run_generator
194-
assert_empty Dir.glob("config/initializers/new_framework_defaults_*.rb", base: destination_root)
195-
end
196-
197173
def test_new_application_load_defaults
198174
run_generator
199175
assert_file "config/application.rb", /\s+config\.load_defaults #{Rails::VERSION::STRING.to_f}/

railties/test/generators/shared_generator_tests.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# frozen_string_literal: true
22

3+
require "find"
34
require "shellwords"
45
require "env_helpers"
56

@@ -52,6 +53,30 @@ def test_implied_options_with_conflicting_option
5253
def test_skeleton_is_created
5354
run_generator
5455

56+
generated_files_and_folders = []
57+
58+
Find.find(destination_root) do |absolute_path|
59+
next if absolute_path == destination_root
60+
61+
pathname = Pathname.new(absolute_path)
62+
git_folder = pathname.basename.to_s == ".git"
63+
64+
if git_folder || pathname.file?
65+
generated_files_and_folders << pathname
66+
.relative_path_from(destination_root)
67+
.to_s
68+
elsif pathname.directory? && pathname.children.empty?
69+
flunk "`#{pathname} was generated but is an empty directory"
70+
end
71+
72+
Find.prune if git_folder
73+
end
74+
75+
# assert differences first for better error messages
76+
assert_empty generated_files_and_folders.difference(default_files)
77+
assert_empty default_files.difference(generated_files_and_folders)
78+
assert_equal generated_files_and_folders.sort, default_files, "The expected list of generated files is not alphabetical"
79+
5580
default_files.each { |path| assert_file path }
5681
end
5782

0 commit comments

Comments
 (0)