-
-
Notifications
You must be signed in to change notification settings - Fork 864
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
[NoMongo] Fix : CodeCov GitHub Action #3271
Conversation
Warning Rate limit exceeded@varshith257 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request modifies the GitHub Actions workflow configuration in Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
254-275
: Good architectural decision to upload coverage reports separately.The approach of uploading Jest and Vitest coverage reports separately is better than merging them because:
- It provides granular coverage data for each testing framework
- It avoids potential format compatibility issues
- It simplifies debugging coverage issues
🧰 Tools
🪛 actionlint (1.7.4)
256-256: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
271-271: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 275-275: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
.github/workflows/pull-request.yml (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
256-256: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
271-271: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 275-275: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test Application
- GitHub Check: Check Python Code Style
- GitHub Check: Analyse Code With CodeQL (javascript)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3271 +/- ##
=====================================================
+ Coverage 8.49% 90.24% +81.75%
=====================================================
Files 309 330 +21
Lines 7832 8492 +660
Branches 1730 1875 +145
=====================================================
+ Hits 665 7664 +6999
+ Misses 7093 598 -6495
- Partials 74 230 +156
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
254-276
: Ensure consistent configuration between Jest and Vitest coverage uploads.The coverage upload steps have inconsistencies in their configuration:
- Different versions of codecov-action (v3 vs v4)
- Inconsistent token usage (missing vs present)
- Reversed conditions for execution
Consider standardizing the configuration between the two steps to ensure consistent behavior and maintainability. The steps should:
- Use the same version of codecov-action (v4)
- Both include authentication tokens
- Have logically correct and consistent conditions
🧰 Tools
🪛 actionlint (1.7.4)
256-256: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
.github/workflows/pull-request.yml (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
256-256: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Check Python Code Style
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
269-276
:⚠️ Potential issueFix condition logic for Vitest coverage upload.
The condition for Vitest coverage upload seems reversed. It should upload when no files have changed.
Apply this diff to fix the condition:
- name: Upload Vitest Coverage to Codecov - if: steps.changed-files.outputs.any_changed == 'true' + if: steps.changed-files.outputs.only_changed != 'true' uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} files: ./coverage/vitest/lcov.info flags: vitest fail_ci_if_error: trueLikely invalid or redundant comment.
254-261
:⚠️ Potential issueUpdate codecov-action version and fix condition logic for Jest coverage upload.
The Jest coverage upload step needs the following improvements:
- Update to the latest version of codecov-action
- Add token for authentication
- Fix the condition logic (should upload when TypeScript files change)
Apply this diff to fix the issues:
- name: Upload Jest Coverage to Codecov - if: steps.changed-files.outputs.only_changed != 'true' + if: steps.changed-files.outputs.any_changed == 'true' - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: + token: ${{ secrets.CODECOV_TOKEN }} files: ./coverage/jest/lcov.info flags: jest fail_ci_if_error: trueLikely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
256-256: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
283-283
: Fix trailing spaces.Remove trailing spaces at the end of line 283.
- +🧰 Tools
🪛 yamllint (1.35.1)
[error] 283-283: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
.github/workflows/pull-request.yml (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 283-283: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test Application
- GitHub Check: Check Python Code Style
- GitHub Check: Analyse Code With CodeQL (javascript)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
248-261
: LGTM! Jest coverage configuration looks good.The changes improve the test execution condition and add proper coverage upload configuration. The use of Jest-specific flags will help in distinguishing coverage metrics.
264-278
: LGTM! Vitest coverage configuration looks good.The changes maintain consistency with Jest configuration while properly segregating Vitest coverage metrics.
279-283
: LGTM! Coverage merging configuration looks good.The use of
lcov-result-merger
is the correct approach for combining coverage reports from different test frameworks, as it properly handles the lcov format from both Jest and Vitest.🧰 Tools
🪛 yamllint (1.35.1)
[error] 283-283: trailing spaces
(trailing-spaces)
284-290
: LGTM! Combined coverage upload configuration looks good.The configuration properly identifies the combined report using the 'combined' flag and maintains consistent error handling.
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
248-261
: Improve Jest test configuration and coverage upload.The Jest test configuration and coverage upload look good, but there are a few improvements to consider:
- The condition
steps.changed-files.outputs.any_changed == 'true'
is correct, as we want to run tests when files have changed.- The coverage upload step correctly uses codecov-action@v4 with the required token.
Consider adding
working-directory
to the codecov action to ensure it runs from the correct directory:- name: Upload Jest Coverage to Codecov if: steps.changed-files.outputs.any_changed == 'true' uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} files: ./coverage/jest/lcov.info flags: jest + working-directory: . fail_ci_if_error: true
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(2 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
.github/workflows/pull-request.yml (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-admin#2457
File: .github/workflows/pull-request.yml:0-0
Timestamp: 2024-11-26T07:19:31.226Z
Learning: When merging coverage reports from Jest and Vitest, ensure both testing frameworks output coverage in the same format (e.g., 'lcov') and use a compatible tool like 'lcov-result-merger' to combine the reports, as 'nyc merge' may not handle the different coverage formats correctly.
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
277-277: input "gcov_ignore" is not defined in action "codecov/codecov-action@v4". available inputs are "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "file", "files", "flags", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugin", "plugins", "report_code", "root_dir", "slug", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "verbose", "version", "working-directory"
(action)
291-291: input "gcov_ignore" is not defined in action "codecov/codecov-action@v4". available inputs are "codecov_yml_path", "commit_parent", "directory", "disable_file_fixes", "disable_safe_directory", "disable_search", "dry_run", "env_vars", "exclude", "fail_ci_if_error", "file", "files", "flags", "git_service", "handle_no_reports_found", "job_code", "name", "network_filter", "network_prefix", "os", "override_branch", "override_build", "override_build_url", "override_commit", "override_pr", "plugin", "plugins", "report_code", "root_dir", "slug", "token", "url", "use_legacy_upload_endpoint", "use_oidc", "verbose", "version", "working-directory"
(action)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 284-284: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test Application
- GitHub Check: Check Python Code Style
- GitHub Check: Analyse Code With CodeQL (javascript)
- GitHub Check: Analyse Code With CodeQL (javascript)
Is this ready to merge? It looks like it's reporting more reasonable values. There is a similar new issue in the API that you could look at. |
Yes |
What kind of change does this PR introduce?
Issue Number:
Fixes #3269
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Summary by CodeRabbit