run tests using PHPUnit 11.5 #58370

xabbuh · 2024-09-24T07:20:33Z

Q	A
Branch?	7.3
Bug fix?	no
New feature?	no
Deprecations?	no
Issues
License	MIT

xabbuh · 2024-09-24T07:21:17Z

The PR cannot be merged at the moment as it is based on a bunch of other open pull requests that are paving the way to be able to actually run the tests with PHPUnit 11.

alexandre-daubois · 2024-09-24T07:47:03Z

This is super nice news! Did you already take care of everything required to be "PHPUnit 11 compliant" in already opened PRs ?

xabbuh · 2024-09-24T07:49:24Z

@alexandre-daubois The "ensure session storages are opened before destroying them" commit is the only one that is not already part of an open PR. I do not really understand yet why that change is necessary (the flow in AbstractSessionHandler is different as headers_sent() returns a different result).

This PR was merged into the 5.4 branch. Discussion ---------- Tweak error/exception handler registration | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #53812 | License | MIT This should allow removing the custom bootstrap file from #58370 /cc `@xabbuh` The change on FrameworkBundle leads to a tweaked behavior: we don't override the previous error handler in case it's not the Symfony one. This shouldn't change anything in practice since the error handler is already registered by the runtime component. The rest is closer to bug fixes. Commits ------- af9c035 Tweak error/exception handler registration

nicolas-grekas · 2024-09-25T14:30:35Z

We need a way to register DebugClassLoader, and maybe a few other things. Basically what we do in SymfonyTestsListenerTrait.

xabbuh · 2025-04-21T04:59:23Z

@symfony/mergers The PR is finally ready to be reviewed.

Status: Needs Review

.github/workflows/unit-tests.yml

stof · 2025-04-22T07:05:09Z

.github/workflows/windows.yml

+
+    env:
+      COMPOSER_NO_INTERACTION: '1'
+      SYMFONY_DEPRECATIONS_HELPER: 'strict'


this env var should not be necessary anymore, as it won't be used with PHPUnit 11

stof · 2025-04-22T07:06:19Z

.github/workflows/unit-tests.yml

@@ -76,7 +76,7 @@ jobs:
          ([ -d "$COMPOSER_HOME" ] || mkdir "$COMPOSER_HOME") && cp .github/composer-config.json "$COMPOSER_HOME/config.json"

          echo COLUMNS=120 >> $GITHUB_ENV
-          echo PHPUNIT="$(pwd)/phpunit --exclude-group tty,benchmark,intl-data,integration" >> $GITHUB_ENV
+          echo PHPUNIT="$(pwd)/phpunit --exclude-group tty --exclude-group benchmark --exclude-group intl-data --exclude-group integration --fail-on-deprecation" >> $GITHUB_ENV


I suggest configuring --fail-on-deprecation through the config file instead, so that it is also failing when running locally.

stof · 2025-04-22T07:07:07Z

.github/workflows/unit-tests.yml

@@ -140,7 +140,7 @@ jobs:
          [[ "${{ matrix.mode }}" = *-deps ]] && mv composer.json.phpunit composer.json || true

          if [[ "${{ matrix.mode }}" = low-deps ]]; then
-            echo SYMFONY_PHPUNIT_REQUIRE="nikic/php-parser:^4.18" >> $GITHUB_ENV
+            echo SYMFONY_PHPUNIT_REQUIRE="nikic/php-parser:^5.0" >> $GITHUB_ENV


Do we still have a job testing our compat with nikic/php-parser v4 for our code using it ?

I am afraid we cannot: #58370 (comment):

PHPUnit 11 (through its dependency on phpunit/php-code-coverage is not compatible with nikic/php-parser 4

Only Translation supports v4, I would bump it to ^5 on 7.3

stof · 2025-04-22T07:11:52Z

src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php

@@ -251,9 +250,7 @@ public static function idsProvider(): iterable
        yield ['foo'];
    }

-    /**
-     * @group legacy


should we keep a legacy group for our tests covering deprecated behaviors ? Our deps=high job is excluding this group when running tests for a version that is not part of the last major version (as the dependencies could be installed at the next major version already):

symfony/.github/workflows/unit-tests.yml

Lines 134 to 135 in fe94cfb

# Legacy tests are skipped when deps=high and when the current branch version has not the same major version number as the next one

[[ "${{ matrix.mode }}" = high-deps && $SYMFONY_VERSION = *.4 ]] && echo LEGACY=,legacy >> $GITHUB_ENV || true

thinking about this again, I think this option was overkill and was leading to not testing some parts of the code in CI in our LTS branches. Many legacy tests we have are about testing deprecations of their own component, which does not need such exclusion. Tests relying on cross-package deprecations could be responsible for skipping themselves when needed.

stof · 2025-04-22T07:14:07Z

src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php

-    /**
-     * @group legacy
-     */
+    #[IgnoreDeprecations]


