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

Implement filtering tests based on XML input file #4449

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion .psalm/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,16 @@
<code>$this-&gt;filter</code>
</PossiblyNullArgument>
</file>
<file src="src/Runner/Filter/XmlTestsIterator.php">
<MissingThrowsDocblock occurrences="1">
<code>getName</code>
</MissingThrowsDocblock>
</file>
<file src="src/Runner/Hook/TestListenerAdapter.php">
<DeprecatedInterface occurrences="1">
<code>TestListenerAdapter</code>
</DeprecatedInterface>
</file>
<file src="src/Runner/PhptTestCase.php">
<InternalClass occurrences="2">
<code>RawCodeCoverageData::fromXdebugWithoutPathCoverage([])</code>
Expand Down Expand Up @@ -889,7 +899,8 @@
<code>$e</code>
</InvalidArgument>
<InvalidStringClass occurrences="1"/>
<MissingThrowsDocblock occurrences="11">
<MissingThrowsDocblock occurrences="12">
<code>addFilter</code>
<code>addFilter</code>
<code>addFilter</code>
<code>addFilter</code>
Expand Down
146 changes: 146 additions & 0 deletions src/Runner/Filter/XmlTestsIterator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
<?php declare(strict_types=1);
/*
* This file is part of PHPUnit.
*
* (c) Sebastian Bergmann <sebastian@phpunit.de>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace PHPUnit\Runner\Filter;

use DOMElement;
use DOMXPath;
use Exception;
use Generator;
use PHPUnit\Framework\TestCase;
use PHPUnit\Framework\TestSuite;
use PHPUnit\Runner\PhptTestCase;
use PHPUnit\Util\Xml\Loader as XmlLoader;
use RecursiveFilterIterator;
use RecursiveIterator;

/**
* @internal This class is not covered by the backward compatibility promise for PHPUnit
*/
final class XmlTestsIterator extends RecursiveFilterIterator
{
/**
* The filter is used as a fast look for
* - the class name
* - the method name plus its optional data set description.
*
* Example: `filter[class name][method name + data set] = true;`
*
* The accept() method then can use a fast isset() to check if a test should
* be included or not.
*
* This works equally for phpt tests, except we hardcode the class name.
*
* @var array<string,array<string,true>>
*/
private array $filter;

/**
* @throws \PHPUnit\Util\Xml\Exception
*
* @return array<string,array<string,true>>
*/
public static function createFilterFromXmlFile(string $xmlFile): array
{
$xml = (new XmlLoader())->loadFile($xmlFile);
$xpath = new DOMXPath($xml);
$filter = [];

foreach (self::extractTestCases($xpath) as [$className, $methodName, $dataSet]) {
if (!isset($filter[$className])) {
$filter[$className] = [];
}

if (!$dataSet) {
$filter[$className][$methodName] = true;

continue;
}

$name = "{$methodName} with data set {$dataSet}";
$filter[$className][$name] = true;
}

foreach (self::extractPhptFile($xpath) as $path) {
$filter[PhptTestCase::class][$path] = true;
}

return $filter;
}

/**
* @throws Exception
*/
public function __construct(RecursiveIterator $iterator, array $filter)
{
parent::__construct($iterator);

$this->filter = $filter;
}

public function accept(): bool
{
$test = $this->getInnerIterator()->current();

if ($test instanceof TestSuite) {
return true;
}

/** @var TestCase $test */
$testClass = get_class($test);
Copy link
Contributor

@epdenouden epdenouden Oct 13, 2020

Choose a reason for hiding this comment

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

There is a way to ask anything that implements a Test its own unique identity, have a look at https://github.com/sebastianbergmann/phpunit/blob/master/src/Framework/Reorderable.php and the uses of Reorderable.

If you need more details about the (origins of) a test, let me know the use cases. It would be best if we can extend a central mechanism for identifying and locating tests.

Copy link
Author

Choose a reason for hiding this comment

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

I want to concentrate on this part first here before solving the others, might have an impact.

When a filter receives a test class in accept(), I've to figure out ifs part of the desired filter, i.e. the XML being feed back to phpunit.

The format of the XML is defined via \PHPUnit\Util\XmlTestListRenderer::render

if (get_class($test) !== $currentTestCase) {

and creates a XML structure like this:

<?xml version="1.0"?>
<tests>
 <testCaseClass name="PHPUnit\Framework\FunctionsTest">
  <testCaseMethod name="testGlobalFunctionsFileContainsAllStaticAssertions" groups="default" dataSet="&quot;assertArrayHasKey&quot;"/>
…
 </testCaseClass>
 <phptFile path="/absolute/path/phpunit/tests/end-to-end/abstract-test-class.phpt"/>
…
</tests>

And this now explains why I'm using get_class() and not something else: as a consumer of the XML, I've to match the producer.

This is, for \PHPUnit\Framework\TestCase, further stipulated with the dataSet-attribute, it's produced using this code

if (!empty($test->getDataSetAsString(false))) {
$writer->writeAttribute(
'dataSet',
str_replace(
' with data set ',
'',
$test->getDataSetAsString(false)
)
);
}

As can be seen, this manually removes some parts of \PHPUnit\Framework\TestCase::getDataSetAsString and writes it to the XML.

For that reason, when reading the XML I'm reconstructing the original "data as string" with this:

            $name                            = "{$methodName} with data set {$dataSet}";
            $this->filter[$className][$name] = true;

so that in accept() I can just call

        $name = $test->getName();

and getName internally calls getDataSetAsString, so that in the end by calling:

  • $testClass = get_class($test); in accept()
    and doing
  • "{$methodName} with data set {$dataSet} in setFilter
    I've built the matching mirror logic for consuming what was produced.

I guess some of this "peekaboo" here is reflected in #4449 (comment)


So currently I don't see how e.g \PHPUnit\Framework\TestCase::sortId (of \PHPUnit\Framework\Reorderable) helps me here, as the generated value is not usable in the context of what is produced in the XML

As for the format used in the \PHPUnit\Runner\Filter\XmlTestsIterator::$filter, I tried to document it:

    /**
     * The filter is used as a fast look for
     * - the class name
     * - the method name plus its optional data set description.
     *
     * Example: `filter[class name][method name + data set] = true;`
     *
     * The accept() method then can use a fast isset() to check if a test should
     * be included or not.
     *
     * This works equally for phpt tests, except we hardcode the class name.
     *
     * @var array<string,array<string,true>>
     */


if (!isset($this->filter[$testClass])) {
return false;
}

$name = $test->getName();

return isset($this->filter[$testClass][$name]);
}

private static function extractTestCases(DOMXPath $xpath): Generator
{
/** @var DOMElement $class */
foreach ($xpath->evaluate('/tests/testCaseClass') as $class) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as loading the XML-file for every new Iterator: please do this once and use a centralized lookup.

Copy link
Author

@mfn mfn Dec 9, 2020

Choose a reason for hiding this comment

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

$className = $class->getAttribute('name');

if (!$className) {
continue;
}

/** @var DOMElement $method */
foreach ($xpath->evaluate('testCaseMethod', $class) as $method) {
$methodName = $method->getAttribute('name');

if (!$methodName) {
continue;
}

$dataSet = $method->getAttribute('dataSet');

yield [$className, $methodName, $dataSet];
}
}
}

/**
* @return Generator<string>
*/
private static function extractPhptFile(DOMXPath $xpath): Generator
{
/* @var DOMElement $phptFile */
foreach ($xpath->evaluate('/tests/phptFile') as $phptFile) {
$path = $phptFile->getAttribute('path');
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why does CodeCov mark these lines as not-reached?


if ($path) {
yield $path;
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why does CodeCov mark these lines as not-reached?

}
}
}
}
10 changes: 9 additions & 1 deletion src/TextUI/CliArguments/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ final class Builder
'testsuite=',
'verbose',
'version',
'tests-xml=',
];

