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

tests: prepare for phpunit 10 #842

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

Conversation

Chris53897
Copy link

add static data generators
fix deprecated file-structure for abstract test classes

@codecov
Copy link

codecov bot commented May 14, 2023

Codecov Report

Merging #842 (842928e) into master (bf5977f) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #842   +/-   ##
=========================================
  Coverage     98.72%   98.72%           
  Complexity      374      374           
=========================================
  Files            24       24           
  Lines          1022     1022           
=========================================
  Hits           1009     1009           
  Misses           13       13           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

tests/Element/AbstractElement.php Outdated Show resolved Hide resolved
@@ -6,7 +6,7 @@
use Behat\Mink\Selector\Xpath\Escaper;
use PHPUnit\Framework\TestCase;

abstract class NamedSelectorTest extends TestCase
abstract class AbstractNamedSelector extends TestCase
Copy link
Member

Choose a reason for hiding this comment

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

What was the purpose of this class renaming?

Copy link
Author

Choose a reason for hiding this comment

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

PHPUnit 10 will raise a warning, if there are classes with the Suffix Test and no tests or abstract Classes. sebastianbergmann/phpunit#4979 (comment)
sebastianbergmann/phpunit#4979 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

The CI Tests failed because of this

Copy link
Member

Choose a reason for hiding this comment

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

I would actually use TestCase as suffix here

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 removed the prefix and added your suggested suffix.

tests/Element/DocumentElementTestCaseTest.php Outdated Show resolved Hide resolved
tests/Element/NodeElementTestCaseTest.php Outdated Show resolved Hide resolved
tests/Selector/ExactNamedSelectorTestCaseTest.php Outdated Show resolved Hide resolved
tests/Selector/PartialNamedSelectorTestCaseTest.php Outdated Show resolved Hide resolved
Copy link
Member

@aik099 aik099 left a comment

Choose a reason for hiding this comment

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

@Chris53897 , what is the general purpose of renaming *Test classes into *TestCase classes?

@stof
Copy link
Member

stof commented Jun 22, 2023

@aik099 PHPUnit 10 does not allow have abstract classes ending with Test (older versions of PHPUnit were silently considering that those were not actually tests that should be loaded in the testsuite)

add static data generators
fix deprecated file-structure for abstract test classes
add static data generators
fix deprecated file-structure for abstract test classes
add static data generators
fix deprecated file-structure for abstract test classes
@Chris53897 Chris53897 force-pushed the feature/prepare-for-phpunit-10 branch from 32c5948 to 842928e Compare June 23, 2023 18:28
@Chris53897
Copy link
Author

I changed the Filenames. I hope everything is correct now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants