search

jejacks0n / teaspoon

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

Restoring coverage for projects using require.js #240

Closed JohnRiv closed 9 years ago

JohnRiv commented 10 years ago

This fixes Issue #220, but there may be a better way. This worked for my setup but as you can see I had to remove some recent edits to the file (that broke my setup), so this is probably breaking some setups while fixing others.

jejacks0n commented 10 years ago

I appreciate the work, but I'm sorry, I can't merge this as it is.

I moved away from this structure because it's slow and I wanted to use the built in functionality as much as possible, because it's a pain to maintain the hacks that were there / this pull request adds back.

I don't use requirejs, so you'll have to help me through this. I personally don't care for requirejs, but that's a different topic.

Are you loading files async with requirejs? If so, you will need to see if requirejs allows you to modify those urls to include the instrument param.

If not, then I assume any sprockets #= require statement should work as expected.

In general, the lack of consistency with requirejs and how people are using it has made this really hard, but sadly this commit doesn't fix it.

JohnRiv commented 10 years ago

Yeah I didn't expect this to be merged on the first shot, was more hoping that it would help the conversation in getting require.js support fixed.

Regarding my setup, I am not loading files async. Upon further investigation, I may actually be able to solve it with a separate boot file and some fixes to the define statements in our spec files. I'll see if that will work and report back.

jejacks0n commented 10 years ago

I would like to get something good in, and I'm down with the conversation. I have just found it to be painful and hackish thus far, but would love a proper and clean solution that I don't have to think about again. =)

JohnRiv commented 10 years ago

OK so I was able to get coverage working without a pull request so I'm closing this. Here's my setup in case anyone else runs into a similar situation:

  1. Use require.js whenever possible, but don't asynchronously load the dependent JS files. Instead include them in the JS manifest(s)
  2. Define all modules with a name
  3. Define all specs with a name that matches the path to the file (view the source of the spec runner in a browser to verify the names you should use)
  4. Include the JS files using sprockets #= require statements in the manifests that are included in spec_helper.js.erb
  5. Create a custom boot file at spec/javascripts/fixtures/_boot_requirejs_sync.html.erb. See this gist for the contents: https://gist.github.com/JohnRiv/63bffffeacbf4dde3833 That is based on the _boot_require_js.html.erb in Teaspoon except it includes the JS files directly in the test runner instead of including require.js and having RequireJS asynchronously load the dependent JS files.
  6. In teaspoon_env.rb, set suite.boot_partial = "/boot_requirejs_sync"
jejacks0n commented 10 years ago

Awesome, thank you for providing those details. Can someone come up with a plan of implementation and documentation as to avoid this stuff in the future? If there's an implementation aspect we need to include requirejs into the dummy app and have proper specs.

It will require someone who has better knowledge of how people use requirejs than I have, since it seems to be very disparate in how it's being used.