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

Support using @depends to depend on classes #3936

Merged
merged 28 commits into from Jun 15, 2020
Merged

Support using @depends to depend on classes #3936

merged 28 commits into from Jun 15, 2020

Conversation

epdenouden
Copy link
Contributor

@epdenouden epdenouden commented Nov 8, 2019

Use case

As a developer of PHPUnit test collections
I want to declare dependencies between tests and whole TestCase classes
so that large or interdependent collections can be more easily managed

This implements feature request #3519 by adding support for declaring interdependency between a test and a TestCase class using the existing @depends annotation. This is often useful for managing slow integration suites: "don't even try this if SomeTestClass didn't pass."

Detailed use case with additional context provided by @spriebsch.

Example

class DependencyOnClassTest extends TestCase
{
    /**
     * Guard support for using annotations to depend on a whole successful TestSuite
     *
     * @depends DependencySuccessTest::class
     *
     * @see https://github.com/sebastianbergmann/phpunit/issues/3519
     */
    public function testThatDependsOnASuccessfulClass(): void
    {
        $this->assertTrue(true);
    }
	// please refer to the PHPUnit source for the full version of this example
}

Example from the new unit test DependencyOnClassTest and configuration file

Implementation

  • implement improved dependency resolver, while still using the proven TestSuiteSorter
  • implement @depends SomeTest::class dependency check mechanism
  • track run state of individual TestSuite objects based on its child tests
  • a TestCase::class is considered passed if it doesn't contain any failing tests
  • test housekeeping to cover the new resolver and supporting infrastructure

Groundwork for future improvements

  • next project: move execution reordering to the core iterator
  • refactoring the on-demand @dataProvider prototype to production code will be much easier with all the new plumbing

Testing

The new functionality is guarded by multiple unit and end-to-end tests.

To do

  • implement full end-to-end scenario with config file and multiple test suites
  • add @depends-annotation normalization as this is provided by user input and not reflection
  • TestSuite pass/fail tracking in TestResult
  • skip-test behaviour for whole TestSuite based on @depends ::class
  • implement tree-traversing provides/requires dependency resolver
    • TestSuite
    • DataProviderTestSuite
    • TestCase
    • PhptTestCase
  • replace resolver in TestSuiteSorter with new one
  • get code to LTS release quality
    • improve simplistic TestSuite dirty flagging
    • full feature code coverage
    • improve project code coverage

Notes

I realize there will be edge-cases with a new feature like this. Give it a try and let me know what you think!

Ping @barbu110

@epdenouden epdenouden added the type/enhancement A new idea that should be implemented label Nov 8, 2019
@codecov
Copy link

codecov bot commented Nov 8, 2019

Codecov Report

Merging #3936 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3936      +/-   ##
============================================
+ Coverage     84.67%   84.70%   +0.02%     
- Complexity     4551     4563      +12     
============================================
  Files           257      257              
  Lines         11428    11487      +59     
============================================
+ Hits           9677     9730      +53     
- Misses         1751     1757       +6     
Impacted Files Coverage Δ Complexity Δ
src/Framework/DataProviderTestSuite.php 100.00% <100.00%> (+7.69%) 3.00 <0.00> (-3.00) ⬆️
src/Framework/TestCase.php 85.33% <100.00%> (+1.28%) 335.00 <4.00> (-1.00) ⬆️
src/Framework/TestResult.php 78.75% <100.00%> (+0.37%) 182.00 <1.00> (+4.00)
src/Framework/TestSuite.php 81.81% <100.00%> (-5.54%) 129.00 <14.00> (+9.00) ⬇️
src/Runner/PhptTestCase.php 71.88% <100.00%> (+1.33%) 113.00 <2.00> (+2.00)
src/Runner/TestSuiteSorter.php 100.00% <100.00%> (ø) 54.00 <0.00> (-7.00)
src/Util/Test.php 95.40% <100.00%> (+1.35%) 134.00 <4.00> (+8.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d9c56c...0fc833e. Read the comment docs.

@sebastianbergmann sebastianbergmann changed the title Feature: declare class-based dependencies using @depends Ceclare class-based dependencies using @depends Mar 30, 2020
@sebastianbergmann sebastianbergmann added the feature/test-dependencies Issues related to explicitly declared dependencies between tests label Mar 30, 2020
@sebastianbergmann sebastianbergmann changed the title Ceclare class-based dependencies using @depends Declare class-based dependencies using @depends Mar 30, 2020
@epdenouden epdenouden marked this pull request as draft June 10, 2020 08:01
@epdenouden epdenouden marked this pull request as ready for review June 14, 2020 20:36
src/Framework/TestCase.php Outdated Show resolved Hide resolved
@sebastianbergmann sebastianbergmann linked an issue Jun 15, 2020 that may be closed by this pull request
src/Framework/TestCase.php Outdated Show resolved Hide resolved
src/Framework/TestCase.php Outdated Show resolved Hide resolved
src/Framework/TestCase.php Outdated Show resolved Hide resolved
src/Framework/TestResult.php Outdated Show resolved Hide resolved
src/Framework/TestResult.php Show resolved Hide resolved
src/Runner/TestSuiteSorter.php Outdated Show resolved Hide resolved
src/Util/Test.php Show resolved Hide resolved
src/Util/Test.php Outdated Show resolved Hide resolved
src/Util/Test.php Outdated Show resolved Hide resolved
tests/_files/NumericGroupAnnotationTest.php Show resolved Hide resolved
@sebastianbergmann
Copy link
Owner

Thank you so much, @epdenouden and @localheinz, for working on this!

src/Framework/TestSuite.php Outdated Show resolved Hide resolved
src/Framework/TestSuite.php Outdated Show resolved Hide resolved
src/Framework/TestSuite.php Show resolved Hide resolved
src/Runner/TestSuiteSorter.php Show resolved Hide resolved
src/Runner/TestSuiteSorter.php Show resolved Hide resolved
src/Util/Test.php Outdated Show resolved Hide resolved
@sebastianbergmann sebastianbergmann merged commit b729cc7 into sebastianbergmann:master Jun 15, 2020
@sebastianbergmann sebastianbergmann changed the title Declare class-based dependencies using @depends Support using @depends to depend on classes Jun 15, 2020
fabpot added a commit to symfony/symfony that referenced this pull request Aug 8, 2020
…s (derrabus)

This PR was merged into the 5.1 branch.

Discussion
----------

[String] We cannot have a "provides" function in test cases

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #37564
| License       | MIT
| Doc PR        | N/A

Because of a change in PHPUnit 9.3 (see sebastianbergmann/phpunit#3936), we cannot have define a method named `provides` in test cases. And since php is case-insensitive regarding method calls, the method `provideS` used by the String component's `FunctionTest` will cause a fatal error. I have renamed it to work around that issue.

cc @fancyweb

Commits
-------

46e2a0c [String] We cannot have a "provides" function in test cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-dependencies Issues related to explicitly declared dependencies between tests type/enhancement A new idea that should be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using @depends to depend on classes
4 participants