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

Support CoffeeScript code coverage #19

Closed
wants to merge 1 commit into from
Closed

Support CoffeeScript code coverage #19

wants to merge 1 commit into from

Conversation

kylewelsby
Copy link
Contributor

Building on top of the work the @can3p has committed. I found that the CoffeeScript-redux compiler was too strict. @jstamerj done an amazing job of switching CoffeeScript-redux to standard CoffeeScript in Ibrik.

I just glued the bits together and updated the README to help avid CoffeeScript users add Coverage support to their projects.

This should resolve #12 and karma-runner/karma#622

Enjoy

@kylewelsby
Copy link
Contributor Author

😉 Oh and fixed up the tests.

@dignifiedquire
Copy link
Member

Can you please squash your commits together? other than that you've got my 👍

@kylewelsby
Copy link
Contributor Author

Squashed :squirrel:

@Rodeoclash
Copy link

Looking forward to getting this pull request in 👍

@janraasch
Copy link

+1

3 similar comments
@teddyhwang
Copy link

+1

@jsilvestre
Copy link

👍

@bwiklund
Copy link

bwiklund commented Nov 3, 2013

+1

@cameronmaske
Copy link

This would be great!

@leachryanb
Copy link

+1

"dependencies": {
"istanbul": "~0.1.41",
"ibrik": "git://github.com/HBOCodeLabs/ibrik.git#304cd84936d33cf38a05a1a4fe58c82d54ff6cfa",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to use https please, as some people have problems due to their networks being restricted to use git protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 🐱

@tbaum
Copy link

tbaum commented Nov 8, 2013

+1

@paulRbr
Copy link

paulRbr commented Nov 10, 2013

👍 This is great thanks for the work!

@kylewelsby
Copy link
Contributor Author

thanks guys, much appreciated. Hope all is fine and it gets merged in soon.

@EvgenyGusev
Copy link

+1
We really need this in our job

@cayblood
Copy link

@kylewelsby I tried out your patch and am having a weird error where preprocessor.coverage seems to be trying to parse files that have already been converted to js as coffeescript. Here is my karma.conf.js. The errors I'm getting are:

Running "karma:unit" (karma) task
INFO [karma]: Karma v0.10.5 server started at http://localhost:8080/
INFO [launcher]: Starting browser PhantomJS
ERROR [preprocessor.coverage]: Error compiling ./app/scripts/app.coffee: reserved word "var"
  at /Users/carl/Source/sandbox/testmanager_yeoman6/app/scripts/app.coffee
ERROR [preprocessor.coverage]: Error compiling ./app/scripts/app.coffee: reserved word "var"
  at /Users/carl/Source/sandbox/testmanager_yeoman6/app/scripts/app.coffee
INFO [PhantomJS 1.9.2 (Mac OS X)]: Connected on socket Dy18KyMuhedVgr3Yr_N5
PhantomJS 1.9.2 (Mac OS X) ERROR
    Error: [$injector:nomod] Module 'app' is not available! You either misspelled the module name or forgot to load it. If registering a module ensure that you specify the dependencies as the second argument.
    http://errors.angularjs.org/1.2.1/$injector/nomod?p0=app
    at /Users/carl/Source/sandbox/testmanager_yeoman6/app/bower_components/angular/angular.js:1507
PhantomJS 1.9.2 (Mac OS X): Executed 0 of 0 ERROR (0.077 secs / 0 secs)
Warning: Task "karma:unit" failed. Use --force to continue.
Aborted due to warnings.

@kylewelsby
Copy link
Contributor Author

Hi @cayblood yes, @karma-coverage with CoffeeScript support does compile the files to JavaScript, so you'll not need to re-compile. This was not intentional. It's how Ibrik handles the files.

You'll need to make sure coverage files are not passed though the coffee-script preprocessor.

reporters: [
    "progress",
    "coverage"
],
preprocessors: {
    "app/scripts/**/*.coffee": "coverage",
    "tests/spec/**/*.coffee": "coffee"
},
coverageReporter: {
    type: "cobertura",
    dir: "coverage/"
}

As seen in the updated README not sure if the coffee-script preprocessor does or will ever check if the document has already been compiled.

@cayblood
Copy link

@kylewelsby Thanks but did you look at my karma.conf.js file linked above? That is precisely how I've configured it and I'm still getting errors.

@cayblood
Copy link

@kylewelsby This debug output may be of help. It looks like the preprocessor is handling each file twice. I'm still trying to debug it.

@kylewelsby
Copy link
Contributor Author

Your file Globbing is checking for app/scripts/*.coffee more than once.

app/scripts/**/*.coffee includes app/scripts/*.coffee

@cayblood
Copy link

@kylewelsby This patch fixes the duplication in the preprocessor but now it seems that karma is just stuck waiting for my tests to run. The test runs fine if I comment out the preprocessor config lines entirely, but then of course I don't get any coverage.

@cayblood
Copy link

@kylewelsby the globbing actually doesn't overlap. app/scripts/*.coffee matches all .coffee files in app/scripts, whereas app/scripts/**/*.coffee only matches .coffee files in subdirectories within app/scripts. It is necessary to have both filespecs to catch everything. Regardless, I explicitly named the .coffee files within app/scripts just to avoid uncertainty, and I'm still experiencing the problem (it hangs when I enable coverage).

@cayblood
Copy link

@kylewelsby Incidentally, if you want to reproduce this error in its entirety, it should be sufficient to run the following code in an empty directory:

npm install -g yeoman
npm install -g generator-angular
yo angular --coffee

Then add

    "karma-coverage": "git://github.com/kylewelsby/karma-coverage.git#73818fc"

as a devDependency in your package.json file, run npm install to update your local version of karma-coverage and modify the karma.conf.js file to be like mine. Then run grunt test.

@kylewelsby
Copy link
Contributor Author

Hi @cayblood I have not been able to replicate your issue, here is the repository of my workings.

npm install -g yeoman
npm install -g generator-angular
yo angular --coffee

Default settings

add to package.json

"karma-coverage": "git://github.com/kylewelsby/karma-coverage.git#73818fc"

add to karma.conf.js

reporters: ['progress', 'coverage'],
    preprocessors: {
      'app/scripts/**/*.coffee': ['coverage'],
      'test/spec/**/*.coffee': ['coffee']
    },

then run

karma start --single-run --log-level DEBUG

results in DEBUG INFO
screenshot 2013-11-25 15 55 19

@kylewelsby
Copy link
Contributor Author

screenshot 2013-11-25 16 04 32

@cayblood
Copy link

@kylewelsby very sorry but after following the same steps I can't reproduce this bug either. It seems like it may have been related to some strange state in my project but even after diffing everything I can see no apparent differences that would be causing this. Anyhow, I'm very glad to have it working now and thanks for your help in getting to the bottom of this.

@kylewelsby
Copy link
Contributor Author

No problem @cayblood glad I was able to help you.

@kylewelsby
Copy link
Contributor Author

There we go @vojtajina, rebased 👍

"dependencies": {
"istanbul": "~0.1.45",
"ibrik": "~1.0.1",
"ibrik": "git+https://github.com/HBOCodeLabs/ibrik.git#304cd84936d33cf38a05a1a4fe58c82d54ff6cfa",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version of Ibrik uses Standard Coffee-Script Library and not the CoffeeScript Redux version the master Ibrik depends upon. CoffeeScript Redux clashes completely with the rest of the Karma build.

@vojtajina
Copy link
Contributor

Cool, can you also update the commit msg to follow http://karma-runner.github.io/0.10/dev/git-commit-msg.html - something like chore: update readme with a coffee example.

Thanks a bunch.

@kylewelsby
Copy link
Contributor Author

@vojtajina I have edited the commit message. Hope all is well.

🎄

@davinov
Copy link

davinov commented Dec 26, 2013

👍

2 similar comments
@nicbaz
Copy link

nicbaz commented Dec 27, 2013

👍

@emiliomg
Copy link

emiliomg commented Jan 3, 2014

👍

@jmorahan13
Copy link

I'm trying to pull your branch fix-coffee-script-compiler in my package.json file as:

"karma-coverage": "https://github.com/kylewelsby/karma-coverage/tree/fix-coffee-script-compiler"

but I am getting the following error:

GET https://github.com/kylewelsby/karma-coverage/tree/fix-coffee-script-compiler
npm http 200 https://github.com/kylewelsby/karma-coverage/tree/fix-coffee-script-compiler
npm ERR! not a package /var/folders/qs/b66jq4214pvf23sly4bpz7k40000gn/T/npm-10691-EX32d6-2/1389063188860-0.931256205542013/tmp.tgz
npm ERR! Error: ENOENT, open '/var/folders/qs/b66jq4214pvf23sly4bpz7k40000gn/T/npm-10691-EX32d6-2/1389063188860-0.931256205542013/package/package.json'

@jmorahan13
Copy link

Figured out my issue.

Add this line to package.json:

"karma-coverage": "git+https://github.com/kylewelsby/karma-coverage#fix-coffee-script-compiler"

Note, this replaces the line above:
"karma-coverage": "git://github.com/kylewelsby/karma-coverage.git#73818fc"

As the 73818fc tag no longer seems to exist in the repository.

This pull request works great for coffee coverage. I hope to see it pulled into the master karma-coverage soon!

@janraasch
Copy link

👍

@hijonathan
Copy link

Been using this for a few weeks now and love it. 🚢

@kylewelsby
Copy link
Contributor Author

thanks @hijonathan 🍰

@blaiz
Copy link

blaiz commented Jan 23, 2014

👍 I set it up yesterday. Works great on my yeoman-maintained AngularJS project. Thanks.

@DavidSouther
Copy link

+1 Any word on when this will land in master?

@mainyaa
Copy link

mainyaa commented Feb 13, 2014

👍 Its works!

@vojtajina
Copy link
Contributor

@kylewelsby sorry for delays.

I merged your docs/readme update as a35d8d0.

The rest of your change is just changing the dependency to git protocol url, which causes problems (on windows) so I'd rather avoid that. Is there any problem with using the released ibrik 1.1.1?

If so, plese re-open/comment. Otherwise I'm closing this. Thank you very much.

@vojtajina vojtajina closed this Mar 16, 2014
@kylewelsby
Copy link
Contributor Author

hey @vojtajina, sorry to say, as long as Constellation/Ibrik uses CoffeeScriptRedux, this will not work for karma as CoffeeScriptRedux id not fully compatible with standard CoffeeScript syntax.

Constellation/Ibrik is currently using CoffeeScriptRedux 2.0.0-beta8

@swayf
Copy link

swayf commented Apr 5, 2014

any news? :) seems it does not work with current https://github.com/Constellation/ibrik

@kylewelsby
Copy link
Contributor Author

Yeah @swayf as long as Constellation/Ibrik uses CoffeeScriptRedux Karma-Coverage Coffeescript support is not going to work. Thats why this pull request changes the CoffeeScript interpreter from CoffeeScriptRedux to the standard CoffeeScript used by the rest of the Karma modules.

@swayf
Copy link

swayf commented Apr 6, 2014

@kylewelsby Do you mean it should work with HBOCodeLabs/ibrik? hm.. seems I'm doing something wrong

@kylewelsby
Copy link
Contributor Author

Yeah @swayf it should work with HBOCodeLabs/ibrik fork of ibrik.

@kuraga
Copy link

kuraga commented Dec 10, 2014

Any updates?

@DavidSouther
Copy link

There is! Looks like posting here got missed in the excitement, but [email protected] uses standard coffeescript! Constellation/ibrik#16 and the version got bumped in ffa596a, which is in [email protected]

@janraasch
Copy link

That is great.

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

Successfully merging this pull request may close these issues.