-
-
Notifications
You must be signed in to change notification settings - Fork 596
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
feat: add codecov to the project #3168
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on undefined |
Is there a way we can see the codecov coverage report before merging this PR? |
I tried testing it by opening a PR in my forked repo, but it didn't work, so I guess not 😅. However, based on this comment, I think we're good to go. |
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.
@vishvamsinh28 Add following things in the PR:
- Add proper PR description on why codecov is being added and how it will be used.
- Add test runs for this branch in your forked PR and add the test workflow links in PR description.
Added links to my PRs in forked repo |
@vishvamsinh28 can you please provide the PR link where we can have a look on the working of this feature |
@vishvamsinh28 Let's add that in this PR only. Kindly make changes. |
@anshgoyalevil sure 👍 |
@anshgoyalevil I tried different configurations, but I'm not getting the desired results. I believe the setup we have in this PR provides what we need. We should merge it and test with and without the flag. |
@vishvamsinh28 Kindly resolve the conflicts in this PR. |
@akshatnema Conflicts Resolved. |
/update |
@@ -1,7 +1,8 @@ | |||
module.exports = { | |||
verbose: true, | |||
collectCoverage: true, | |||
collectCoverageFrom: ['scripts/**/*.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.
Why did you remove this?
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.
We don't need that jest will only include files that are directly imported in our test files for coverage.
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.
That's the case. Jest is now taking fixture
files also into coverage, and that's wrong.
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.
I will update it for excluding fixture files
This PR integrates Codecov into our CI workflow to enhance the visibility of our test coverage across the codebase. By adding Codecov, we aim to:
Codecov will provide detailed reports on which lines of code are being covered by our tests and highlight any gaps in coverage. This helps identify areas that require additional testing.
With Codecov, every PR will now include a comment indicating the impact on test coverage. This ensures that as new features or bug fixes are added, we can maintain or improve the test coverage, ensuring code quality remains high.
How Codecov Will Be Used:
Codecov is integrated with the CI workflow. Once tests are run, the coverage reports will be uploaded to Codecov.
Codecov setup in forked repe :- vishvamsinh28#7
Codecov report for tests in forked repo :- vishvamsinh28#8