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

Add code coverage output generation to npm test #610

Merged
merged 3 commits into from
Sep 20, 2017

Conversation

dopry
Copy link
Contributor

@dopry dopry commented Sep 16, 2017

- Summary

This is a first step towards adding support for a 3rd party code coverage service. It adds code coverage output to the npm test command.

- Test plan
on your command line run

npm test

verify you see the code coverage output as a result of the test.
verify the coverage folder is generated with the lconv output.
verify the coverage folder is ignored by git.

- Description for the changelog

Add code coverage to npm test

- A picture of a cute animal (not mandatory but encouraged)

1bc455ab81c687860e175b5a8a91a45e

@dopry dopry force-pushed the 609_jest_code_coverage branch 3 times, most recently from 44fa8a9 to cc0092a Compare September 16, 2017 01:22
@dopry
Copy link
Contributor Author

dopry commented Sep 16, 2017

applies to #609

@dopry dopry force-pushed the 609_jest_code_coverage branch 2 times, most recently from aefdb41 to 606b23a Compare September 18, 2017 22:46
@@ -97,7 +97,9 @@ export default function remarkPaddedLinks() {
* nesting. If `end` is truthy, get the last node, otherwise first.
*/
function getEdgeTextChild(node, end) {
const findFn = end ? findLast : find;
let findFn;
Copy link
Contributor

Choose a reason for hiding this comment

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

This really bothers me. I commented here to see if we can get some clarity: istanbuljs/babel-plugin-istanbul#95.

Copy link
Contributor

Choose a reason for hiding this comment

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

For now just add a comment explaining why it's written this way.

…on assignment.

.\node_modules\.bin\jest --coverage --coverageReporters lcov

was triggering t the error

\src\components\Widgets\Markdown\serializers\remarkPaddedLinks.js:100
        var findFn = /* istanbul ignore next */(cov_1gwlaxh2sq.s[21]++, end ? /* istanbul ignore next */() : /* istanbul ignore next */());
                                                                                                         ^
        SyntaxError: Unexpected token )
@dopry
Copy link
Contributor Author

dopry commented Sep 19, 2017

commented and rebased.

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

LGTM

@erquhart
Copy link
Contributor

@Benaiah @tech4him1 feel free to merge when one of you approves.

Copy link
Contributor

@Benaiah Benaiah left a comment

Choose a reason for hiding this comment

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

LGTM. I'm fine with the one code change to get Istanbul working, but if we run into it repeatedly I'd rather wait on/figure out an Istanbul fix or workaround instead of continuing to change code to get coverage working.

@Benaiah Benaiah merged commit 1f06885 into decaporg:master Sep 20, 2017
@dopry dopry deleted the 609_jest_code_coverage branch September 21, 2017 18:11
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.

3 participants