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

Track and report test coverage #1783

Merged
merged 10 commits into from
Jul 8, 2017
Merged

Track and report test coverage #1783

merged 10 commits into from
Jul 8, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jul 7, 2017

Use Jest code coverage added in #1382 (with some tweaks/improvements in this PR to ignore built files, Storybook stories, etc.) and coveralls.io and node-coveralls for reporting coverage during Travis CI builds.

@nylen nylen added [Type] Build Tooling Issues or PRs related to build tooling [Status] In Progress Tracking issues with work in progress labels Jul 7, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6cda42908d786e2cc5a4ce7969651c9cb837b2d6 on add/coverage-badge into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f765a03c7dbeca0109760d8128ebfe3752579a40 on add/coverage-badge into ** on master**.

@nylen nylen removed the [Status] In Progress Tracking issues with work in progress label Jul 7, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9453c971dcdcceb24c58328d812a76ead9de6e38 on add/coverage-badge into ** on master**.

.gitignore Outdated
.DS_Store
.vscode
Copy link
Member

Choose a reason for hiding this comment

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

I see .vscode was already here and this is just moving its location, though should it really be here in the first place?

Via #1382 (comment) from a couple of days ago:

IDE specific entries in .gitignore should not be in the repo, users should manage these locally in ~/.gitignore_global

See also https://core.trac.wordpress.org/ticket/34345

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Originally added in #294.

@ntwb
Copy link
Member

ntwb commented Jul 8, 2017

Coveralls comments and notifications can get noisy, I think turning off comments and changing the failure threshold to 5% would be something we should do:

Via https://coveralls.io/github/WordPress/gutenberg/settings
• Uncheck the "LEAVE COMMENTS?" checkbox
• Change the "COVERAGE DECREASE THRESHOLD FOR FAILURE" value to 5%

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nylen
Copy link
Member Author

nylen commented Jul 8, 2017

Coveralls comments and notifications can get noisy, I think turning off comments and changing the failure threshold to 5% would be something we should do

Maybe so. However, coverage is at 18%. Eighteen percent. This shows that we are not developing with testing in mind.

So I'd like to leave this enabled for a while.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3a1cc23 on add/coverage-badge into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3a1cc23 on add/coverage-badge into ** on master**.

@nylen nylen changed the title Add test coverage badge to readme Track and report test coverage Jul 8, 2017
@nylen nylen merged commit 8284fa6 into master Jul 8, 2017
@nylen nylen deleted the add/coverage-badge branch July 8, 2017 10:57
@aduth
Copy link
Member

aduth commented Jul 12, 2017

With these changes, local testing and Travis testing don't produce the same result. You can see that running npm test locally fails on the master branch with an error "Your test suite must contain at least one test" (because it's trying to run e.g. test/setup-globals.js as a test, fix pending). I see that npm run test-unit:coverage-ci runs the test, but does not fail with the same error.

@aduth
Copy link
Member

aduth commented Jul 12, 2017

Hmm, actually, npm run test-unit:coverage-ci does fail with the same error for me. Not sure why yet...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants