Skip to content

Conversation

swissspidy
Copy link
Member

@swissspidy swissspidy commented Sep 11, 2025

Trac ticket: https://core.trac.wordpress.org/ticket/63061


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@swissspidy swissspidy marked this pull request as ready for review September 13, 2025 13:42
Copy link

github-actions bot commented Sep 13, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props swissspidy, desrosj.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@desrosj desrosj left a comment

Choose a reason for hiding this comment

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

Unless the tests will pass, would it be better to split this out into a separate job for now? I was thinking something like this:

  php-8-5-testing:
    name: PHP ${{ matrix.php }}
    uses: ./.github/workflows/reusable-phpunit-tests-v3.yml
    permissions:
      contents: read
    secrets: inherit
    if: ${{ github.repository == 'WordPress/wordpress-develop' }}
    continue-on-error: ${{ true }}
    strategy:
      fail-fast: false
      matrix:
        php: [ '8.5' ]
        db-version: [ '8.4', '11.8' ]
        db-type: [ 'mysql', 'mariadb' ]
        tests-domain: [ 'example.org' ]
        multisite: [ false, true ]


        exclude:
          # Exclude PHP versions that are not supported by the database versions.
          - db-type: 'mysql'
            db-version: '11.8'
          - db-type: 'mariadb'
            db-version: '8.4'

    with:
      php: ${{ matrix.php }}
      db-version: ${{ matrix.db-version }}
      db-type: ${{ matrix.db-type }}
      multisite: ${{ matrix.multisite }}
      phpunit-config: ${{ matrix.multisite && 'tests/phpunit/multisite.xml' || 'phpunit.xml.dist' }}
      tests-domain: ${{ matrix.tests-domain }}

Note the continue-on-error there, which allows this to be committed without failing the build. This also prevents any 8.5 jobs from running on forks until they are passing (they'll still run on pull requests back to this repo, though).

@swissspidy
Copy link
Member Author

Not sure the effort is worth it. We're very close to getting them to pass now. #9849 resolves all but a couple issues. (reviews welcome)

The remaining ones are in the ID3 library, which is already fixed but lacking a release. I doubt we'll see a release there soon though, so we can just patch ot ourselves.

Then we'd have green tests in time for PHP 8.5 beta :)

@desrosj
Copy link
Member

desrosj commented Sep 17, 2025

Not sure the effort is worth it. We're very close to getting them to pass now. #9849 resolves all but a couple issues. (reviews welcome)

Ah! That's great. I hadn't gotten to check that one out or look deeply into what the failures were. Agreed that waiting a few weeks makes sense.

The remaining ones are in the ID3 library, which is already fixed but lacking a release. I doubt we'll see a release there soon though, so we can just patch ot ourselves.

👍 from me for this. There's previous precedent in r46113 prior to the full release in r46166, and r49621 and r50714 before the full release in r51900.

Then we'd have green tests in time for PHP 8.5 beta :)

🎉

@swissspidy
Copy link
Member Author

swissspidy commented Sep 29, 2025

This is very close. A new issue popped up in SimplePie (simplepie/simplepie#949) but otherwise tests all pass without failures and errors. The __wakeup one might become a soft deprecation

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