private const SHORT_OPTIONS = 'd:c:hv';
Expand Down Expand Up @@ -225,6 +226,7 @@ public function fromParameters(array $parameters, array $additionalLongOptions):
$useDefaultConfiguration = null;
$verbose = null;
$version = null;
$testsXml = null;

if (isset($options[1][0])) {
$argument = $options[1][0];
Expand Down Expand Up @@ -752,6 +754,11 @@ public function fromParameters(array $parameters, array $additionalLongOptions):

break;

case '--tests-xml':
$testsXml = $option[1];

break;

default:
$unrecognizedOptions[str_replace('--', '', $option[0])] = $option[1];
}
Expand Down Expand Up @@ -862,7 +869,8 @@ public function fromParameters(array $parameters, array $additionalLongOptions):
$useDefaultConfiguration,
$verbose,
$version,
$coverageFilter
$coverageFilter,
$testsXml
);
}
}
22 changes: 21 additions & 1 deletion src/TextUI/CliArguments/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,12 @@ final class Configuration

private ?bool $version = null;

private ?string $testsXml;

/**
* @param null|int|string $columns
*/
public function __construct(?string $argument, ?string $atLeastVersion, ?bool $backupGlobals, ?bool $backupStaticAttributes, ?bool $beStrictAboutChangesToGlobalState, ?bool $beStrictAboutResourceUsageDuringSmallTests, ?string $bootstrap, ?bool $cacheResult, ?string $cacheResultFile, ?bool $checkVersion, ?string $colors, $columns, ?string $configuration, ?string $coverageClover, ?string $coverageCobertura, ?string $coverageCrap4J, ?string $coverageHtml, ?string $coveragePhp, ?string $coverageText, ?bool $coverageTextShowUncoveredFiles, ?bool $coverageTextShowOnlySummary, ?string $coverageXml, ?bool $pathCoverage, ?string $coverageCacheDirectory, ?bool $warmCoverageCache, ?bool $debug, ?int $defaultTimeLimit, ?bool $disableCodeCoverageIgnore, ?bool $disallowTestOutput, ?bool $disallowTodoAnnotatedTests, ?bool $enforceTimeLimit, ?array $excludeGroups, ?int $executionOrder, ?int $executionOrderDefects, ?array $extensions, ?array $unavailableExtensions, ?bool $failOnEmptyTestSuite, ?bool $failOnIncomplete, ?bool $failOnRisky, ?bool $failOnSkipped, ?bool $failOnWarning, ?string $filter, ?bool $generateConfiguration, ?bool $migrateConfiguration, ?array $groups, ?array $testsCovering, ?array $testsUsing, ?bool $help, ?string $includePath, ?array $iniSettings, ?string $junitLogfile, ?bool $listGroups, ?bool $listSuites, ?bool $listTests, ?string $listTestsXml, ?bool $noCoverage, ?bool $noExtensions, ?bool $noInteraction, ?bool $noLogging, ?string $printer, ?bool $processIsolation, ?int $randomOrderSeed, ?int $repeat, ?bool $reportUselessTests, ?bool $resolveDependencies, ?bool $reverseList, ?bool $stderr, ?bool $strictCoverage, ?bool $stopOnDefect, ?bool $stopOnError, ?bool $stopOnFailure, ?bool $stopOnIncomplete, ?bool $stopOnRisky, ?bool $stopOnSkipped, ?bool $stopOnWarning, ?string $teamcityLogfile, ?array $testdoxExcludeGroups, ?array $testdoxGroups, ?string $testdoxHtmlFile, ?string $testdoxTextFile, ?string $testdoxXmlFile, ?array $testSuffixes, ?string $testSuite, array $unrecognizedOptions, ?string $unrecognizedOrderBy, ?bool $useDefaultConfiguration, ?bool $verbose, ?bool $version, ?array $coverageFilter)
public function __construct(?string $argument, ?string $atLeastVersion, ?bool $backupGlobals, ?bool $backupStaticAttributes, ?bool $beStrictAboutChangesToGlobalState, ?bool $beStrictAboutResourceUsageDuringSmallTests, ?string $bootstrap, ?bool $cacheResult, ?string $cacheResultFile, ?bool $checkVersion, ?string $colors, $columns, ?string $configuration, ?string $coverageClover, ?string $coverageCobertura, ?string $coverageCrap4J, ?string $coverageHtml, ?string $coveragePhp, ?string $coverageText, ?bool $coverageTextShowUncoveredFiles, ?bool $coverageTextShowOnlySummary, ?string $coverageXml, ?bool $pathCoverage, ?string $coverageCacheDirectory, ?bool $warmCoverageCache, ?bool $debug, ?int $defaultTimeLimit, ?bool $disableCodeCoverageIgnore, ?bool $disallowTestOutput, ?bool $disallowTodoAnnotatedTests, ?bool $enforceTimeLimit, ?array $excludeGroups, ?int $executionOrder, ?int $executionOrderDefects, ?array $extensions, ?array $unavailableExtensions, ?bool $failOnEmptyTestSuite, ?bool $failOnIncomplete, ?bool $failOnRisky, ?bool $failOnSkipped, ?bool $failOnWarning, ?string $filter, ?bool $generateConfiguration, ?bool $migrateConfiguration, ?array $groups, ?array $testsCovering, ?array $testsUsing, ?bool $help, ?string $includePath, ?array $iniSettings, ?string $junitLogfile, ?bool $listGroups, ?bool $listSuites, ?bool $listTests, ?string $listTestsXml, ?bool $noCoverage, ?bool $noExtensions, ?bool $noInteraction, ?bool $noLogging, ?string $printer, ?bool $processIsolation, ?int $randomOrderSeed, ?int $repeat, ?bool $reportUselessTests, ?bool $resolveDependencies, ?bool $reverseList, ?bool $stderr, ?bool $strictCoverage, ?bool $stopOnDefect, ?bool $stopOnError, ?bool $stopOnFailure, ?bool $stopOnIncomplete, ?bool $stopOnRisky, ?bool $stopOnSkipped, ?bool $stopOnWarning, ?string $teamcityLogfile, ?array $testdoxExcludeGroups, ?array $testdoxGroups, ?string $testdoxHtmlFile, ?string $testdoxTextFile, ?string $testdoxXmlFile, ?array $testSuffixes, ?string $testSuite, array $unrecognizedOptions, ?string $unrecognizedOrderBy, ?bool $useDefaultConfiguration, ?bool $verbose, ?bool $version, ?array $coverageFilter, ?string $testsXml)
{
$this->argument = $argument;
$this->atLeastVersion = $atLeastVersion;
Expand Down Expand Up @@ -293,6 +295,7 @@ public function __construct(?string $argument, ?string $atLeastVersion, ?bool $b
$this->useDefaultConfiguration = $useDefaultConfiguration;
$this->verbose = $verbose;
$this->version = $version;
$this->testsXml = $testsXml;
}

public function hasArgument(): bool
Expand Down Expand Up @@ -1795,4 +1798,21 @@ public function version(): bool

return $this->version;
}

