-
Notifications
You must be signed in to change notification settings - Fork 23
Conversation
|
Any feeling about this pull request ? |
|
+1 There may be some opportunities to break up larger parts of code to make something more testable and add tests. |
| phantom.exit(0) | ||
|
|
||
|
|
||
| checkingStatus = setInterval -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite a lot of cyclomatic complexity here.
|
@serut, once we finish with windows support, we'll take a look at this. |
|
I've sometimes dependency injection issues when coverage is active that fails the test when phantomjs opens the page :
https://travis-ci.org/serut/meteor-coverage-app-exemple/builds/144737808#L4119
https://travis-ci.org/serut/meteor-coverage-app-exemple/builds/144737808#L6036 Same issue on circle.ci : https://circleci.com/gh/serut/meteor-coverage-app-exemple/30 These errors happens only when the code is intrumented, so I will try to ignore all mocha stuff from instrumentation to see if these errors disappear...! Also I have another commit that improve functionnalities of spacejam when coverage is launched on that branch : https://github.com/serut/spacejam/tree/windows-suppport-rc4 |
…e|out_lcovonly" + fix returning code when there is an error on the browser side
|
This commit adds the ability to define what the test runner will do during its test. So you can do: I propose to fork https://github.com/serut/meteor-coverage-app-exemple in the To conclude :
|
|
I've been using the coverage fork for over a month now. It's working great. What must happen to get this merged? |
|
Thanks for testing @vangorra
|
I worked a lot on spacejam to implement the coverage support using the lmieulet:meteor-coverage package.
The result is not perfect, but it's working at least !
Here is a repository that uses my own fork of spacejam (test, test --full-app, test-package using mocha and test-package using tinytest):
https://github.com/serut/meteor-coverage-app-exemple
Missing feature:
Things missing to this pull request: