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

Can't run non-first test class in test file #4030

Closed
isfedorov opened this issue Jan 30, 2020 · 19 comments
Closed

Can't run non-first test class in test file #4030

isfedorov opened this issue Jan 30, 2020 · 19 comments
Labels
feature/test-runner CLI test runner type/bug Something is broken

Comments

@isfedorov
Copy link
Contributor

Q A
PHPUnit version 8.5.2
PHP version 7.4.2
Installation Method Composer

Summary

No tests executed when try to run non-first test class in a test file

Current behavior

"No tests executed!" result message

How to reproduce

Create file MyTest.php with several test classes like

<?php
use PHPUnit\Framework\TestCase;

class TestFirst extends TestCase
{

    public function testOne()
    {
        static::assertTrue(true);
    }

    public function testTwo()
    {
        static::assertTrue(true);
    }
}

class TestSecond extends TestCase
{
    public function testOne()
    {
        static::assertFalse(false);
    }
}

Run command from cli
/usr/bin/php vendor/phpunit/phpunit/phpunit --no-configuration --filter TestSecond MyTests.php

Expected behavior

TestSecond test should be run

@isfedorov isfedorov added the type/bug Something is broken label Jan 30, 2020
@sebastianbergmann
Copy link
Owner

@flow-control Could this be related to the changes you made?

@realFlowControl
Copy link
Sponsor Contributor

Might be, though i did not (at least not on purpose) change something on how the filter argument works. I'll have a look anyway.
@isfedorov do know the latest PHPUnit version this was working?

@realFlowControl
Copy link
Sponsor Contributor

I tried in various versions between v8.5.2 and v8.0.0 but could not get this to work as expected. This is the output with v8.1.6:

$ php phpunit --no-configuration --filter TestSecond MyTests.php
PHPUnit 8.1.6 by Sebastian Bergmann and contributors.



Time: 16 ms, Memory: 6.00 MB

No tests executed!

@realFlowControl
Copy link
Sponsor Contributor

This is what happens in master

$ php phpunit --no-configuration --filter TestSecond MyTests.php
Class 'MyTests' could not be found in '/home/bowman/Work/flow-control/phpunit/MyTests.php'.

@realFlowControl
Copy link
Sponsor Contributor

I found out another info. It has only worked this way in the past, by using the class name in CLI:

$ php phpunit --no-configuration --filter TestSecond TestSecond MyTests.php

This feature has been depreacted in v8.5 and removed in v9 as of #4014

@sebastianbergmann I think i could make this work technically, by "just" passing the filter to the PHPUnit\Runner\StandardTestSuiteLoader and try to extract a class name from there. If the class is not loaded there, the filtering will not find any tests. I might find time tonight or on saturday/sunday for this.

@isfedorov
Copy link
Contributor Author

@flow-control but according to #3860 (comment) and https://phpunit.readthedocs.io/en/8.5/textui.html#textui-examples-filter-patterns --filter TestSecond is the valid syntax

@realFlowControl
Copy link
Sponsor Contributor

realFlowControl commented Jan 30, 2020

@isfedorov exactly, but the difference is in that case, that the class in the file had the exact same name as the filename (without the .php) and that's why it's working.

The "problem" is, that the suite loader does not load the test class (as there is no test class in that file named MyTests) and therefore filtering is done on an empty list ;-)

OTOH what is working is the following scenario.

$ ls test/
MyTests.php
$ php phpunit --no-configuration --filter TestSecond --test-suffix Tests.php test/
PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 17 ms, Memory: 6.00 MB

OK (1 test, 1 assertion)

You only need the --test-suffix argument as the default suffix is Test.php

@sebastianbergmann
Copy link
Owner

TBH, there should only ever be one (test) class declared in a file. I think I will deprecate support for multiple test case classes declared in a source file in PHPUnit 9 and remove support for it in PHPUnit 10. This should make things even simpler.

@realFlowControl
Copy link
Sponsor Contributor

In master it is already working like what you said when calling PHPUnit with a filename like this:

./phpunit --no-configuration MyTests.php

This will only succeed if there is a class named MyTests in the file, that extends from PHPUnit\Framework\TestCase.

@sebastianbergmann
Copy link
Owner

Even better. :)

@sebastianbergmann sebastianbergmann added the feature/test-runner CLI test runner label Feb 11, 2020
@bzixilu
Copy link

bzixilu commented Feb 20, 2020

Hi all, we have applied the suggestion with --test-suffix and found another problem:

