Skip to content

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Sep 16, 2025

PHPC-2617

Adds testing and support for PHP 8.5, backporting the fix for IS_INTERNED from #1849.

@alcaeus alcaeus force-pushed the phpc-2617-test-php-8.5 branch from ab72120 to 5583766 Compare September 24, 2025 14:40
@alcaeus alcaeus changed the base branch from v2.x to v2.1 September 24, 2025 14:40
@alcaeus alcaeus marked this pull request as ready for review September 24, 2025 14:45
@alcaeus alcaeus requested a review from a team as a code owner September 24, 2025 14:45
@alcaeus alcaeus requested review from jmikola and Copilot and removed request for a team September 24, 2025 14:45
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds PHP 8.5 support to GitHub Actions test suite while maintaining compatibility with existing PHP versions. The changes update test files to work with PHP 8.5's stricter type handling and deprecated API changes.

  • Adds PHP 8.5 to test matrices in GitHub Actions workflows
  • Updates test files to use string literals instead of numeric constants for Decimal128 tests
  • Removes deprecated setAccessible(true) calls from reflection-based tests
  • Updates internal API calls to use current PHP 8.5 compatible functions

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/tests.yml Adds PHP 8.5 to test matrix
.github/workflows/package-release.yml Adds PHP 8.5 to Windows packaging matrix
.github/actions/windows/prepare-build/action.yml Updates php/setup-php-sdk to v0.11 for PHP 8.5 support
src/contrib/php_array_api.h Updates deprecated IS_INTERNED macro to ZSTR_IS_INTERNED
tests/server/server-construct-001.phpt Changes (integer) cast to (int)
Multiple exception test files Removes deprecated setAccessible(true) calls
Multiple BSON Decimal128 test files Converts numeric literals to strings for constructor compatibility

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@alcaeus alcaeus changed the title PHPC-2617: Run Github Actions tests on PHP 8.5 PHPC-2617: Support PHP 8.5 Sep 24, 2025
- name: Setup PHP SDK
id: setup-php
uses: php/setup-php-sdk@v0.10
uses: php/setup-php-sdk@v0.11
Copy link
Member

Choose a reason for hiding this comment

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

Noted that v0.11 added support for PHP 8.5.


$tests = [
acos(8),
NAN,
Copy link
Member

Choose a reason for hiding this comment

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

Per our earlier Slack conversation, you opted to remove any Decimal128 test cases that depended on type-coercion. I have no objections here, as the documentation has always indicated that the constructor expects a string value.

I wasn't able to find any mention of the type-coercion behavioral change in PHP 8.5's UPGRADING file. If you happen to find the change responsible for this I'd be interested in seeing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The offending commit is php/php-src@320fe29, which is the result of the large PHP 8.5 deprecation vote.


$expectedHost = $parsed['host'];
$expectedPort = (integer) (isset($parsed['port']) ? $parsed['port'] : 27017);
$expectedPort = (int) (isset($parsed['port']) ? $parsed['port'] : 27017);
Copy link
Member

Choose a reason for hiding this comment

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

@alcaeus alcaeus merged commit 986ce2c into mongodb:v2.1 Sep 25, 2025
70 checks passed
@alcaeus alcaeus deleted the phpc-2617-test-php-8.5 branch September 25, 2025 06:39
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.

3 participants