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

Upgrade to PHPUnit 11 #1549

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ jobs:
- name: "Install dependencies"
run: composer install --ansi --no-interaction --no-progress

- name: Install PHPUnit
id: install
run: vendor/bin/simple-phpunit install

- name: Lint YAML files
if: always() && steps.install.outcome == 'success'
run: ./bin/console lint:yaml config --parse-tags
Expand Down
11 changes: 1 addition & 10 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ on:

env:
fail-fast: true
PHPUNIT_FLAGS: "-v"
xabbuh marked this conversation as resolved.
Show resolved Hide resolved
SYMFONY_PHPUNIT_DIR: "$HOME/symfony-bridge/.phpunit"
SYMFONY_DEPRECATIONS_HELPER: 'max[indirect]=52'

permissions:
contents: read
Expand Down Expand Up @@ -68,11 +65,5 @@ jobs:
php bin/console sass:build
php bin/console asset-map:compile

- name: "Install PHPUnit"
run: vendor/bin/simple-phpunit install

- name: "PHPUnit version"
run: vendor/bin/simple-phpunit --version

- name: "Run tests"
run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }}
run: bin/phpunit
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be vendor/bin/phpunit ?

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 phpunit/phpunit recipe also installs bin/phpunit. This is done to make migrating away from our simple-phpunit wrapper easy and without impacting documentation (as both use the same helper script).

I'm not sure why this project used the vendor binary in the CI (as the bin/phpunit file is mentioned in the README).

Copy link
Member

Choose a reason for hiding this comment

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

but you have not replaced the bin/phpunit file in this PR, so it still tries to use the phpunit-bridge

5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,8 @@
###> phpstan/phpstan ###
phpstan.neon
###< phpstan/phpstan ###

###> phpunit/phpunit ###
/phpunit.xml
.phpunit.result.cache
###< phpunit/phpunit ###
1 change: 1 addition & 0 deletions assets/icons/symfony.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@
"phpstan/phpstan": "^1.2",
"phpstan/phpstan-doctrine": "^1.3",
"phpstan/phpstan-symfony": "^1.2",
"phpunit/phpunit": "^11.3",
"symfony/browser-kit": "^7",
"symfony/css-selector": "^7",
"symfony/debug-bundle": "^7",
"symfony/maker-bundle": "^1.36",
"symfony/phpunit-bridge": "^7",
"symfony/stopwatch": "^7",
"symfony/web-profiler-bundle": "^7",
"twbs/bootstrap": "^5"
Expand Down
11 changes: 11 additions & 0 deletions config/packages/csrf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Enable stateless CSRF protection for forms and logins/logouts
framework:
form:
csrf_protection:
token_id: submit

csrf_protection:
Copy link
Member

Choose a reason for hiding this comment

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

this looks like a separate change

Copy link
Member Author

@wouterj wouterj Dec 13, 2024

Choose a reason for hiding this comment

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

Yes, this was just to get it working. I'm not really understanding the set-up of this project, because it has a symfony.lock but no composer.lock (although composer.lock is not gitignored). So I couldn't install phpunit/phpunit without also getting these recipe updates.

This is in a separate commit, which probably should be removed from this PR before merging.

stateless_token_ids:
- submit
- authenticate
- logout
2 changes: 0 additions & 2 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ parameters:
paths:
- src
- tests
bootstrapFiles:
- vendor/bin/.phpunit/phpunit/vendor/autoload.php
doctrine:
objectManagerLoader: tests/object-manager.php
symfony:
Expand Down
35 changes: 19 additions & 16 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
<?xml version="1.0" encoding="UTF-8"?>

<!-- https://phpunit.readthedocs.io/en/latest/configuration.html -->
<!-- https://docs.phpunit.de/en/11.5/configuration.html -->
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
backupGlobals="false"
failOnDeprecation="true"
wouterj marked this conversation as resolved.
Show resolved Hide resolved
failOnNotice="true"
failOnWarning="true"
colors="true"
bootstrap="tests/bootstrap.php"
convertDeprecationsToExceptions="false"
>
<php>
<ini name="display_errors" value="1" />
<ini name="error_reporting" value="-1" />
<server name="APP_ENV" value="test" force="true" />
<server name="SHELL_VERBOSITY" value="-1" />
<server name="SYMFONY_PHPUNIT_REMOVE" value="" />
<server name="SYMFONY_PHPUNIT_VERSION" value="9.6" />
<ini name="display_errors" value="1"/>
<ini name="error_reporting" value="-1"/>
<server name="APP_ENV" value="test" force="true"/>
<server name="SHELL_VERBOSITY" value="-1"/>
</php>

