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

Inconsistency between "phpunit --list-suites" and startTestSuite events #3188

Closed
vitanivanov opened this issue Jul 2, 2018 · 15 comments
Closed

Comments

@vitanivanov
Copy link

vitanivanov commented Jul 2, 2018

Q A
PHPUnit version 6.5.8
PHP version 7.2.3
Installation Method Composer

Preconditions

  1. phpunit.xml:
<?xml version="1.0" encoding="UTF-8"?>
<phpunit backupGlobals="false"
         backupStaticAttributes="false"
         bootstrap="vendor/autoload.php"
         colors="true"
         convertErrorsToExceptions="true"
         convertNoticesToExceptions="true"
         convertWarningsToExceptions="true"
         processIsolation="false"
         stopOnFailure="false">
    <testsuites>
        <testsuite name="Feature">
            <directory suffix="Test.php">./tests/Feature</directory>
        </testsuite>
        <testsuite name="Unit">
            <directory suffix="Test.php">./tests/Unit</directory>
        </testsuite>
    </testsuites>
    <filter>
        <whitelist processUncoveredFilesFromWhitelist="true">
            <directory suffix=".php">./app</directory>
        </whitelist>
    </filter>
    <listeners>
        <listener class="App\Listeners\Testing\CustomTestListener">
        </listener>
    </listeners>
    <php>
        <env name="APP_ENV" value="testing"/>
        <env name="BCRYPT_ROUNDS" value="4"/>
        <env name="CACHE_DRIVER" value="array"/>
        <env name="SESSION_DRIVER" value="array"/>
        <env name="QUEUE_DRIVER" value="sync"/>
        <env name="MAIL_DRIVER" value="array"/>
    </php>
</phpunit>
  1. CustomTestListener.php:
<?php

namespace App\Listeners\Testing;

use PHPUnit\Framework\TestListener;
use PHPUnit\Framework\TestListenerDefaultImplementation;
use PHPUnit\Framework\TestSuite;

class CustomTestListener implements TestListener
{
    use TestListenerDefaultImplementation;

    public function startTestSuite(TestSuite $suite)
    {
        printf("TestSuite '%s' started.\n", $suite->getName());
    }
}
  1. I have 2 test methods in tests\Unit\RegistrationServiceTest.php

Actual behavior

  1. Running "vendor/bin/phpunit --list-suites" returns the following output:
    PHPUnit 6.5.8 by Sebastian Bergmann and contributors.

Available test suite(s):

  • Feature
  • Unit
  1. Running "vendor/bin/phpunit --testsuite Unit" returns the following output:
    PHPUnit 6.5.8 by Sebastian Bergmann and contributors.

TestSuite '' started.
TestSuite 'Unit' started.
TestSuite 'Tests\Unit\RegistrationServiceTest' started.
.. 2 / 2 (100%)

Expected behavior

I do not expect going through startTestSuite() multiple when I have explicitly set the test suite I want to execute.

  1. Why are 3 suites started: '', 'Unit', 'Tests\Unit\RegistrationServiceTest'?
  2. Is this expected?
  3. What are the benefits of this behavior?
  4. Is it configurable?
@epdenouden
Copy link
Contributor

Hello @vitanivanov !

A quick first reply: yes, the general behaviour you see is as expected. Although my work on sorting TestSuites and TestCases was for version 7.x, I do believe 6.x has much of the same handling. I will (try to) answer your question in more detail on Wednesday as I am currently travelling.

Some quick first answers:

  1. '' is the nameless default or 'root' TestSuite and Unit is the suite you actually called. I do not know the contents of Tests\Unit\RegistrationServiceTest, can I see it on Github somewhere? Tests that use a @dataprovider are run as a suite of tests, each with a new data row.
  2. so yes, it is expected
  3. the main benefit is that TestSuites can be nested inside other TestSuites; for the user on the 'outside' it can look a bit confusing, I agree. I had to dig into PHPUnit's internals to understand it.
  4. No. To make this configurable would require a lot of work and redesign.

@vitanivanov
Copy link
Author

@epdenouden :

  1. Since '' (the nameless default root) never contains any tests, why does it have to go through startTestSuite() when the --testsuite option is specified
  2. The RegistrationServiceTest is not public: It consists of methods marked as "@test" with a single method call and a single assertion regarding the result without any dataproviders.
  3. I haven't dig through the whole framework, but I think this is a good candidate for a new annotation: e.g. "@testsuite" before a class/method in order to regard it as a new test suite

@sebastianbergmann
Copy link
Owner

TestSuite is a relic from the past that I would like to get rid off sooner rather than later.

@vitanivanov
Copy link
Author

vitanivanov commented Jul 2, 2018

Is there an elegant way to actually to handle the event of starting the test suite I had specified?

@epdenouden
Copy link
Contributor

@vitanivanov You could always dig through $ARGV ofcourse ;-) Other than that you can only get at it during the startTestSuite() with $suite->getName(). There doesn't seem to be a cleaner way to get at the configuration during a test run. Which is unsurprising as it's all supposed to be isolated and sandboxed :)

@epdenouden
Copy link
Contributor

@sebastianbergmann are there any issues/proposals/prototypes for refactoring the structures of suites, tests, dataprovidersuites?

@karser
Copy link

karser commented Jul 20, 2018

PHPUnit 7.2.6, I ended up with this condition:

class PhpUnitListener implements TestListener
{
    use TestListenerDefaultImplementation;

    public function startTestSuite(TestSuite $suite): void
    {
        if ($this->isSuite($suite)) {
            //the suit started
        }
    }

    private function isSuite(TestSuite $suite): bool
    {
        return !empty($suite->getName()) && false === strpos($suite->getName(), '\\');
    }
}

@vitanivanov
Copy link
Author

@karser Thanks. However, parsing the relevant phpunit.xml file and checking for the suite inside it is a more reliable solution imo.

@stale
Copy link

stale bot commented Sep 18, 2018

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Sep 18, 2018
@stale
Copy link

stale bot commented Nov 21, 2018

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 21, 2018
@karser
Copy link

karser commented Nov 21, 2018

TestSuite is a relic from the past that I would like to get rid off sooner rather than later.

@sebastianbergmann Has the situation changed? What would you recommend using instead of startTestSuite?

@stale stale bot removed the stale label Nov 21, 2018
@sebastianbergmann
Copy link
Owner

I can only recommend to not use the TestListener interface but the new TestHook interfaces instead.

@epdenouden
Copy link
Contributor

@karser As @sebastianbergmann says, please use the new interfaces if you are creating new functionality.

I am currently refactoring the old TestDox printer to get existing functionality back that was lost due to new execution reordering features. It is a messy little project. Afterwards I will work on a cleaner TestHook version, mostly to see what functionality is still missing from that interface.

If you have any suggestions or wishes, feel free to drop a proposal here at the RFC ticket for the TestHook interfaces: #3390

@stale
Copy link

stale bot commented Jan 22, 2019

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 22, 2019
@stale
Copy link

stale bot commented Jan 29, 2019

This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions.

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

No branches or pull requests

4 participants