fix: use proper GitHub Actions conditional for ParaTest installation#1061
fix: use proper GitHub Actions conditional for ParaTest installation#1061huangdijia merged 21 commits intomainfrom
Conversation
Use `if: ${{ matrix.php == '8.1' }}` at the step level instead of
using `${{ matrix.php }}` inside the shell script, which doesn't get
properly evaluated.
This fixes the CI errors that occurred when trying to conditionally
install paratest for PHP 8.1.
|
Warning Rate limit exceeded
⌛ 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. Walkthrough在 GitHub Actions 工作流 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟 Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the CI workflow to conditionally install ParaTest using GitHub Actions step-level if expressions, avoiding invalid expression syntax inside shell scripts and fixing PHP 8.1 CI failures.
Changes:
- Add a dedicated “Setup ParaTest for PHP 8.1” step guarded by
if: ${{ matrix.php == '8.1' }}in the workflow jobs. - Keep package installation (
composer update -o) as a separate, unconditional step.
.github/workflows/tests.yaml
Outdated
| tools: phpize | ||
| coverage: none | ||
| - name: Setup ParaTest for PHP 8.1 | ||
| if: ${{ matrix.php == '8.1' }} |
There was a problem hiding this comment.
In the cs-fix job the matrix only includes PHP 8.1 (php: ['8.1']), so this step’s if: ${{ matrix.php == '8.1' }} condition is always true and adds unnecessary noise. Consider removing the if (or expanding the matrix if the condition is meant to support additional PHP versions later).
| if: ${{ matrix.php == '8.1' }} |
.github/workflows/tests.yaml
Outdated
| - name: Setup ParaTest for PHP 8.1 | ||
| if: ${{ matrix.php == '8.1' }} | ||
| run: | | ||
| composer require pestphp/pest:~2.8.0 --dev --no-update | ||
| composer require pestphp/pest-plugin-faker:~2.0 --dev --no-update | ||
| composer require brianium/paratest:~7.3.1 --dev --no-update |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.github/workflows/tests.yaml
Outdated
| composer require pestphp/pest:~2.8.0 --dev --no-update | ||
| composer require pestphp/pest-plugin-faker:~2.0 --dev --no-update |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.github/workflows/tests.yaml
Outdated
| - name: Setup ParaTest for PHP 8.1 | ||
| if: ${{ matrix.php == '8.1' }} | ||
| run: | | ||
| composer require pestphp/pest:^2.35.0 --dev --no-update | ||
| composer require pestphp/pest-plugin-faker:^2.0 --dev --no-update | ||
| composer require brianium/paratest:~7.3.1 --dev --no-update |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.github/workflows/tests.yaml
Outdated
| - name: Setup ParaTest for PHP 8.1 | ||
| if: ${{ matrix.php == '8.1' }} | ||
| run: | | ||
| composer require pestphp/pest:~2.8.0 --dev --no-update | ||
| composer require pestphp/pest-plugin-faker:~2.0 --dev --no-update | ||
| composer require brianium/paratest:~7.3.1 --dev --no-update |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@copilot fix the errors in CI |
|
@huangdijia I've opened a new pull request, #1062, to work on those changes. Once the pull request is ready, I'll request review from you. |
…e specific versions
…dependencies flag
…1061) * fix: use proper GitHub Actions conditional for ParaTest installation Use `if: ${{ matrix.php == '8.1' }}` at the step level instead of using `${{ matrix.php }}` inside the shell script, which doesn't get properly evaluated. This fixes the CI errors that occurred when trying to conditionally install paratest for PHP 8.1. * fix: update ParaTest setup to include Pest and its Faker plugin for PHP 8.1 * fix: add Pest type coverage plugin to ParaTest setup * fix: update Pest and its plugins version in ParaTest setup for PHP 8.1 * fix: update Pest and its Faker plugin versions in ParaTest setup for PHP 8.1 * fix: update Pest and its Faker plugin versions in ParaTest setup for consistency * fix: add ParaTest dependency to setup for consistency * fix: update ParaTest version in workflow setup to 7.3.0 * fix: update ParaTest version in workflow setup to 7.3.0 * fix: update Pest version constraint in ParaTest setup for consistency * fix: add PHPUnit dependency to ParaTest setup for compatibility * fix: comment out ParaTest setup for PHP 8.1 in workflow * fix: update ParaTest setup for PHP 8.1 to require specific Pest versions * fix: update setup for Pest in workflow for PHP 8.1 * fix: update Pest setup in workflow to use composer require for PHP 8.1 * fix: update Pest and ParaTest setup in workflow for PHP 8.1 to require specific versions * fix: update composer update command in workflow to remove --with-all-dependencies flag * fix: add audit configuration to composer.json to block insecure plugins * fix: reorder audit configuration in composer.json for clarity * fix: comment out Pest setup for PHP 8.1 in workflow * fix: 注释掉 PHP 8.1 的 Pest 设置以简化工作流 --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
ifcondition at the step level instead of using expression syntax inside shell scriptsProblem
The original implementation used
${{ matrix.php }}inside the shell script:This caused CI failures because GitHub Actions expressions don't get properly evaluated when embedded in shell scripts.
Solution
Use GitHub Actions'
ifcondition at the step level:This way the condition is evaluated by GitHub Actions before the step executes.
Test plan
Summary by CodeRabbit
发布说明