ci: rework coveralls upload for unit tests with parallel+finish events; add config for coverage for integration tests; unify coverage paths & remove now obsolete clover script #420
Conversation
…s; add config for coverage for integration tests; unify coverage paths & remove now obsolete clover script
WalkthroughCI pipeline changed to run unit tests without the Clover flag, upload per-node coverage files in parallel to Coveralls, and add a finalization job; PHPUnit configs add integration coverage and relocate unit coverage outputs; Composer script that emitted Clover was removed. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Node as tests (matrix node)
participant Cover as Coveralls
participant Finish as coveralls-finish job
GH->>Node: Run `composer run-unit-tests`
Node->>Node: Generate clover at ./var/logs/test-coverage/unit/clover.xml
Node->>Cover: Upload clover file (parallel: true, flag-name: "Unit")
GH->>Finish: needs: tests (if: always())
Finish->>Cover: Finalize parallel run (parallel-finished: true)
Cover-->>GH: Aggregated coverage report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
123-126: Coveralls: parallel upload config LGTM; consider explicit format and path mapping (optional)
- Using file: ./var/logs/test-coverage/unit/clover.xml with parallel: true and a stable flag-name is correct for a one-node coverage upload finalized later.
- Optional hardening:
- Explicitly set format: clover (the action typically auto-detects, but this removes ambiguity).
- If Coveralls shows “Unknown file” mappings, consider setting base-path to github.workspace.
Apply this minimal diff if you prefer being explicit:
with: github-token: ${{ secrets.GITHUB_TOKEN }} file: ./var/logs/test-coverage/unit/clover.xml + format: clover parallel: true flag-name: "unit" fail-on-error: false
128-137: Finalize Coveralls parallel run — good patternAdding a dedicated coveralls-finish job with needs: tests and if: always() correctly finalizes even if the tests job fails. This matches Coveralls’ recommended parallel flow.
If you want to avoid running finalize on workflows where tests are skipped (e.g., release-please), you can make the condition explicit:
- if: always() + if: ${{ always() && needs.tests.result != 'skipped' }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)ci/phpunit/config-integration.xml(1 hunks)ci/phpunit/config-unit.xml(1 hunks)composer.json(0 hunks)
💤 Files with no reviewable changes (1)
- composer.json
🔇 Additional comments (3)
ci/phpunit/config-unit.xml (1)
24-25: Coverage output path update verifiedThe composer.json scripts for unit testing correctly reference
./ci/phpunit/config-unit.xml, and there are no remaining references to the oldvar/coveragepaths. Everything is aligned—approving these changes.ci/phpunit/config-integration.xml (1)
22-27: Integration coverage configuration verified and approvedNice addition: dedicated integration coverage reports under
var/logs/test-coverage/integration.• composer.json’s
run-integration-testsscript invokesphpunit:integration
•phpunit:integrationcorrectly uses--configuration=./ci/phpunit/config-integration.xml
• The bootstrap filetests/Integration/bootstrap.phpexists, ensuring reports will be generatedIf/when you decide to publish integration coverage to Coveralls, you can mirror the unit upload with a separate flag (e.g.,
"integration") and gate it with a matrix condition—let me know if you’d like help drafting those CI steps..github/workflows/ci.yml (1)
116-116: Switch to composer run-unit-tests aligns with new phpunit configThis keeps the workflow simpler and relies on the config to emit Clover to the unified path.
Please confirm that run-unit-tests runs phpunit with the ci/phpunit/config-unit.xml so the Clover lands at ./var/logs/test-coverage/unit/clover.xml (see the verification script attached in the config-unit.xml comment).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.scrutinizer.yml (1)
100-104: Consider removing external_code_coverage to avoid confusion with local Clover ingestionSince you’re providing a local Clover file via tests.override.coverage, the external_code_coverage tool is likely redundant and can cause ambiguity (Scrutinizer may wait for an upload that never comes). If you’re not uploading coverage externally to Scrutinizer, drop this block.
-tools: - external_code_coverage: - timeout: 600 - runs: 1 + +# tools: +# external_code_coverage: +# timeout: 600 +# runs: 1If you do rely on external uploads, keep this but ensure your CI actually posts to Scrutinizer; otherwise Scrutinizer will time out waiting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.scrutinizer.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: sloth
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
🔇 Additional comments (1)
.scrutinizer.yml (1)
11-14: Coverage Path and Script Verification CompleteConfirmed that:
composer run-unit-testsinvokes thephpunit:unitscript (@phpunit --configuration=./ci/phpunit/config-unit.xml).config-unit.xmlwrites the Clover report tovar/logs/test-coverage/unit/clover.xml.- The
phpunitscript is prefixed withXDEBUG_MODE=coverage, ensuring a coverage driver is active.Everything aligns with the unified coverage paths. ✅
Summary by CodeRabbit
Tests
Chores