In the case when test class is named following the conventions ( https://phpunit.readthedocs.io/en/9.0/writing-tests-for-phpunit.html) and located in the file named as FirstTest.php:

<?php

use PHPUnit\Framework\TestCase;

class FirstTest extends TestCase
{
    public function testOne()
    {
        static::assertTrue(true);
    }
}

class SecondTest extends TestCase
{
    public function testTwo()
    {
        $this->assertTrue(false);
    }
}

Output of the following command is still "No tests executed!"

/usr/bin/php ../vendor/phpunit/phpunit/phpunit --no-configuration --filter "/(SecondTest::testTwo)( .*)?$/" --test-suffix FirstTest.php 2

Could you please advise me on how I can avoid such behavior.

@realFlowControl
Copy link
Sponsor Contributor

Hey @bzixilu,

I assume the 2 at the end is a mistake. Which PHPUnit version are you using?

/Flo

@bzixilu
Copy link

bzixilu commented Feb 20, 2020

@flow-control sorry, forgot to mention it, 2 is the name of the folder where tests are located

I've tried a few PHPUnit versions: 8.5.2, 7.5.0, 9.0.0

@realFlowControl
Copy link
Sponsor Contributor

realFlowControl commented Feb 20, 2020

Hi again @bzixilu,

prior to PHPUnit v9 you could invoke PHPUnit with className FileName.php or only className or directory. Invoking with Filename.php directory should never have worked (at least I think so). You could just leave away the directory and call like this:

/usr/bin/php ../vendor/phpunit/phpunit/phpunit --no-configuration --filter "/(SecondTest::testTwo)( .*)?$/" FirstTest.php

This should work up to v8.5.2, but only due to some tricky fallback mechanism in the StandardTestSuiteLoader which has been refactored for v9 (to only accept one test case class thats named like the filename) and will be deprecated by v9.1. In v9.0 you should call with only the path to the tests without filename to make this work:

/usr/bin/php ../vendor/phpunit/phpunit/phpunit --no-configuration --filter "/(SecondTest::testTwo)( .*)?$/" /path/to/test/2/

(this is using another test case class loader which still uses this fallback)

For PHPUnit v10 the option to have test case class names different from the filename will be removed, it is already removed for the test case loaded used when invoked with filename (thats why it is not working for you) and i will be deprecated via #4105 for the rest.

So your best bet is to refactor your test suite to have only on test case class per file.

Hope I could help

/Flo

@realFlowControl
Copy link
Sponsor Contributor

realFlowControl commented Feb 20, 2020

I see the problem ... I will check later today, maybe i find something to help you with this. But just as a reminder: the possibility for multiple test case classes in a single file and naming the test case class different from the filename is/will be deprecated and will be removed vor v10.

Further info about this at #4105, #4039, #3859 and #4012

@bzixilu
Copy link

bzixilu commented Feb 20, 2020

@flow-control Thank you for the support!
To make the things clear, we are trying to support PHPUnit 9 in PhpStorm (WI-51010 Can't run non-first test class in file) without branching (before PHPUnit 9, PHPUnit 9+)
So, unfortunately, in our case, it's not possible to change the test suite.

@realFlowControl
Copy link
Sponsor Contributor

realFlowControl commented Feb 20, 2020

Hey @bzixilu,

I have looked it up and the only way to have this working was prior to PHPUnit v9 when using the old CLI way phpunit --no-configuration SecondTest 2/FirstTest.php. Even phpunit --no-configuration 2/ is not working (tested back till v8.2.5) because there is a FirstTest Class in that file. If there is a test case class in that file named like the filename (either full, or as in PEAR/PSR-0 prefixed short) the fallback loading all test case classes from a given file will not be called.

When you change the FirstTest.php to look like this:

<?php

use PHPUnit\Framework\TestCase;

class FooFirstTest extends TestCase
{
    public function testOne()
    {
        static::assertTrue(true);
    }
}

class SecondTest extends TestCase
{
    public function testTwo()
    {
        $this->assertTrue(false);
    }
}

then your command works as expected:

$ ./phpunit --no-configuration --filter "/(SecondTest::testTwo)( .*)?$/" --test-suffix FirstTest.php 2
PHPUnit 8.2.5 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 38 ms, Memory: 6.00 MB

There was 1 failure:

1) SecondTest::testTwo
Failed asserting that false is true.

/home/bowman/Work/flowcontrol/phpunit/2/FirstTest.php:17

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

With the branch deprecate-old-test-suite-loader-4105 from #4105 you would see this:

$ ./phpunit --no-configuration --filter "/(SecondTest::testTwo)( .*)?$/" --test-suffix FirstTest.php 2
PHPUnit 9.0-gb87137081 by Sebastian Bergmann and contributors.

Warning: Test case class not matching filename is deprecated
         in /home/bowman/Work/flowcontrol/phpunit/2/FirstTest.php
         class name was 'FooFirstTest', expected 'FirstTest', see #4105
Warning: Test case class not matching filename is deprecated
         in /home/bowman/Work/flowcontrol/phpunit/2/FirstTest.php
         class name was 'SecondTest', expected 'FirstTest', see #4105
Warning: Multiple test case classes per file is deprecated
         in /home/bowman/Work/flowcontrol/phpunit/2/FirstTest.php, see #4105

F                                                                   1 / 1 (100%)

Time: 45 ms, Memory: 6.00 MB

There was 1 failure:

1) SecondTest::testTwo
Failed asserting that false is true.

/home/bowman/Work/flowcontrol/phpunit/2/FirstTest.php:17

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

So, I know you can't change it, but this means, that as stated in the ChangeLog-9.0.md it is from PHPUnit v9.0 not possible anymore to have multiple test case in a single class. Only exception is if PHPUnit is invoked with a directory and there is no test case class matching the filename in that test file, so that the fallback triggers.

With PHPUnit v10 this fallback will be removed.

@sebastianbergmann should we do something about this? This not a bug, but the only trick that allowed to run the test case SecondClass in a file called FirstClass.php with also a FirstClass test class in it was deprecated in v8.5 and now removed. I guess no 😉

/Flo

@sebastianbergmann
Copy link
Owner

I do not think that there is anything we should do here but better communicate what was deprecated in PHPUnit 8.5, what was removed in PHPUnit 9.0, what is deprecated now, and what will be removed in PHPUnit 10.0.

@flobee
Copy link

flobee commented Feb 17, 2021

Please give me some hints about "Test case class not matching filename is deprecated" This does not make sence for me for a test tool.
A link to the docs or so. i just end up in this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-runner CLI test runner type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

5 participants