Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC: Speed optimize class-loading for levels tests #2916

Merged
merged 17 commits into from Feb 18, 2024
Merged
33 changes: 33 additions & 0 deletions .github/workflows/tests-levels-matrix.php
@@ -0,0 +1,33 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

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

declare strict types please


$testList = shell_exec('php vendor/bin/phpunit --list-tests');
Copy link
Contributor Author

@staabm staabm Feb 16, 2024

Choose a reason for hiding this comment

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

currently the --list-tests phpunit feature does not allow filtering by test-groups.

I have opened a feature request about that, so we may can get rid/simplify this script in the future.

sebastianbergmann/phpunit#5702


if (!is_string($testList)) {
throw new RuntimeException('Error while listing tests');
}

$testFilters = [];
foreach(explode("\n", $testList) as $line) {
$cleanedLine = trim($line, ' -');

if ($cleanedLine === '') {
continue;
}

if (
!str_contains($cleanedLine, 'PHPStan\Generics\GenericsIntegrationTest') &&
!str_contains($cleanedLine, 'PHPStan\Levels\LevelsIntegrationTest')
Copy link
Member

Choose a reason for hiding this comment

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

We should check the @group inside the docblock instead of hardcoding these two classes.

Copy link
Contributor Author

@staabm staabm Feb 17, 2024

Choose a reason for hiding this comment

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

to make this work based on the phpdoc, I would have needed to reflect on classes and read all class-comments.

instead I updated phpunit to 10.x for the matrix job and used --list-tests-xml which returns a xml containing group information. its a option which does not exist in phpunit 9.x

) {
continue;
}

$cleanedLine = str_replace('\\', '\\\\', $cleanedLine);

$testFilters[] = $cleanedLine;
}

if ($testFilters === []) {
throw new RuntimeException('No tests found');
}

echo json_encode($testFilters);
36 changes: 35 additions & 1 deletion .github/workflows/tests.yml
Expand Up @@ -98,11 +98,45 @@ jobs:
- name: "Tests"
run: "make tests-integration"

tests-levels-matrix:
name: "Determine levels tests matrix"
runs-on: ubuntu-latest
timeout-minutes: 60

steps:
- name: "Checkout"
uses: actions/checkout@v3

- name: "Install PHP"
uses: "shivammathur/setup-php@v2"
with:
coverage: "none"
php-version: "8.1"
Copy link
Member

Choose a reason for hiding this comment

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

Let's do 8.3 here :)

tools: pecl
extensions: ds,mbstring
ini-file: development
ini-values: memory_limit=1G

- name: "Install dependencies"
run: "composer install --no-interaction --no-progress"

- id: set-matrix
run: echo "matrix=$(php .github/workflows/tests-levels-matrix.php)" >> $GITHUB_OUTPUT

outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}

tests-levels:
needs: tests-levels-matrix

name: "Levels tests"
runs-on: ubuntu-latest
timeout-minutes: 60

strategy:
matrix:
test-filter: "${{fromJson(needs.tests-levels-matrix.outputs.matrix)}}"

steps:
- name: "Checkout"
uses: actions/checkout@v3
Expand All @@ -121,7 +155,7 @@ jobs:
run: "composer install --no-interaction --no-progress"

- name: "Tests"
run: "make tests-levels"
run: "php vendor/bin/phpunit --filter '${{ matrix.test-filter }}' --group levels"
Copy link
Member

Choose a reason for hiding this comment

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

I just did some local testing and in this format PHPUnit will still execute all data providers from all tests. It would be good to also add the file in which the filtered test is. I think we can get even faster that way.

See the comparison:

time vendor/bin/phpunit --filter 'PHPStan\\Type\\UnionTypeTest::testSorting'
15.7s

time vendor/bin/phpunit tests/PHPStan/Type/UnionTypeTest.php --filter 'PHPStan\\Type\\UnionTypeTest::testSorting'
1.1s

Copy link
Contributor Author

@staabm staabm Feb 17, 2024

Choose a reason for hiding this comment

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

indeed, it got even faster now :)

grafik


tests-old-phpunit:
name: "Tests with old PHPUnit"
Expand Down
Expand Up @@ -112,6 +112,7 @@ public function create(string $projectInstallationPath): ?SourceLocator
foreach ($classMapPaths as $classMapPath) {
if (is_dir($classMapPath)) {
$locators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($classMapPath);
continue;
}
if (!is_file($classMapPath)) {
continue;
Expand Down