<testsuites>
Expand All @@ -23,22 +22,26 @@
</testsuite>
</testsuites>

<coverage processUncoveredFiles="true">
<source ignoreSuppressionOfDeprecations="true"
ignoreIndirectDeprecations="true"
>
<include>
<directory suffix=".php">src</directory>
</include>
</coverage>

<listeners>
<listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
</listeners>
<deprecationTrigger>
<function>trigger_deprecation</function>
<method>Doctrine\Deprecations\Deprecation::trigger</method>
<method>Doctrine\Deprecations\Deprecation::delegateTriggerToBackend</method>
</deprecationTrigger>
</source>

<extensions>
<!-- it begins a database transaction before every testcase and rolls it back after
the test finished, so tests can manipulate the database without affecting other tests -->
<extension class="DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension" />
<bootstrap class="DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension"/>

<!-- Run `composer require symfony/panther` before enabling this extension -->
<!-- <extension class="Symfony\Component\Panther\ServerExtension" /> -->
<!-- <bootstrap class="Symfony\Component\Panther\ServerExtension" /> -->
</extensions>
</phpunit>
57 changes: 57 additions & 0 deletions symfony.lock
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@
"phpstan.dist.neon"
]
},
"phpunit/phpunit": {
"version": "11.5",
"recipe": {
"repo": "github.com/symfony/recipes",
"branch": "main",
"version": "9.6",
"ref": "7364a21d87e658eb363c5020c072ecfdc12e2326"
},
"files": [
".env.test",
"phpunit.xml.dist",
"tests/bootstrap.php"
]
},
"symfony/apache-pack": {
"version": "1.0",
"recipe": {
Expand Down Expand Up @@ -125,6 +139,37 @@
".env"
]
},
"symfony/form": {
"version": "7.2",
"recipe": {
"repo": "github.com/symfony/recipes",
"branch": "main",
"version": "7.2",
"ref": "7d86a6723f4a623f59e2bf966b6aad2fc461d36b"
},
"files": [
"config/packages/csrf.yaml"
]
},
"symfony/framework-bundle": {
"version": "7.2",
"recipe": {
"repo": "github.com/symfony/recipes",
"branch": "main",
"version": "7.2",
"ref": "87bcf6f7c55201f345d8895deda46d2adbdbaa89"
},
"files": [
"config/packages/cache.yaml",
"config/packages/framework.yaml",
"config/preload.php",
"config/routes/framework.yaml",
"config/services.yaml",
"public/index.php",
"src/Controller/.gitignore",
"src/Kernel.php"
]
},
"symfony/mailer": {
"version": "7.2",
"recipe": {
Expand Down Expand Up @@ -239,6 +284,18 @@
"templates/base.html.twig"
]
},
"symfony/ux-icons": {
"version": "2.22",
"recipe": {
"repo": "github.com/symfony/recipes",
"branch": "main",
"version": "2.17",
"ref": "803a3bbd5893f9584969ab8670290cdfb6a0a5b5"
},
"files": [
"assets/icons/symfony.svg"
]
},
"symfony/ux-live-component": {
"version": "2.20",
"recipe": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Tester\CommandTester;

abstract class AbstractCommandTest extends KernelTestCase
abstract class AbstractCommandTestCase extends KernelTestCase
{
/**
* This helper method abstracts the boilerplate code needed to test the
Expand Down
14 changes: 7 additions & 7 deletions tests/Command/AddUserCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@

use App\Command\AddUserCommand;
use App\Repository\UserRepository;
use PHPUnit\Framework\Attributes\DataProvider;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;

final class AddUserCommandTest extends AbstractCommandTest
final class AddUserCommandTest extends AbstractCommandTestCase
{
/**
* @var string[]
Expand All @@ -35,11 +36,10 @@ protected function setUp(): void
}

/**
* @dataProvider isAdminDataProvider
*
* This test provides all the arguments required by the command, so the
* command runs non-interactively and it won't ask for any argument.
*/
#[DataProvider('isAdminDataProvider')]
public function testCreateUserNonInteractive(bool $isAdmin): void
{
$input = $this->userData;
Expand All @@ -52,13 +52,13 @@ public function testCreateUserNonInteractive(bool $isAdmin): void
}

/**
* @dataProvider isAdminDataProvider
*
* This test doesn't provide all the arguments required by the command, so
* the command runs interactively and it will ask for the value of the missing
* arguments.
* See https://symfony.com/doc/current/components/console/helpers/questionhelper.html#testing-a-command-that-expects-input
*
* @see https://symfony.com/doc/current/components/console/helpers/questionhelper.html#testing-a-command-that-expects-input
*/
#[DataProvider('isAdminDataProvider')]
public function testCreateUserInteractive(bool $isAdmin): void
{
$this->executeCommand(
Expand All @@ -76,7 +76,7 @@ public function testCreateUserInteractive(bool $isAdmin): void
* This is used to execute the same test twice: first for normal users
* (isAdmin = false) and then for admin users (isAdmin = true).
*/
public function isAdminDataProvider(): \Generator
public static function isAdminDataProvider(): \Generator
{
yield [false];
yield [true];
Expand Down
8 changes: 4 additions & 4 deletions tests/Command/ListUsersCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
namespace App\Tests\Command;

use App\Command\ListUsersCommand;
use PHPUnit\Framework\Attributes\DataProvider;

final class ListUsersCommandTest extends AbstractCommandTest
final class ListUsersCommandTest extends AbstractCommandTestCase
{
/**
* @dataProvider maxResultsProvider
*
* This test verifies the amount of data is right according to the given parameter max results.
*/
#[DataProvider('maxResultsProvider')]
public function testListUsers(int $maxResults): void
{
$tester = $this->executeCommand(
Expand All @@ -30,7 +30,7 @@ public function testListUsers(int $maxResults): void
$this->assertSame($emptyDisplayLines + $maxResults, mb_substr_count($tester->getDisplay(), "\n"));
}

public function maxResultsProvider(): \Generator
public static function maxResultsProvider(): \Generator
{
yield [1];
yield [2];
Expand Down
7 changes: 3 additions & 4 deletions tests/Controller/Admin/BlogControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use App\Entity\User;
use App\Repository\PostRepository;
use App\Repository\UserRepository;
use PHPUnit\Framework\Attributes\DataProvider;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\HttpFoundation\Response;
Expand Down Expand Up @@ -49,9 +50,7 @@ protected function setUp(): void
$this->client->loginUser($user);
}

/**
* @dataProvider getUrlsForRegularUsers
*/
#[DataProvider('getUrlsForRegularUsers')]
public function testAccessDeniedForRegularUsers(string $httpMethod, string $url): void
{
$this->client->getCookieJar()->clear();
Expand All @@ -67,7 +66,7 @@ public function testAccessDeniedForRegularUsers(string $httpMethod, string $url)
$this->assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN);
}

public function getUrlsForRegularUsers(): \Generator
public static function getUrlsForRegularUsers(): \Generator
{
yield ['GET', '/en/admin/post/'];
yield ['GET', '/en/admin/post/1'];
Expand Down
11 changes: 5 additions & 6 deletions tests/Controller/DefaultControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use App\Entity\Post;
use Doctrine\Bundle\DoctrineBundle\Registry;
use PHPUnit\Framework\Attributes\DataProvider;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\HttpFoundation\Response;

Expand All @@ -32,9 +33,8 @@ final class DefaultControllerTest extends WebTestCase
* PHPUnit's data providers allow to execute the same tests repeated times
* using a different set of data each time.
* See https://symfony.com/doc/current/testing.html#testing-against-different-sets-of-data.
*
* @dataProvider getPublicUrls
*/
#[DataProvider('getPublicUrls')]
public function testPublicUrls(string $url): void
{
$client = static::createClient();
Expand Down Expand Up @@ -69,9 +69,8 @@ public function testPublicBlogPost(): void
* The application contains a lot of secure URLs which shouldn't be
* publicly accessible. This tests ensures that whenever a user tries to
* access one of those pages, a redirection to the login form is performed.
*
* @dataProvider getSecureUrls
*/
#[DataProvider('getSecureUrls')]
public function testSecureUrls(string $url): void
{
$client = static::createClient();
Expand All @@ -84,14 +83,14 @@ public function testSecureUrls(string $url): void
);
}

public function getPublicUrls(): \Generator
public static function getPublicUrls(): \Generator
{
yield ['/'];
yield ['/en/blog/'];
yield ['/en/login'];
}

public function getSecureUrls(): \Generator
public static function getSecureUrls(): \Generator
{
yield ['/en/admin/post/'];
yield ['/en/admin/post/new'];
Expand Down
Loading
Loading