jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

Add coffee-rails dependency #528

Closed md5 closed 6 years ago

md5 commented 6 years ago

Scaffolded Rails projects include coffee-rails unless they use --skip-coffee, but not all applications include it.

This PR makes the dependency explicit.

See #405

md5 commented 6 years ago

Looks like the Travis failures are not specific to this branch. I see the same failures for other recent PRs.

BrianHawley commented 6 years ago

Please don't. This will add the coffee-rails dependency to projects that don't have or want it. It's better to make a change to the README or wiki to mention this issue.

md5 commented 6 years ago

@BrianHawley That doesn't make any sense. The fact of the matter is that teaspoon requires coffee-rails. You simply can't use it in a project that does not have coffee-rails installed somehow.

BrianHawley commented 6 years ago

If you don't use the --coffee option and don't write your specs in CoffeeScript then you don't need coffee-rails. Teaspoon probably could use coffee-rails as a development dependency, to convert its own source from CoffeeScript to js, and it needs it for its tests. However, it doesn't need it as a runtime dependency of the packaged gem because all of the js is preconverted. If you as a user of Teaspoon don't use CoffeeScript, it should still work. It works for us.

md5 commented 6 years ago

@BrianHawley Where is this preconverted JS you speak of? I only see Coffeescript files here: https://github.com/jejacks0n/teaspoon/tree/master/app/assets/javascripts/teaspoon

Are you not using asset precompilation? The problem is that if you run assets:precompile, the Sprockets pipeline attempts to compile the *.coffee files from this gem and fails if it can't require 'coffee_script'.

md5 commented 6 years ago

Ah, I see them now: https://github.com/jejacks0n/teaspoon/tree/master/teaspoon-jasmine/lib/teaspoon/jasmine/assets

md5 commented 6 years ago

Actually, I see a bunch of *.coffee files there as well...

BrianHawley commented 6 years ago

It looks like we only run the tests in CI mode, which doesn't use the browser UI, with its coffee files. Weird that we've been using this for a year without coffee-rails and it's never caused a problem for us. We should consider adding coffee-rails as a development dependency ourselves.

BrianHawley commented 6 years ago

Nope, I was wrong, the coffee files are precompiled by teaspoon-devkit.rb in Teaspoon's development mode. I'm going to check the generated files in the installed's gem's directory to be sure.

md5 commented 6 years ago

Looks like the teaspoon/*.js here may be the source of this problem: https://github.com/jejacks0n/teaspoon/blob/0335e79b385c2a74d2d963417b8857368a6cc928/lib/teaspoon/configuration.rb#L24

It's unclear to me which *.js files this is attempting to precompile, but this is causing Sprockets to try to process files like app/assets/javascripts/teaspoon/error.coffee. As far as I can tell (thanks to you pointing it out @BrianHawley), these Coffeescript files should already be precompiled into individual driver files (like teaspoon-jasmine/lib/teaspoon/jasmine/assets/teaspoon-jasmine2.js).

BrianHawley commented 6 years ago

Looks like the coffee files exist in the "app/assets/javascripts/teaspoon" directory when the gem is installed, so the js files aren't precompiled. I should look into this, because not having coffee-rails as a runtime dependency would be considered an advantage to people who don't want it. I don't really know CoffeeScript so I have no opinion, but I know many people who don't like it.

md5 commented 6 years ago

@BrianHawley I'm not a fan myself

BrianHawley commented 6 years ago

I checked with our devs and they use the browser UI, so that means that the coffee files are compiled somehow without using coffee-rails. It's possible that the old sprockets version we use has coffee support built-in and doesn't need coffee-rails.

BrianHawley commented 6 years ago

Yup, that was it. Our version of sprockets uses tilt to handle the coffee files, no coffee-rails needed. It looks like Teaspoon's development dependencies are handled by the teaspoon-devkit gemspec, which has coffee-rails as a dependency already.

That means that the best solution for your problem is to document that if you're using sprockets 3+, you need to add coffee-rails as a dependency. I think the Installation section of the README is the best place to put it.

md5 commented 6 years ago

@BrianHawley I disagree that that's the best solution since Sprockets 4 is the current version and earlier versions are not maintained.

md5 commented 6 years ago

After applying the following diff, I was able to run my test from https://github.com/jejacks0n/teaspoon/issues/405#issuecomment-323471804 (adjusting to point at my local copy of teaspoon):

commit c8320b0214ef4d7521aac5acecd5a0d3b07faf9a
Author: Mike Dillon <mike.dillon@disney.com>
Date:   Fri Dec 29 12:41:42 2017 -0800

    Remove wildcards for teaspoon/*.js and support/*.js from precompile paths

diff --git a/lib/teaspoon/configuration.rb b/lib/teaspoon/configuration.rb
index 00ee7b7..f9b48d8 100644
--- a/lib/teaspoon/configuration.rb
+++ b/lib/teaspoon/configuration.rb
@@ -21,7 +21,7 @@ module Teaspoon
     @@asset_paths    = ["spec/javascripts", "spec/javascripts/stylesheets",
                         "test/javascripts", "test/javascripts/stylesheets"]
     @@fixture_paths  = ["spec/javascripts/fixtures", "test/javascripts/fixtures"]
-    @@asset_manifest = ["teaspoon.css", "teaspoon-filterer.js", "teaspoon/*.js", "support/*.js"]
+    @@asset_manifest = ["teaspoon.css", "teaspoon-filterer.js"]

     # console runner specific
md5 commented 6 years ago

That diff is probably too aggressive. It looks like it would be OK to leave support/*.js (or just explicitly list the three support/*.js files).

md5 commented 6 years ago

Closing this in favor of #536

BrianHawley commented 6 years ago

Sprockets 4 is in beta, not the current version; Sprockets 3.5.2 is the current version. Sprockets 2.x is still in wide use, even though it's not currently maintained, so it's not a good idea to drop it when you don't really have to. In any case, there are several different CoffeeScript processors in common use, so there's no reason for Teaspoon to take sides.