[#2384] Updated CI config to hide coverage details under collapsible section.#2386
Merged
Conversation
WalkthroughSplits CI coverage output into a concise summary and per-class details, adds a configurable coverage threshold ( Changes
Sequence DiagramsequenceDiagram
participant CI as CI Job
participant Parser as Coverage Parser
participant Formatter as Comment Builder
participant GitHub as GitHub API
CI->>Parser: Produce coverage report (multi-line)
Parser->>Parser: Extract Summary -> set COVERAGE_SUMMARY
Parser->>Parser: Extract per-class details -> set COVERAGE_DETAILS
Formatter->>Formatter: Read VORTEX_CI_CODE_COVERAGE_THRESHOLD
Formatter->>Formatter: Build comment (heading with threshold + COVERAGE_SUMMARY + collapsible COVERAGE_DETAILS)
Formatter->>GitHub: Post PR comment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/post-coverage-comment.sh:
- Around line 36-37: The current awk splits use a strict empty-line match /^$/
which fails if the separator line contains spaces; update both COVERAGE_SUMMARY
and COVERAGE_DETAILS patterns to treat separator lines as whitespace-only by
replacing the /^$/ tests with a whitespace-aware regex such as /^[[:space:]]*$/
(or an equivalent POSIX-safe pattern) so the summary/details extraction logic
still stops/starts correctly even when the separator contains spaces.
In @.github/workflows/build-test-deploy.yml:
- Around line 437-438: The awk commands that set COVERAGE_SUMMARY and
COVERAGE_DETAILS use the blank-line pattern ^$ which fails on lines containing
spaces; update both awk scripts (the first using '/^ *Summary:/{f=1;next} f &&
/^$/{exit} f' and the second using 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 &&
/^$/{s=2;next} s==2') to use a whitespace-tolerant blank-line matcher like
/^[[:space:]]*$/ so separator lines with spaces are recognized and extraction is
robust.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6d3d228-c886-4731-8bd3-65bfe0c1c090
⛔ Files ignored due to path filters (26)
.vortex/installer/tests/Fixtures/handler_process/_baseline/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/ciprovider_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_all_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_gha/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_ci_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/timezone_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpmd_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpstan_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_none/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.circleci/post-coverage-comment.sh.github/workflows/build-test-deploy.yml.vortex/tests/bats/unit/post-coverage-comment.bats
Comment on lines
+36
to
+37
| COVERAGE_SUMMARY=$(awk '/^ *Summary:/{f=1;next} f && /^$/{exit} f' "${COVERAGE_FILE}") | ||
| COVERAGE_DETAILS=$(awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^$/{s=2;next} s==2' "${COVERAGE_FILE}") |
There was a problem hiding this comment.
Whitespace-only separator lines can break summary/details splitting.
On Line 36 and Line 37, ^$ only matches completely empty lines. If the separator line contains spaces, COVERAGE_SUMMARY may absorb details and COVERAGE_DETAILS may be wrong.
💡 Suggested hardening
-COVERAGE_SUMMARY=$(awk '/^ *Summary:/{f=1;next} f && /^$/{exit} f' "${COVERAGE_FILE}")
-COVERAGE_DETAILS=$(awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^$/{s=2;next} s==2' "${COVERAGE_FILE}")
+COVERAGE_SUMMARY=$(awk '/^ *Summary:/{f=1;next} f && /^[[:space:]]*$/{exit} f' "${COVERAGE_FILE}")
+COVERAGE_DETAILS=$(awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^[[:space:]]*$/{s=2;next} s==2' "${COVERAGE_FILE}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| COVERAGE_SUMMARY=$(awk '/^ *Summary:/{f=1;next} f && /^$/{exit} f' "${COVERAGE_FILE}") | |
| COVERAGE_DETAILS=$(awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^$/{s=2;next} s==2' "${COVERAGE_FILE}") | |
| COVERAGE_SUMMARY=$(awk '/^ *Summary:/{f=1;next} f && /^[[:space:]]*$/{exit} f' "${COVERAGE_FILE}") | |
| COVERAGE_DETAILS=$(awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^[[:space:]]*$/{s=2;next} s==2' "${COVERAGE_FILE}") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.circleci/post-coverage-comment.sh around lines 36 - 37, The current awk
splits use a strict empty-line match /^$/ which fails if the separator line
contains spaces; update both COVERAGE_SUMMARY and COVERAGE_DETAILS patterns to
treat separator lines as whitespace-only by replacing the /^$/ tests with a
whitespace-aware regex such as /^[[:space:]]*$/ (or an equivalent POSIX-safe
pattern) so the summary/details extraction logic still stops/starts correctly
even when the separator contains spaces.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2386 +/- ##
==========================================
- Coverage 79.38% 78.83% -0.56%
==========================================
Files 126 119 -7
Lines 6708 6557 -151
Branches 44 0 -44
==========================================
- Hits 5325 5169 -156
- Misses 1383 1388 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
force-pushed
the
feature/2384-split-coverage
branch
from
226dfdf to
b96c75f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/tests/bats/unit/post-coverage-comment.bats (1)
75-80:⚠️ Potential issue | 🟠 MajorAdd explicit assertions for summary/details markdown in the posted comment
Line 75 and Line 103 only assert that
curlwas called, not that the payload includes the new summary + collapsed details format (<details>...). This leaves the main PR objective effectively untested.Based on learnings: In `.vortex/tests/bats/unit` tests using `../_helper.bash (run_steps)`, unprefixed `STEPS` entries assert presence, and `- ` entries assert absence.✅ Suggested test-hardening pattern
declare -a STEPS=( # GET existing comments - return empty array. '@curl * # []' - # POST new comment. - '@curl * # {"id": 1}' + # POST new comment must include summary and collapsed details. + '@curl *Classes: 100.00% (1/1)* # {"id": 1}' + '@curl *Methods: 100.00% (2/2)* # {"id": 1}' + '@curl *Lines: 100.00% (4/4)* # {"id": 1}' + '@curl *<details>* # {"id": 1}' + '@curl *<summary>Per-class coverage</summary>* # {"id": 1}' + '@curl *</details>* # {"id": 1}' )Also applies to: 103-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/tests/bats/unit/post-coverage-comment.bats around lines 75 - 80, The test currently only asserts that curl was called (STEPS entries like '@curl * # []') but doesn't assert the POST payload contains the new summary plus collapsed details HTML; update the STEPS array entries for the POST step(s) (the '@curl * # {"id": 1}' entries) to assert the payload includes the summary text and the '<details>' block (e.g., include patterns matching the markdown/HTML summary and '<details>...<\/details>') using the same STEPS format relied on by run_steps/_helper.bash so the test verifies the exact comment body is posted; apply the same change to the other POST assertion group around lines 103-110 to harden both cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/bats/unit/post-coverage-comment.bats:
- Line 101: Update the coverage fixture so the reported percentage matches the
fraction: replace the string "Lines: 95.00%% (6/8)" in the printf fixture with
"Lines: 75.00%% (6/8)" (the printf line that writes the coverage report used by
the post-coverage-comment test).
---
Outside diff comments:
In @.vortex/tests/bats/unit/post-coverage-comment.bats:
- Around line 75-80: The test currently only asserts that curl was called (STEPS
entries like '@curl * # []') but doesn't assert the POST payload contains the
new summary plus collapsed details HTML; update the STEPS array entries for the
POST step(s) (the '@curl * # {"id": 1}' entries) to assert the payload includes
the summary text and the '<details>' block (e.g., include patterns matching the
markdown/HTML summary and '<details>...<\/details>') using the same STEPS format
relied on by run_steps/_helper.bash so the test verifies the exact comment body
is posted; apply the same change to the other POST assertion group around lines
103-110 to harden both cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63e2c9d6-4182-4783-a936-e736c4b00b5f
⛔ Files ignored due to path filters (26)
.vortex/installer/tests/Fixtures/handler_process/_baseline/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/ciprovider_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_all_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_gha/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_ci_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/timezone_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpmd_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpstan_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_none/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.circleci/post-coverage-comment.sh.github/workflows/build-test-deploy.yml.vortex/tests/bats/unit/post-coverage-comment.bats
|
|
||
| mkdir -p .logs/coverage/phpunit | ||
| printf "Code Coverage Report:\n Lines: 95.00%%\n" >.logs/coverage/phpunit/coverage.txt | ||
| printf "Code Coverage Report:\n 2024-01-01 12:00:00\n\n Summary:\n Classes: 50.00%% (1/2)\n Methods: 66.67%% (2/3)\n Lines: 95.00%% (6/8)\n\nApp\\\\ClassA\n Methods: 100.00%% ( 2/ 2) Lines: 100.00%% ( 4/ 4)\nApp\\\\ClassB\n Methods: 0.00%% ( 0/ 1) Lines: 50.00%% ( 2/ 4)\n" >.logs/coverage/phpunit/coverage.txt |
There was a problem hiding this comment.
Fix inconsistent line coverage fixture value
Line 101 declares Lines: 95.00%% (6/8), but 6/8 equals 75.00%. This inconsistency can hide parser/formatting regressions and makes the test intent ambiguous.
🔧 Suggested correction
- printf "Code Coverage Report:\n 2024-01-01 12:00:00\n\n Summary:\n Classes: 50.00%% (1/2)\n Methods: 66.67%% (2/3)\n Lines: 95.00%% (6/8)\n\nApp\\\\ClassA\n Methods: 100.00%% ( 2/ 2) Lines: 100.00%% ( 4/ 4)\nApp\\\\ClassB\n Methods: 0.00%% ( 0/ 1) Lines: 50.00%% ( 2/ 4)\n" >.logs/coverage/phpunit/coverage.txt
+ printf "Code Coverage Report:\n 2024-01-01 12:00:00\n\n Summary:\n Classes: 50.00%% (1/2)\n Methods: 66.67%% (2/3)\n Lines: 75.00%% (6/8)\n\nApp\\\\ClassA\n Methods: 100.00%% ( 2/ 2) Lines: 100.00%% ( 4/ 4)\nApp\\\\ClassB\n Methods: 0.00%% ( 0/ 1) Lines: 50.00%% ( 2/ 4)\n" >.logs/coverage/phpunit/coverage.txt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| printf "Code Coverage Report:\n 2024-01-01 12:00:00\n\n Summary:\n Classes: 50.00%% (1/2)\n Methods: 66.67%% (2/3)\n Lines: 95.00%% (6/8)\n\nApp\\\\ClassA\n Methods: 100.00%% ( 2/ 2) Lines: 100.00%% ( 4/ 4)\nApp\\\\ClassB\n Methods: 0.00%% ( 0/ 1) Lines: 50.00%% ( 2/ 4)\n" >.logs/coverage/phpunit/coverage.txt | |
| printf "Code Coverage Report:\n 2024-01-01 12:00:00\n\n Summary:\n Classes: 50.00%% (1/2)\n Methods: 66.67%% (2/3)\n Lines: 75.00%% (6/8)\n\nApp\\\\ClassA\n Methods: 100.00%% ( 2/ 2) Lines: 100.00%% ( 4/ 4)\nApp\\\\ClassB\n Methods: 0.00%% ( 0/ 1) Lines: 50.00%% ( 2/ 4)\n" >.logs/coverage/phpunit/coverage.txt |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/bats/unit/post-coverage-comment.bats at line 101, Update the
coverage fixture so the reported percentage matches the fraction: replace the
string "Lines: 95.00%% (6/8)" in the printf fixture with "Lines: 75.00%% (6/8)"
(the printf line that writes the coverage report used by the
post-coverage-comment test).
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2384
Summary by CodeRabbit