Skip to content

Add PHP 8.5 to the test matrix and fix PHP 8.5 deprecations#232

Merged
LukeTowers merged 2 commits into
wintercms:developfrom
austinderrick:ci/php85-tests
Jun 11, 2026
Merged

Add PHP 8.5 to the test matrix and fix PHP 8.5 deprecations#232
LukeTowers merged 2 commits into
wintercms:developfrom
austinderrick:ci/php85-tests

Conversation

@austinderrick

@austinderrick austinderrick commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Adds PHP 8.5 to the unit test matrix on both Ubuntu and Windows, and fixes the PHP 8.5 deprecations in Storm code that the new matrix entry surfaces:

  • src/Network/Http.phpcurl_close() is deprecated since 8.5 (it has been a no-op since PHP 8.0, when curl handles became CurlHandle objects). Replaced with unset() to keep the early-release intent.
  • src/Config/Repository.phpisset($this->afterLoad[$namespace]) is hit with $namespace = null for every non-namespaced config key, and null array offsets are deprecated since 8.5. Added an explicit null check; behaviour is unchanged (the old null→"" coercion could never match a registered callback, since afterLoading() is always called with a string namespace).
  • src/Console/Traits/ProcessesQuery.php — implicitly nullable int $limit = null parameter is deprecated; made it explicitly ?int.

After these fixes, a php -l sweep of src/ on 8.5 reports no compile-time deprecations, and the full test suite emits none at runtime either.

Verified locally on PHP 8.5.5 and 8.4.14 against current develop (identical results on both):

OK, but incomplete, skipped, or risky tests!
Tests: 701, Assertions: 3085, Skipped: 9.

phpcs and phpstan analyse are clean on the changed files.

Note: until orchestral/testbench-core#415 (submitted separately) is merged and tagged, one remaining deprecation notice from the vendored testbench skeleton (PDO::MYSQL_ATTR_SSL_CA) will still appear in 8.5 test output. It is non-fatal and outside Storm's control.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The pull request updates the GitHub Actions test workflow to expand PHP version coverage. The phpVersion matrix in the PHPUnit job configuration now includes PHP 8.5 in addition to previously tested versions up to 8.4. This change ensures that unit tests run against the newer PHP version across all configured operating systems in the test matrix.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title mentions adding PHP 8.5 to the test matrix, which is the main change in the PR. However, the second part claims 'fix PHP 8.5 deprecations' but the PR objectives state no deprecations were found in Storm itself—only in vendored code. Clarify whether the PR actually fixes any deprecations in Storm's code. If no fixes are included, revise the title to 'Add PHP 8.5 to the test matrix' to accurately reflect the actual changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)

42-44: 💤 Low value

Consider updating problem matcher to PHP 8.5.

The PHPUnit problem matcher is currently enabled only for PHP 8.3 (line 43). Now that PHP 8.5 is the latest version in the matrix, you may want to update this condition to matrix.phpVersion == '8.5' for consistency.

This is purely cosmetic—problem matchers provide GitHub UI annotations for test failures but don't affect test execution.

♻️ Proposed update
       - name: Setup problem matchers for PHPUnit
-        if: matrix.phpVersion == '8.3'
+        if: matrix.phpVersion == '8.5'
         run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/tests.yml around lines 42 - 44, Update the PHPUnit problem
matcher step's conditional to target PHP 8.5: locate the "Setup problem matchers
for PHPUnit" job step and change the if condition from matrix.phpVersion ==
'8.3' to matrix.phpVersion == '8.5' so the echo "::add-matcher::${{
runner.tool_cache }}/phpunit.json" runs for the current PHP matrix version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/tests.yml:
- Line 21: The CI matrix adds PHP 8.5 but composer.json pins phpunit/phpunit to
^9.5.8 which lacks official PHP 8.5 support; either remove '8.5' from the
phpVersion matrix in .github/workflows/tests.yml or update composer.json to a
PHPUnit release that supports PHP 8.5 (e.g., bump phpunit/phpunit to a
PHP‑8.5‑compatible major such as ^10.x and run composer update), and also adjust
the workflow's PHPUnit problem-matcher condition (the step that checks
matrix.phpVersion == '8.3') to include '8.5' or make it unconditional so test
annotations are consistent across the matrix.

---

Nitpick comments:
In @.github/workflows/tests.yml:
- Around line 42-44: Update the PHPUnit problem matcher step's conditional to
target PHP 8.5: locate the "Setup problem matchers for PHPUnit" job step and
change the if condition from matrix.phpVersion == '8.3' to matrix.phpVersion ==
'8.5' so the echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" runs
for the current PHP matrix version.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8a592deb-9148-4582-902e-3e341ccc0a2b

📥 Commits

Reviewing files that changed from the base of the PR and between 003e71a and f460e79.

📒 Files selected for processing (1)
  • .github/workflows/tests.yml

Comment thread .github/workflows/tests.yml
@austinderrick austinderrick changed the title Add PHP 8.5 to the test matrix Add PHP 8.5 to the test matrix and fix PHP 8.5 deprecations Jun 11, 2026
@LukeTowers LukeTowers added this to the 1.2.13 milestone Jun 11, 2026
@LukeTowers LukeTowers merged commit 989e5b4 into wintercms:develop Jun 11, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants