-
Notifications
You must be signed in to change notification settings - Fork 57
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 published to codecov.io #61
Conversation
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
=========================================
Coverage ? 57.23%
=========================================
Files ? 362
Lines ? 5745
Branches ? 789
=========================================
Hits ? 3288
Misses ? 2457
Partials ? 0 Continue to review full report at Codecov.
|
|
||
.PHONY: test_unit_codecov | ||
test_unit_codecov: test_unit | ||
npm install codecov -g |
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.
This should just be added to the project dependencies, not installed every time we run make. Also, we use yarn
for dependency management. Using npm
here will probably generate a separate lock file that is incompatible with our yarn lockfile.
.PHONY: test_unit_codecov | ||
test_unit_codecov: test_unit | ||
npm install codecov -g | ||
codecov -f .coverage/coverage-final.json |
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.
This doesn't appear to actually run the unit tests anymore, just generates code coverage. I think we need something like yarn test && yarn run codecov
.
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.
Nvmd, I missed that it has a dependency on the other make target
@@ -20,7 +20,7 @@ | |||
"start": "node -r dotenv/config index.js", | |||
"storybook": "start-storybook -p 9001 -c .storybook", | |||
"tdd": "yarn run test --watch --verbose false", | |||
"test": "NODE_ENV=test jest --config=jest.config.js" | |||
"test": "NODE_ENV=test jest --coverage --config=jest.config.js" |
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.
Generating coverage is a lot slower than just running the tests. I would recommend we create a separate script here just to run code coverage, and not run it every time unit tests are run.
TL;DR
The PR publishes test coverage report to codecov.io. This allows tracking every checkin and the code coverage over time
Type
Are all requirements met?
Complete description
Same as TL;DR
Tracking Issue
flyteorg/flyte#260
Follow-up issue
NA