public function hasTestsXml(): bool
{
return $this->testsXml !== null;
}

/**
* @throws Exception
*/
public function testsXml(): string
{
if ($this->testsXml === null) {
throw new Exception;
}

return $this->testsXml;
}
}
4 changes: 4 additions & 0 deletions src/TextUI/CliArguments/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ public function mapToLegacyArray(Configuration $arguments): array
$result['randomOrderSeed'] = $arguments->randomOrderSeed();
}

if ($arguments->hasTestsXml()) {
$result['testsXml'] = $arguments->testsXml();
}

return $result;
}
}
1 change: 1 addition & 0 deletions src/TextUI/Help.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ final class Help
['arg' => '--list-tests', 'desc' => 'List available tests'],
['arg' => '--list-tests-xml <file>', 'desc' => 'List available tests in XML format'],
['arg' => '--filter <pattern>', 'desc' => 'Filter which tests to run'],
['arg' => '--tests-xml <file>', 'desc' => 'Filter which tests to run in XML format'],
mfn marked this conversation as resolved.
Show resolved Hide resolved
['arg' => '--test-suffix <suffixes>', 'desc' => 'Only search for test in files with specified suffix(es). Default: Test.php,.phpt'],
],

Expand Down
15 changes: 14 additions & 1 deletion src/TextUI/TestRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use PHPUnit\Runner\Filter\Factory;
use PHPUnit\Runner\Filter\IncludeGroupFilterIterator;
use PHPUnit\Runner\Filter\NameFilterIterator;
use PHPUnit\Runner\Filter\XmlTestsIterator;
use PHPUnit\Runner\Hook;
use PHPUnit\Runner\NullTestResultCache;
use PHPUnit\Runner\ResultCacheExtension;
Expand Down Expand Up @@ -113,6 +114,7 @@ public function __construct(CodeCoverageFilter $filter = null)
/**
* @throws \PHPUnit\Runner\Exception
* @throws \PHPUnit\TextUI\XmlConfiguration\Exception
* @throws \PHPUnit\Util\Xml\Exception
* @throws Exception
*/
public function run(TestSuite $suite, array $arguments = [], array $warnings = [], bool $exit = true): TestResult
Expand Down Expand Up @@ -1046,13 +1048,17 @@ private function handleConfiguration(array &$arguments): void
$arguments['verbose'] = $arguments['verbose'] ?? false;
}

