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.

Sign up for GitHub

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

Feature/allow phpunit 10 #841

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feature/allow phpunit 10 #841

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81eed3b
chore: fix ci deprecations, add test-runs for php 8.2, allow phpunit 10
Chris8934 May 12, 2023
bf8a96e
chore: fix ci deprecations, add test-runs for php 8.2, allow phpunit 10
Chris8934 May 12, 2023
bf9c7d1
chore: fix ci deprecations, add test-runs for php 8.2, allow phpunit 10
Chris8934 May 12, 2023
440cdaa
chore: fix ci deprecations, add test-runs for php 8.2, allow phpunit 10
Chris8934 May 12, 2023
f04540b
chore: fix ci deprecations, add test-runs for php 8.2, allow phpunit 10
Chris8934 May 14, 2023
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
8 changes: 4 additions & 4 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:

strategy:
matrix:
php: ['7.2', '7.3', '7.4', '8.0', '8.1']
php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2']
composer_flags: [ '' ]
minimum_stability: [ '' ]
symfony_deprecations_helper: [ '' ]
Expand All @@ -32,7 +32,7 @@ jobs:

steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
fetch-depth: 1

Expand All @@ -55,9 +55,9 @@ jobs:
- name: Run tests
run: |
export SYMFONY_DEPRECATIONS_HELPER="${{ matrix.symfony_deprecations_helper }}"
vendor/bin/phpunit -v --coverage-clover=coverage.xml
vendor/bin/phpunit --coverage-clover=coverage.xml

- name: Upload coverage
uses: codecov/codecov-action@v2
uses: codecov/codecov-action@v3
with:
files: coverage.xml
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
*.phar
phpunit.xml
.phpunit.result.cache
.phpunit.cache
composer.lock
vendor
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

"require-dev": {
"symfony/error-handler": "^4.4 || ^5.0 || ^6.0",
"phpunit/phpunit": "^8.5.22 || ^9.5.11",
"phpunit/phpunit": "^8.5.33 || ^9.6 || ^10.1",
"symfony/phpunit-bridge": "^5.4 || ^6.0"
},

Expand Down
33 changes: 19 additions & 14 deletions phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
colors="true"
bootstrap="vendor/autoload.php"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.1/phpunit.xsd"
cacheDirectory=".phpunit.cache">
<coverage/>

<phpunit colors="true" bootstrap="vendor/autoload.php">
<testsuites>
<testsuite name="Behat Mink test suite">
<directory>tests</directory>
</testsuite>
</testsuites>
<testsuites>
<testsuite name="Behat Mink test suite">
<directory>tests</directory>
<exclude>./tests/Element/ElementTest.php</exclude>
<exclude>./tests/Selector/NamedSelectorTest.php</exclude>
Copy link
Member

Choose a reason for hiding this comment

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

excluding some tests is a no-go. Our testsuite must run.

If those are not actually tests but abstract test cases, the right fix is to rename them rather than having to maintain exclude rules.

Copy link
Author

Choose a reason for hiding this comment

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

These are abstract Tests. I changed the tests. There are of course multiple solutions. Please check if my solution is ok.

</testsuite>

<filter>
<whitelist>
<directory>./src</directory>
</whitelist>
</filter>
</testsuites>

<source>
<include>
<directory>./src</directory>
</include>
</source>

<listeners>
<listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
Copy link
Member

Choose a reason for hiding this comment

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

This makes us loose the existing feature where our CI fails if we trigger deprecated APIs, which is something we wanted to have

Copy link
Author

Choose a reason for hiding this comment

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

Yes of course. This --migrate-configuration does remove it, and i did not checked the backup file. I will re-add this

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the symfony/phpunit-bridge is not yet compatible with PHPUnit 10. So this is a blocker for using it in our CI

Copy link
Author

Choose a reason for hiding this comment

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

