Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

bundle exec rake assets:precompile shouldn't fail quietly.

If JavaScript runtime is not installed, execjs fails with error quietly,
while tests continue to run. This should not happen since it causes tests
to fail for unknown reason (#6621).

This commit assures that if JavaScript runtime is not installed, an assertion
is raised.
  • Loading branch information...
commit ebb906d23ae3212a2914b78b7d58366970898ffb 1 parent d10eb69
John Yani Vanuan authored
Showing with 24 additions and 17 deletions.
  1. +24 −17 railties/test/application/assets_test.rb
41 railties/test/application/assets_test.rb
View
@@ -17,9 +17,21 @@ def teardown
teardown_app
end
- def precompile!
+ def precompile!(env = nil)
quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:precompile` }
+ precompile_task = 'bundle exec rake assets:precompile'
+ precompile_task += ' ' + env if env
+ out = Dir.chdir(app_path){ %x[ #{precompile_task} ] }
+ assert $?.exitstatus == 0,
+ "#{precompile_task} has failed: #{out}.\
+ Probably you didn't install JavaScript runtime."
+ return out
+ end
+ end
+
+ def clean_assets!
+ quietly do
+ assert Dir.chdir(app_path){ system('bundle exec rake assets:clean') }
end
end
@@ -253,9 +265,8 @@ class ::PostsController < ActionController::Base ; end
# digest is default in false, we must enable it for test environment
add_to_env_config "test", "config.assets.digest = true"
- quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:precompile RAILS_ENV=test` }
- end
+ precompile!('RAILS_ENV=test')
+
file = Dir["#{app_path}/public/assets/application.css"].first
assert_match(/\/assets\/rails\.png/, File.read(file))
file = Dir["#{app_path}/public/assets/application-*.css"].first
@@ -285,9 +296,9 @@ class ::PostsController < ActionController::Base ; end
add_to_config "config.assets.compile = true"
ENV["RAILS_ENV"] = nil
- quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:precompile RAILS_GROUPS=assets` }
- end
+
+ precompile!('RAILS_GROUPS=assets')
+
file = Dir["#{app_path}/public/assets/application-*.css"].first
assert_match(/\/assets\/rails-([0-z]+)\.png/, File.read(file))
end
@@ -310,9 +321,7 @@ class ::PostsController < ActionController::Base ; end
app_file "public/assets/application.css", "a { color: green; }"
app_file "public/assets/subdir/broken.png", "not really an image file"
- quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:clean` }
- end
+ clean_assets!
files = Dir["#{app_path}/public/assets/**/*", "#{app_path}/tmp/cache/assets/*"]
assert_equal 0, files.length, "Expected no assets, but found #{files.join(', ')}"
@@ -440,9 +449,8 @@ class ::PostsController < ActionController::Base ; end
add_to_config "config.assets.compile = true"
add_to_config "config.assets.digest = true"
- quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:clean assets:precompile` }
- end
+ clean_assets!
+ precompile!
files = Dir["#{app_path}/public/assets/application-*.js"]
assert_equal 1, files.length, "Expected digested application.js asset to be generated, but none found"
@@ -453,9 +461,8 @@ class ::PostsController < ActionController::Base ; end
add_to_env_config "production", "config.assets.prefix = 'production_assets'"
ENV["RAILS_ENV"] = nil
- quietly do
- Dir.chdir(app_path){ `bundle exec rake assets:clean` }
- end
+
+ clean_assets!
files = Dir["#{app_path}/public/production_assets/application.js"]
assert_equal 0, files.length, "Expected application.js asset to be removed, but still exists"

4 comments on commit ebb906d

Guillermo Iguaran

This is hiding and making hard to track some errors, for example changing to rails to use last sprockets-rails version, with this commit, I'm getting as error:

  1) Failure:
test_0004_precompile application.js and application.css and all other non JS/CSS files(ApplicationTests::AssetsTest) [test/application/assets_test.rb:25]:
bundle exec rake assets:precompile has failed: .                Probably you didn't install JavaScript runtime.

reverting this commit I get a better idea about what is happening:

 1) Failure:
test_0004_precompile application.js and application.css and all other non JS/CSS files(ApplicationTests::AssetsTest) [test/application/assets_test.rb:97]:
missing /private/var/folders/nn/w852mb3j0xldmfmk32sgqz380000gn/T/d20120909-79528-s81jv8/app/public/assets/a.png
Dmitry Vorotilin

Off the top of my head, since assets compilation runs in the separate process and should always be successful we can capture STDERR and instead of probably error we can show to user original. The original issue #6621 is really hard to track in tests if we won't catch exit status.

John Yani

This commit doesn't hide anything. It just shows that precompile step has failed and quits earlier. It shows the exact line of failure. If you revert it you wouldn't know what is the cause in most cases. The missing file doesn't instantly make you aware that the precompile task has failed.

I agree that "Probably you didn't install JavaScript runtime" message might be misleading, but the "bundle exec rake assets:precompile has failed" part is perfectly correct.

I also agree that stderr should've been catched instead. I think it would be something like:

err = Dir.chdir(app_path){  %x[ #{precompile_task} 2>\&1 ] }
assert $?.exitstatus == 0,  "#{precompile_task} has failed: #{err}."
John Yani

Oh, I see somebody has already implemented this: #7586

Please sign in to comment.
Something went wrong with that request. Please try again.