/**
* @throws \PHPUnit\Util\Xml\Exception
*/
private function processSuiteFilters(TestSuite $suite, array $arguments): void
{
if (!$arguments['filter'] &&
empty($arguments['groups']) &&
empty($arguments['excludeGroups']) &&
empty($arguments['testsCovering']) &&
empty($arguments['testsUsing'])) {
empty($arguments['testsUsing']) &&
empty($arguments['testsXml'])) {
return;
}

Expand Down Expand Up @@ -1103,6 +1109,13 @@ static function (string $name): string {
);
}

if (!empty($arguments['testsXml'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Around here is the place to load+parse your configuration and get a list of tests. Perhaps you can even adapt/reuse the current NameFilterIterator, as it's basically what it is. :)

$filterFactory->addFilter(
new ReflectionClass(XmlTestsIterator::class),
XmlTestsIterator::createFilterFromXmlFile($arguments['testsXml']),
);
}

$suite->injectFilter($filterFactory);
}

Expand Down
1 change: 1 addition & 0 deletions tests/end-to-end/cli/_files/output-cli-help-color.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
--list-tests  List available tests
--list-tests-xml <file>  List available tests in XML format
--filter <pattern>  Filter which tests to run
--tests-xml <file>  Filter which tests to run in XML format
--test-suffix <suffixes>  Only search for test in files with
specified suffix(es). Default:
Test.php,.phpt
Expand Down
1 change: 1 addition & 0 deletions tests/end-to-end/cli/_files/output-cli-usage.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Test Selection Options:
--list-tests List available tests
--list-tests-xml <file> List available tests in XML format
--filter <pattern> Filter which tests to run
--tests-xml <file> Filter which tests to run in XML format
--test-suffix <suffixes> Only search for test in files with specified suffix(es). Default: Test.php,.phpt

Test Execution Options:
Expand Down
48 changes: 48 additions & 0 deletions tests/end-to-end/tests-xml-dataprovider.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
--TEST--
phpunit --list-tests-xml ../../_files/DataProviderTest.php
--FILE--
<?php declare(strict_types=1);
$xml = tempnam(sys_get_temp_dir(), __FILE__);
file_put_contents($xml, <<<XML
<?xml version="1.0"?>
<tests>
<!-- This class exists -->
<testCaseClass name="PHPUnit\TestFixture\DataProviderTest">
<testCaseMethod name="testAdd" groups="default" dataSet="#0"/>
<!-- This method does not exist -->
<testCaseMethod name="methodDoesNotExist"/>
<!-- name attribute missing -->
<testCaseMethod />
</testCaseClass>

<!-- name attribute missing -->
<testCaseClass>
<testCaseMethod name="testAdd" groups="default" dataSet="#0"/>
</testCaseClass>

<ignoredTag/>

<testCaseClass name="Class\Does\Not\Exist">
<testCaseMethod name="methodAlsoDoesNotExist"/>
</testCaseClass>
</tests>
XML
);

$_SERVER['argv'][1] = '--no-configuration';
$_SERVER['argv'][2] = '--tests-xml';
$_SERVER['argv'][3] = $xml;
$_SERVER['argv'][4] = __DIR__ . '/../_files/DataProviderTest.php';

require __DIR__ . '/../bootstrap.php';
PHPUnit\TextUI\Command::main(false);

unlink($xml);
--EXPECTF--
PHPUnit %s by Sebastian Bergmann and contributors.

. 1 / 1 (100%)

Time: %s, Memory: %s

OK (1 test, 1 assertion)