I just noticed that ;(
I added it, but outcommented the code. Xml-Names changed too, i am not sure if phunit 8 can handle the new Xml-Names.

Copy link
Author

Choose a reason for hiding this comment

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

I guess the best solution is, to wait until the bridge symfony/phpunit-bridge is compatible with PHPUnit 10.
There could be 2 seperate phpunit (for 8/9 and 10) config files and a switch in the ci tests.
But i guess that is not what we want.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. There is nothing wrong with using PHPUnit 9 in our CI. We don't want to maintain 2 configurations if we can avoid it.

However, the TestCase renaming and the migration to static methods for data-providers are something we could merge even if we don't use PHPUnit 10 yet.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, i will provide a seperate PR. (probably this weekend)

</listeners>
</phpunit>
8 changes: 7 additions & 1 deletion tests/Driver/CoreDriverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ public function testNoExtraMethods()

public function testCreateNodeElements()
{
if(\PHPUnit\Runner\Version::id() >= 10) {
$driver = $this->getMockBuilder('Behat\Mink\Driver\CoreDriver')
->onlyMethods(array('findElementXpaths'))
->getMockForAbstractClass();
} else {
$driver = $this->getMockBuilder('Behat\Mink\Driver\CoreDriver')
->setMethods(array('findElementXpaths'))
->getMockForAbstractClass();
}

$session = $this->getMockBuilder('Behat\Mink\Session')
->disableOriginalConstructor()
Expand Down Expand Up @@ -72,7 +78,7 @@ public function testInterfaceMethods(\ReflectionMethod $method)
call_user_func_array(array($driver, $method->getName()), $this->getArguments($method));
}

public function getDriverInterfaceMethods()
public static function getDriverInterfaceMethods()
{
$ref = new \ReflectionClass('Behat\Mink\Driver\DriverInterface');

Expand Down
2 changes: 1 addition & 1 deletion tests/Exception/ElementNotFoundExceptionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function testBuildMessage($message, $type, $selector = null, $locator = n
$this->assertEquals($message, $exception->getMessage());
}

public function provideExceptionMessage()
public static function provideExceptionMessage()
{
return array(
array('Tag not found.', null),
Expand Down
4 changes: 2 additions & 2 deletions tests/Selector/NamedSelectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function testEscapedSelectors($fixtureFile, $selector, $locator, $expecte
$this->testSelectors($fixtureFile, $selector, $locator, $expectedExactCount, $expectedPartialCount);
}

public function getSelectorTests()
public static function getSelectorTests()
{
$fieldCount = 8; // fields without `type` attribute
$fieldCount += 4; // fields with `type=checkbox` attribute
Expand Down Expand Up @@ -187,7 +187,7 @@ public function getSelectorTests()
);
}

public function getLateRegisteredReplacements()
public static function getLateRegisteredReplacements()
{
// The following tests all use `test.html` from the fixtures directory.
return array(
Expand Down
2 changes: 1 addition & 1 deletion tests/Selector/Xpath/EscaperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function testXpathLiteral($string, $expected)
$this->assertEquals($expected, $escaper->escapeLiteral($string));
}

public function getXpathLiterals()
public static function getXpathLiterals()
{
return array(
array('some simple string', "'some simple string'"),
Expand Down
2 changes: 1 addition & 1 deletion tests/Selector/Xpath/ManipulatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function testPrepend($prefix, $xpath, $expectedXpath)
$this->assertEquals($expectedXpath, $manipulator->prepend($xpath, $prefix));
}

public function getPrependedXpath()
public static function getPrependedXpath()
{
return array(
'simple' => array(
Expand Down
2 changes: 1 addition & 1 deletion tests/SessionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public function testGetResponseHeader($expected, $name, array $headers)
$this->assertSame($expected, $this->session->getResponseHeader($name));
}

public function provideResponseHeader()
public static function provideResponseHeader()
{
return array(
array('test', 'Mink', array('Mink' => 'test')),
Expand Down
2 changes: 1 addition & 1 deletion tests/WebAssertTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ public function testElementNotExistsArrayLocator($selector, $locator, $expectedM
);
}

public function getArrayLocatorFormats()
public static function getArrayLocatorFormats()
{
return array(
'named' => array(
Expand Down