Unrelated to this PR, but we should update those tests. I'm quite sure most of them are not legacy ones that are supposed to be removed in 8.0 (but they have not been updated to use named arguments for the constraint constructor)

I'm quite sure that they can indeed be removed. Before #54744 we had data providers configuring the constraints with both ways (named arguments and associative options array) to configure them, but I split those tests into two when deprecating the non named arguments way.

stof · 2025-04-22T07:14:52Z

src/Symfony/Bridge/Doctrine/phpunit.xml.dist

@@ -18,7 +17,7 @@
        </testsuite>
    </testsuites>

-    <coverage>
+    <source>


We need the ignoreSuppressionOfDeprecations="true" parameter in the component-level config files as well.

stof · 2025-04-22T07:15:35Z

src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/ConfigurationTest.php

@@ -17,6 +17,9 @@
 use Symfony\Bridge\PhpUnit\DeprecationErrorHandler\DeprecationGroup;
 use Symfony\Component\ErrorHandler\DebugClassLoader;

+/**
+ * @requires PHPUnit < 10


shouldn't this have the attribute as well, so that PHPUnit 11 excludes it ?

in fact, I even think that having the attribute without the annotation should be sufficient

stof · 2025-04-22T07:16:56Z

src/Symfony/Bridge/Twig/Tests/Validator/Constraints/TwigValidatorTest.php

@@ -67,9 +68,8 @@ public function testInvalidValues($value, $message, $line)

    /**
     * When deprecations are skipped by the validator, the testsuite reporter will catch them so we need to mark the test as legacy.


Suggested change

* When deprecations are skipped by the validator, the testsuite reporter will catch them so we need to mark the test as legacy.

* When deprecations are skipped by the validator, the testsuite reporter will catch them so we need to mark the test as ignoring deprecations.

stof · 2025-04-22T07:31:28Z

.github/workflows/windows.yml

          # HttpClient tests need to run separately, they block when run with other components' tests concurrently
          php phpunit src\Symfony\Component\HttpClient || ($x = 1)

          exit $x

+  windows-all-extensions:


To make things easier to maintain, would it be worth backporting this split of the Windows jobs ? what do you think @nicolas-grekas ?

we talked about that in #58721 but decided to not split the two test runs as we would otherwise run many of the tests twice since the phpunit.skipped would not be shared

I don't see any discussion about that in the PR you linked

hm, maybe @nicolas-grekas and me talked about that on Slack directly 🤔

stof · 2025-04-22T07:33:28Z

Tests are not actually running with PHPUnit 11.5: https://github.com/symfony/symfony/actions/runs/14563576793/job/40849847611?pr=58370#step:6:44

xabbuh · 2025-04-22T08:53:01Z

Tests are not actually running with PHPUnit 11.5: https://github.com/symfony/symfony/actions/runs/14563576793/job/40849847611?pr=58370#step:6:44

Isn't that related to how our CI is configured? I mean the previous commit that is checked out does not have the changes needed in phpunit to install PHPUnit 11.5.

xabbuh · 2025-04-23T05:34:31Z

src/Symfony/Bridge/Doctrine/composer.json

@@ -30,7 +30,7 @@
        "symfony/dependency-injection": "^6.4|^7.0",
        "symfony/doctrine-messenger": "^6.4|^7.0",
        "symfony/expression-language": "^6.4|^7.0",
-        "symfony/form": "^6.4.6|^7.0.6",
+        "symfony/form": "^7.2",


because we need #58357

xabbuh requested review from welcoMattic, dunglas, OskarStark and yceruto as code owners September 24, 2024 07:20

carsonbot added Status: Needs Review labels Sep 24, 2024

carsonbot added this to the 7.2 milestone Sep 24, 2024

xabbuh force-pushed the phpunit11-integration-tests branch 2 times, most recently from 7178ae5 to 76245c5 Compare September 24, 2024 07:31

nicolas-grekas mentioned this pull request Sep 24, 2024

Tweak error/exception handler registration #58372

Merged

xabbuh force-pushed the phpunit11-integration-tests branch from 76245c5 to 24d86e8 Compare September 24, 2024 11:18

xabbuh requested a review from chalasr as a code owner September 24, 2024 11:18

xabbuh force-pushed the phpunit11-integration-tests branch 2 times, most recently from ab2f604 to e56f932 Compare September 25, 2024 16:42

xabbuh added the Status: Needs Work label Sep 27, 2024

xabbuh changed the title ~~run integration tests using PHPUnit 11~~ [WIP] run integration tests using PHPUnit 11 Sep 27, 2024

xabbuh force-pushed the phpunit11-integration-tests branch from e56f932 to bddc3f5 Compare September 27, 2024 11:30

xabbuh force-pushed the phpunit11-integration-tests branch 5 times, most recently from d72eadc to c89477a Compare October 10, 2024 06:49

xabbuh changed the title ~~[WIP] run integration tests using PHPUnit 11~~ run integration tests using PHPUnit 11 Oct 10, 2024

carsonbot removed the Status: Needs Work label Oct 10, 2024

xabbuh changed the title ~~run integration tests using PHPUnit 11~~ [WIP] run integration tests using PHPUnit 11 Oct 10, 2024

xabbuh mentioned this pull request Apr 17, 2025

[Lock] read (possible) error from Redis instance where evalSha() was called #60238

Merged

xabbuh force-pushed the phpunit11-integration-tests branch 9 times, most recently from 56d703c to eb73303 Compare April 20, 2025 21:42

xabbuh changed the title ~~[WIP] run tests using PHPUnit 11.5~~ run tests using PHPUnit 11.5 Apr 21, 2025

stof reviewed Apr 22, 2025

View reviewed changes

xabbuh force-pushed the phpunit11-integration-tests branch 4 times, most recently from 70f5b92 to 3e19ea6 Compare April 22, 2025 08:46

xabbuh force-pushed the phpunit11-integration-tests branch 2 times, most recently from 39cc605 to ca642e4 Compare April 23, 2025 05:17

xabbuh mentioned this pull request Apr 23, 2025

[Translation] drop support for nikic/php-parser 4 #60255

Merged

xabbuh commented Apr 23, 2025

View reviewed changes

xabbuh force-pushed the phpunit11-integration-tests branch 2 times, most recently from 03885db to 890b854 Compare April 24, 2025 18:13

run tests using PHPUnit 11.5

8f0b304

xabbuh force-pushed the phpunit11-integration-tests branch from 890b854 to 8f0b304 Compare April 24, 2025 19:19

xabbuh added 2 commits April 24, 2025 21:52

test the sigchld enabled job

bfa2a03

fixup! run tests using PHPUnit 11.5

bf35789

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

run tests using PHPUnit 11.5 #58370

run tests using PHPUnit 11.5 #58370

xabbuh commented Sep 24, 2024 •

edited by derrabus

Loading

xabbuh commented Sep 24, 2024

alexandre-daubois commented Sep 24, 2024

xabbuh commented Sep 24, 2024

nicolas-grekas commented Sep 25, 2024

xabbuh commented Apr 21, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

chalasr Apr 22, 2025

xabbuh Apr 23, 2025

stof Apr 22, 2025

stof Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof Apr 22, 2025

stof Apr 22, 2025

xabbuh Apr 22, 2025 •

edited

Loading

stof Apr 22, 2025

xabbuh Apr 22, 2025

stof commented Apr 22, 2025

xabbuh commented Apr 22, 2025

xabbuh Apr 23, 2025

	# Legacy tests are skipped when deps=high and when the current branch version has not the same major version number as the next one
	[[ "${{ matrix.mode }}" = high-deps && $SYMFONY_VERSION = *.4 ]] && echo LEGACY=,legacy >> $GITHUB_ENV \|\| true

		@@ -67,9 +68,8 @@ public function testInvalidValues($value, $message, $line)

		/**
		* When deprecations are skipped by the validator, the testsuite reporter will catch them so we need to mark the test as legacy.

run tests using PHPUnit 11.5 #58370

Are you sure you want to change the base?

run tests using PHPUnit 11.5 #58370

Conversation

xabbuh commented Sep 24, 2024 • edited by derrabus Loading

xabbuh commented Sep 24, 2024

alexandre-daubois commented Sep 24, 2024

xabbuh commented Sep 24, 2024

nicolas-grekas commented Sep 25, 2024

xabbuh commented Apr 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabbuh Apr 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stof commented Apr 22, 2025

xabbuh commented Apr 22, 2025

Choose a reason for hiding this comment

xabbuh commented Sep 24, 2024 •

edited by derrabus

Loading

xabbuh Apr 22, 2025 •

edited

Loading