Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code coverage breaks 'no jshint' for tests #16

Closed
djmitchella opened this issue Jan 9, 2015 · 4 comments
Closed

Code coverage breaks 'no jshint' for tests #16

djmitchella opened this issue Jan 9, 2015 · 4 comments

Comments

@djmitchella
Copy link

Another one related to the way things are loaded differently, I think. Repro steps:

  1. ember init
  2. ember serve
  3. go to localhost:4200/tests; there's jshint tests there (and nothing else)
  4. check the 'disable jshint' box. Now all the tests disappear.
  5. Stop server, npm install ember-cli-blanket --save-dev, ember generate ember-cli-blanket
  6. ember serve
  7. go to localhost:4200/tests. Tests still pass, but:
  8. check the 'disable jshint' box. Now, nothing changes; with blanket there, all the jshint tests still run.

(I'm not sure, but from looking at test-loader.js in ember-cli-test-loader, it seems that jshint is somehow automatically getting run on everything that's loaded. I'm guessing that the nojshint setting stops test-loader loading the files, but blanket is still loading them anyway for coverage purposes; I'm not sure if it's possible to fix this without changing how the nojshint code works)

@sglanzer-deprecated
Copy link
Owner

Some investigation into using the client flags would be useful. Ideally I'd only like to instrument and run the coverage when the "enable coverage" checkbox is flagged, so the nojshint would fall into a similar investigation.

@djmitchella
Copy link
Author

(edited, previous version of this comment was wrong):

A quick and dirty way to solve this is to change the end of blanket-loader.js from:

if (!exclude) {

to

if (!exclude && window.location.href.indexOf("coverage=true") !== -1) {

but there's probably a better way to get that flag. It means that 'enable coverage' also forces jshint on, but (for me at least) that seems reasonable.

@sglanzer-deprecated
Copy link
Owner

@djmitchella Good to know - definitely a useful starting point for making better use of the flags.

@djmitchella
Copy link
Author

This is fixed in 0.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants