-
-
Notifications
You must be signed in to change notification settings - Fork 2
[#57] Added testing for lowest deps to CI. #58
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
Conversation
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions Runner
participant Cache as Actions Cache
participant Repo as Source Repository (checkout)
participant PHP as Setup PHP
participant Composer as Composer
participant Lint as Linter
participant PHPUnit as PHPUnit
participant Artifacts as Upload Artifact
participant Codecov as Codecov
Note over GH,Repo: Workflow start (matrix: php x deps)
GH->>Repo: actions/checkout
GH->>PHP: setup-php (matrix.php)
GH->>Cache: restore cache (key includes matrix.deps)
alt deps == "lowest"
GH->>Composer: composer update --prefer-lowest --prefer-stable [maybe --ignore-platform-reqs for PHP 8.5]
else deps == "normal"
GH->>Composer: composer install [maybe --ignore-platform-reqs for PHP 8.5]
end
Composer->>Cache: save cache (key includes matrix.deps)
opt PHP 8.3 and deps == normal
GH->>Lint: composer lint
end
GH->>PHPUnit: run tests & coverage (generates HTML, Cobertura, and text .logs/phpunit/coverage.txt)
PHPUnit->>Artifacts: upload test artifacts (artifact name includes deps)
opt PHP 8.3 and deps == normal and CODECOV_TOKEN set
GH->>Codecov: upload coverage
end
Note over GH,Codecov: Workflow end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Comment |
523214e to
80434d3
Compare
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: 1
🤖 Fix all issues with AI Agents
In @composer.json:
- Line 37: Update the phpunit dependency constraint in composer.json: replace
the tightened version string "phpunit/phpunit": "^11.4" with the broader
constraint "phpunit/phpunit": "^11" so PHPUnit 11.x minor versions (>=11.0) are
allowed; ensure you modify the dependency entry where "phpunit/phpunit" is
declared and run composer update or validate composer.json after the change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test-php.ymlcomposer.jsonphpunit.xml
⏰ 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). (1)
- GitHub Check: PHP 8.3, Deps normal on macos-latest
🔇 Additional comments (8)
phpunit.xml (1)
40-40: LGTM! Text coverage output is a useful addition for CI logs.The text coverage report complements the existing HTML and Cobertura reports nicely, making it easier to view coverage summaries directly in CI logs.
.github/workflows/test-php.yml (7)
21-21: LGTM! Clear job naming.Including the deps dimension in the job name makes it easy to distinguish between normal and lowest dependency test runs in the CI output.
27-27: LGTM! Standard approach for dependency testing.The deps matrix dimension enables testing against both normal and lowest supported dependency versions, which is a best practice for library maintainability.
40-40: Verify cache behavior with lowest deps runs.The cache key now includes
matrix.deps, which is good, but usinghashFiles('**/composer.lock')may not work optimally with--prefer-lowest. When runningcomposer update --prefer-lowest, the composer.lock file will be modified, potentially causing cache invalidation issues or incorrect cache hits between normal and lowest deps runs.Consider whether the current caching strategy provides the expected behavior, or if additional cache isolation is needed.
50-55: LGTM! Correct approach for lowest deps installation.The conditional logic correctly uses
composer update --prefer-lowest --prefer-stablefor lowest deps andcomposer installfor normal deps. The platform requirements flag is appropriately applied for PHP 8.5.
63-63: LGTM! Efficient lint execution.Restricting the coding standards check to PHP 8.3 with normal deps is a sensible optimization, as linting results shouldn't vary based on dependency versions. This reduces redundant CI execution time.
78-78: LGTM! Clear artifact naming.Including the deps dimension in the artifact name ensures coverage reports from different dependency variants are properly distinguished and preserved.
85-94: LGTM! Appropriate Codecov upload strategy.Restricting Codecov uploads to PHP 8.3 with normal deps prevents duplicate coverage reports and focuses on the canonical configuration. The addition of the
CODECOV_TOKENcheck is a good safeguard.Note: This means coverage metrics from lowest deps runs won't be tracked separately, which is typically the desired behavior.
| "friends-of-behat/mink-extension": "^2.7.5", | ||
| "phpstan/phpstan": "^2", | ||
| "phpunit/phpunit": "^11", | ||
| "phpunit/phpunit": "^11.4", |
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.
🧩 Analysis chain
🌐 Web query:
PHPUnit 11.4 text coverage output support changelog release notes
💡 Result:
PHPUnit 11.4 — release notes / changelog and text-coverage output references:
- Release announcement (PHPUnit 11.4) — released October 5, 2024. [1]
- GitHub release notes for 11.4.0 (changes/deprecations list). [2]
- PHPUnit 11.4 manual (Documents the CLI coverage options including --coverage-text, --show-uncovered-for-coverage-text and related flags). [3]
Sources:
[1] PHPUnit announcements — PHPUnit 11.4 (Oct 5, 2024).
[2] sebastianbergmann/phpunit — Release 11.4.0 (GitHub release notes).
[3] PHPUnit 11.4 Manual — Code Coverage / Command-Line options (covers --coverage-text and related flags).
🌐 Web query:
PHPUnit 11.0 11.1 11.2 11.3 text coverage support changelog when introduced
💡 Result:
Summary — when "text" code-coverage support/flags appeared in PHPUnit 11.x:
- PHPUnit 11.0 (release Feb 2, 2024): text coverage support (--coverage-text) is available in the PHPUnit 11 command‑line options. [1][2]
- PHPUnit 11.1 (release Apr 5, 2024): added CLI options specifically for text output: --only-summary-for-coverage-text and --show-uncovered-for-coverage-text. [3][4]
- PHPUnit 11.2 (release Jun 7, 2024): no new "text coverage" feature introduced in the changelog; existing text coverage flags remain. [4]
- PHPUnit 11.3 (release Aug 2, 2024): no new "text coverage" feature introduced in the changelog; existing text coverage flags remain. [4]
Sources
- PHPUnit 11 manual — Command‑Line Test Runner (lists --coverage-text). [PHPUnit docs]. [5]
- PHPUnit 11 release announcement (Feb 2, 2024). [phpunit.de]. [2]
- PHPUnit 11.1 release notes (lists --only-summary-for-coverage-text and --show-uncovered-for-coverage-text). [release notes]. [3]
- PHPUnit announcements index (release dates for 11.1, 11.2, 11.3). [phpunit.de]. [4]
References:
[1] PHPUnit 11 docs — Code Coverage & CLI. https://docs.phpunit.de/en/11.5/code-coverage.html (see CLI options).
[2] PHPUnit 11 announcement — Feb 2, 2024. https://phpunit.de/announcements/phpunit-11.html
[3] PHPUnit 11.1 changelog / release notes (added text-coverage CLI options). (see 11.1 release notes).
[4] PHPUnit announcements index (11.1 / 11.2 / 11.3 release dates). https://phpunit.de/announcements/index.html
[5] PHPUnit CLI reference showing --coverage-text. https://docs.phpunit.de/en/11.5/textui.html
Would you like the exact changelog lines or direct links to the 11.1 release notes?
🏁 Script executed:
# Let me check the phpunit.xml file to see what specific text coverage configuration is being used
find . -name "phpunit.xml" -type f | head -5 && echo "---" && cat phpunit.xml | grep -A 5 -B 5 "coverage" | head -30Repository: drevops/behat-phpserver
Length of output: 703
Relax the PHPUnit version constraint back to ^11.
The constraint was unnecessarily tightened from ^11 to ^11.4. Text coverage output support (via <text outputFile="..."/>) has been available since PHPUnit 11.0 (released February 2024). There is no technical requirement for 11.4+, so broaden the constraint to ^11 to maximize compatibility with earlier minor versions.
🤖 Prompt for AI Agents
In @composer.json at line 37, Update the phpunit dependency constraint in
composer.json: replace the tightened version string "phpunit/phpunit": "^11.4"
with the broader constraint "phpunit/phpunit": "^11" so PHPUnit 11.x minor
versions (>=11.0) are allowed; ensure you modify the dependency entry where
"phpunit/phpunit" is declared and run composer update or validate composer.json
after the change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
=======================================
Coverage 75.34% 75.34%
=======================================
Files 3 3
Lines 430 430
=======================================
Hits 324 324
Misses 106 106 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.