One thing that would be nice for CI: add --no-progress when running tests, because we don't really need to scroll through pages of progress output to get to the test failures.

For the following:

To figure out: it looks like the data provider phase runs MUCH slower in PHPUnit 10; locally, PHPUnit hangs for ~40 seconds before actually starting tests, as opposed to just a few seconds in PHPUnit 9. I still don't know what causes this, whether it's caused by the data provider changes, whether it's caused by our bootstrap code, etc.

I ran PHPUnit with excimer enabled, and from the speedscope output it was immediately clear that 40% of the overall runtime was spent in TestSuiteBuilder::process, and particularly TestCase::valueObjectForEvents -> TestMethodBuilder::fromTestCase -> TestMethodBuilder::dataFor -> Exporter::export, which then recurses a ton of times (Exporter::recursiveExport). Looking at the code in question, what it does is take the arguments provided by a data provider, and obtain a string representation for them. Exporter is responsible for the "obtain a string representation" part, which as you can see has lots of special cases and really iterates objects deeply to obtain a full representation. This becomes an issue when a data provider returns a huge object, for which the exporting process takes a veeeery long time. I added some debug code to PHPUnit to print info about the exported data when the string representation is longer than 10M. The biggest offenders for core's "includes" suite are Language (13M), Message (14M), MockApi mocks (15M, and yes, we're mocking something called "Mock"), AuthenticationResponse (15M, often inside StatusValue), SystemBlock (15M), CompositeBlock (16M), FileBackendGroup (13M) and ParserOptions (15M).

I guess this would be addressed by the "return plain data" of T332865, but this may be quite hard to enforce, unlike the "static functions" part.

On top of that, TestCase::valueObjectForEvents is also called while tests are running, though I haven't looked into what it's used for. Overall, according to the excimer output, valueObjectForEvents() is responsible for 53% of the overall runtime of the PHPUnit process.

One feature I particularly like in PHPUnit 10, is the ability to pass multiple directories or files to phpunit. I've considered it a bug not to have this, in particular because the command is completely silent about the subsequent arguments, e.g. you can run phpunit tests/includes/ResourceLoader/* today and it would "succeed" with a bunch of meaningless dots, having actually only executed the first file that bash expanded the star to.

Likewise if you run phpunit includes/foo includes/bar it passes today, but actually only runs the first directory. No error, no warning.

https://github.com/sebastianbergmann/phpunit/pull/5462

To figure out: it looks like the data provider phase runs MUCH slower in PHPUnit 10; locally, PHPUnit hangs for ~40 seconds before actually starting tests, as opposed to just a few seconds in PHPUnit 9. I still don't know what causes this, whether it's caused by the data provider changes, whether it's caused by our bootstrap code, etc.

Have we reported this to upstream PHPUnit? Sebastian seems pretty interested in performance recently. It's possible that there's a way we can do something different or better to avoid it, but I don't think he would mind finding out that this is something that can happen during upgrades. Perhaps he'd improve docs as a result to make the recommended solution easier to find, or perhaps some kind of warning or deprecation notice for whatever unusual thing we do.

In T328919#10046294, @Krinkle wrote:

To figure out: it looks like the data provider phase runs MUCH slower in PHPUnit 10; locally, PHPUnit hangs for ~40 seconds before actually starting tests, as opposed to just a few seconds in PHPUnit 9. I still don't know what causes this, whether it's caused by the data provider changes, whether it's caused by our bootstrap code, etc.

Have we reported this to upstream PHPUnit? Sebastian seems pretty interested in performance recently. It's possible that there's a way we can do something different or better to avoid it, but I don't think he would mind finding out that this is something that can happen during upgrades. Perhaps he'd improve docs as a result to make the recommended solution easier to find, or perhaps some kind of warning or deprecation notice for whatever unusual thing we do.

No, but at the time I came across issue #5184 (and duplicate #5183) which seems to be closely related. AIUI, the recommended solution is to use data providers as they're meant to be used, i.e. returning only plain data from them, as noted in T328919#9138704.