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

All-day breakfast at the dataprovider buffet #3401

Closed
wants to merge 10 commits into from
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ insert_final_newline = true
indent_style = space
indent_size = 4
charset = utf-8

[tests/_files/*_result_cache.txt]
insert_final_newline = false
4 changes: 2 additions & 2 deletions src/Runner/ResultCacheExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ private function getTestName(string $test): string
{
$matches = [];

if (\preg_match('/^(?:\S+::)?(?<name>\S+)(?:(?<data> with data set (?:#\d+|"[^"]+"))\s\()?/', $test, $matches)) {
$test = $matches['name'] . ($matches['data'] ?? '');
if (\preg_match('/^(?<name>\S+::\S+)(?:(?<dataname> with data set (?:#\d+|"[^"]+"))\s\()?/', $test, $matches)) {
$test = $matches['name'] . ($matches['dataname'] ?? '');
}

return $test;
Expand Down
30 changes: 14 additions & 16 deletions src/Runner/TestSuiteSorter.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,11 @@ private function addSuiteToDefectSortOrder(TestSuite $suite): void
$max = 0;

foreach ($suite->tests() as $test) {
if (!isset($this->defectSortOrder[$test->getName()])) {
$this->defectSortOrder[$test->getName()] = self::DEFECT_SORT_WEIGHT[$this->cache->getState($test->getName())];
$max = \max($max, $this->defectSortOrder[$test->getName()]);
$testname = $this->getNormalizedTestName($test);

if (!isset($this->defectSortOrder[$testname])) {
$this->defectSortOrder[$testname] = self::DEFECT_SORT_WEIGHT[$this->cache->getState($testname)];
$max = \max($max, $this->defectSortOrder[$testname]);
}
}

Expand Down Expand Up @@ -203,12 +205,8 @@ function ($left, $right) {
*/
private function cmpDefectPriorityAndTime(Test $a, Test $b): int
{
if (!$a instanceof TestCase || !$b instanceof TestCase) {
return 0;
}

$priorityA = $this->defectSortOrder[$a->getName()] ?? 0;
$priorityB = $this->defectSortOrder[$b->getName()] ?? 0;
$priorityA = $this->defectSortOrder[$this->getNormalizedTestName($a)] ?? 0;
$priorityB = $this->defectSortOrder[$this->getNormalizedTestName($b)] ?? 0;

if ($priorityB <=> $priorityA) {
// Sort defect weight descending
Expand All @@ -228,11 +226,7 @@ private function cmpDefectPriorityAndTime(Test $a, Test $b): int
*/
private function cmpDuration(Test $a, Test $b): int
{
if (!$a instanceof TestCase || !$b instanceof TestCase) {
return 0;
}

return $this->cache->getTime($a->getName()) <=> $this->cache->getTime($b->getName());
return $this->cache->getTime($this->getNormalizedTestName($a)) <=> $this->cache->getTime($this->getNormalizedTestName($b));
}

/**
Expand Down Expand Up @@ -281,11 +275,15 @@ function ($test) {
*/
private function getNormalizedTestName($test): string
{
if (\strpos($test->getName(), '::') !== false) {
if ($test instanceof PhptTestCase) {
return $test->getName();
}

return \get_class($test) . '::' . $test->getName();
if (\strpos($test->getName(), '::') !== false) {
return $test->getName(true);
}

return \get_class($test) . '::' . $test->getName(true);
}

/**
Expand Down
48 changes: 48 additions & 0 deletions tests/_files/DataproviderExecutionOrderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
/*
* 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.
*/
use PHPUnit\Framework\TestCase;

class DataproviderExecutionOrderTest extends TestCase
{
public function testFirstTestThatAlwaysWorks()
{
$this->assertTrue(true);
}

/**
* @dataProvider dataproviderAdditions
*/
public function testAddNumbersWithADataprovider(int $a, int $b, int $sum)
{
$this->assertSame($sum, $a + $b);
}

public function testTestInTheMiddleThatAlwaysWorks()
{
$this->assertTrue(true);
}

/**
* @dataProvider dataproviderAdditions
*/
public function testAddMoreNumbersWithADataprovider(int $a, int $b, int $sum)
{
$this->assertSame($sum, $a + $b);
}

public function dataproviderAdditions()
{
return [
'1+2=3' => [1, 2, 3],
'2+1=3' => [2, 1, 3],
'1+1=3' => [1, 1, 3],
];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
C:30:"PHPUnit\Runner\TestResultCache":2119:{a:2:{s:7:"defects";a:5:{s:60:"tests/end-to-end/regression/GitHub/3396/issue-3396-test.phpt";i:3;s:88:"MultiDependencyExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+1=3"";i:3;s:92:"MultiDependencyExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+1=3"";i:3;s:85:"DataproviderExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+1=3"";i:3;s:89:"DataproviderExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+1=3"";i:3;}s:5:"times";a:17:{s:60:"tests/end-to-end/regression/GitHub/3396/issue-3396-test.phpt";d:0.115;s:63:"MultiDependencyExecutionOrderTest::testFirstTestThatAlwaysWorks";d:0;s:88:"MultiDependencyExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+2=3"";d:0;s:88:"MultiDependencyExecutionOrderTest::testAddNumbersWithADataprovider with data set "2+1=3"";d:0;s:88:"MultiDependencyExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+1=3"";d:0.003;s:69:"MultiDependencyExecutionOrderTest::testTestInTheMiddleThatAlwaysWorks";d:0;s:92:"MultiDependencyExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+2=3"";d:0;s:92:"MultiDependencyExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "2+1=3"";d:0;s:92:"MultiDependencyExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+1=3"";d:0;s:60:"DataproviderExecutionOrderTest::testFirstTestThatAlwaysWorks";d:0.002;s:85:"DataproviderExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+2=3"";d:0;s:85:"DataproviderExecutionOrderTest::testAddNumbersWithADataprovider with data set "2+1=3"";d:0;s:85:"DataproviderExecutionOrderTest::testAddNumbersWithADataprovider with data set "1+1=3"";d:0.001;s:66:"DataproviderExecutionOrderTest::testTestInTheMiddleThatAlwaysWorks";d:0;s:89:"DataproviderExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+2=3"";d:0;s:89:"DataproviderExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "2+1=3"";d:0;s:89:"DataproviderExecutionOrderTest::testAddMoreNumbersWithADataprovider with data set "1+1=3"";d:0;}}}
2 changes: 1 addition & 1 deletion tests/_files/MultiDependencyTest_result_cache.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
C:30:"PHPUnit\Runner\TestResultCache":157:{a:2:{s:7:"defects";a:1:{s:8:"testFive";i:1;}s:5:"times";a:5:{s:7:"testOne";d:0;s:7:"testTwo";d:0;s:9:"testThree";d:0;s:8:"testFour";d:0;s:8:"testFive";d:0;}}}
C:30:"PHPUnit\Runner\TestResultCache":289:{a:2:{s:7:"defects";a:1:{s:29:"MultiDependencyTest::testFive";i:1;}s:5:"times";a:5:{s:28:"MultiDependencyTest::testOne";d:0;s:28:"MultiDependencyTest::testTwo";d:0;s:30:"MultiDependencyTest::testThree";d:0;s:29:"MultiDependencyTest::testFour";d:0;s:29:"MultiDependencyTest::testFive";d:0;}}}
2 changes: 1 addition & 1 deletion tests/end-to-end/cache-result.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ Time: %s, Memory: %s

OK, but incomplete, skipped, or risky tests!
Tests: 5, Assertions: 3, Skipped: 2.
C:30:"PHPUnit\Runner\TestResultCache":%d:{a:2:{s:7:"defects";a:2:{s:8:"testFour";i:1;s:9:"testThree";i:1;}s:5:"times";a:5:{s:8:"testFive";d:%f;s:8:"testFour";d:%f;s:9:"testThree";d:%f;s:7:"testTwo";d:%f;s:7:"testOne";d:%f;}}}
C:30:"PHPUnit\Runner\TestResultCache":%d:{a:2:{s:7:"defects";a:2:{s:29:"MultiDependencyTest::testFour";i:1;s:30:"MultiDependencyTest::testThree";i:1;}s:5:"times";a:5:{s:29:"MultiDependencyTest::testFive";d:%f;s:29:"MultiDependencyTest::testFour";d:%f;s:30:"MultiDependencyTest::testThree";d:%f;s:28:"MultiDependencyTest::testTwo";d:%f;s:28:"MultiDependencyTest::testOne";d:%f;}}}
41 changes: 41 additions & 0 deletions tests/end-to-end/regression/GitHub/3396/issue-3396-test.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
https://github.com/sebastianbergmann/phpunit/issues/3396
--FILE--
<?php
$_SERVER['argv'][1] = '--no-configuration';
$_SERVER['argv'][2] = '--order-by=defects';
$_SERVER['argv'][3] = '--testdox';
$_SERVER['argv'][4] = \dirname(\dirname(\dirname(__DIR__))) . '/../_files/DataproviderExecutionOrderTest.php';

require __DIR__ . '/../../../../bootstrap.php';
PHPUnit\TextUI\Command::main();
--EXPECTF--
PHPUnit %s by Sebastian Bergmann and contributors.

DataproviderExecutionOrder
✔ First test that always works
✔ Add numbers with a dataprovider with data set "1+2=3"
✔ Add numbers with a dataprovider with data set "2+1=3"
✘ Add numbers with a dataprovider with data set "1+1=3"
│ Failed asserting that 2 is identical to 3.
│%w
│ %s%etests%e_files%eDataproviderExecutionOrderTest.php:24
│%w

✔ Test in the middle that always works
✔ Add more numbers with a dataprovider with data set "1+2=3"
✔ Add more numbers with a dataprovider with data set "2+1=3"
✘ Add more numbers with a dataprovider with data set "1+1=3"
│ Failed asserting that 2 is identical to 3.
│%w
│ %s%etests%e_files%eDataproviderExecutionOrderTest.php:37
│%w
%A
Time: %s, Memory: %s

Summary of non-successful tests:
%A
FAILURES!
Tests: 8, Assertions: 8, Failures: 2.
26 changes: 13 additions & 13 deletions tests/unit/Runner/ResultCacheExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ public function longTestNamesDataprovider(): array
{
return [
'ClassName::testMethod' => [
TestCaseTest::class . '::testSomething',
'testSomething', ],
'testSomething',
TestCaseTest::class . '::testSomething', ],
'ClassName::testMethod and data set number and vardump' => [
TestCaseTest::class . '::testMethod with data set #123 (\'a\', "A", 0, false)',
'testMethod with data set #123', ],
'testMethod with data set #123 (\'a\', "A", 0, false)',
TestCaseTest::class . '::testMethod with data set #123', ],
'ClassName::testMethod and data set name and vardump' => [
TestCaseTest::class . '::testMethod with data set "data name" (\'a\', "A\", 0, false)',
'testMethod with data set "data name"', ],
'testMethod with data set "data name" (\'a\', "A\", 0, false)',
TestCaseTest::class . '::testMethod with data set "data name"', ],
];
}

Expand All @@ -77,55 +77,55 @@ public function testError(): void
$test = new \TestError('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_ERROR, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_ERROR, $this->cache->getState(\TestError::class . '::test_name'));
}

public function testFailure(): void
{
$test = new \Failure('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_FAILURE, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_FAILURE, $this->cache->getState(\Failure::class . '::test_name'));
}

public function testSkipped(): void
{
$test = new \TestSkipped('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $this->cache->getState(\TestSkipped::class . '::test_name'));
}

public function testIncomplete(): void
{
$test = new \TestIncomplete('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_INCOMPLETE, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_INCOMPLETE, $this->cache->getState(\TestIncomplete::class . '::test_name'));
}

public function testPassedTestsOnlyCacheTime(): void
{
$test = new \Success('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $this->cache->getState(\Success::class . '::test_name'));
}

public function testWarning(): void
{
$test = new \TestWarning('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_WARNING, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_WARNING, $this->cache->getState(\TestWarning::class . '::test_name'));
}

public function testRisky(): void
{
$test = new \TestRisky('test_name');
$test->run($this->result);

$this->assertSame(BaseTestRunner::STATUS_RISKY, $this->cache->getState('test_name'));
$this->assertSame(BaseTestRunner::STATUS_RISKY, $this->cache->getState(\TestRisky::class . '::test_name'));
}

public function testEmptySuite(): void
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/Runner/TestSuiteSorterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function testOrderDurationWithCache(bool $resolveDependencies, array $tes
$cache = new TestResultCache;

foreach ($testTimes as $testName => $time) {
$cache->setTime($testName, $time);
$cache->setTime(\MultiDependencyTest::class . '::' . $testName, $time);
}

$sorter = new TestSuiteSorter($cache);
Expand Down Expand Up @@ -258,8 +258,8 @@ public function testSuiteSorterDefectsOptions(int $order, bool $resolveDependenc
$cache = new TestResultCache;

foreach ($runState as $testName => $data) {
$cache->setState($testName, $data['state']);
$cache->setTime($testName, $data['time']);
$cache->setState(\MultiDependencyTest::class . '::' . $testName, $data['state']);
$cache->setTime(\MultiDependencyTest::class . '::' . $testName, $data['time']);
}

$sorter = new TestSuiteSorter($cache);
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/Util/TestResultCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public function testReadsCacheFromProvidedFilename(): void
$cache = new TestResultCache($cacheFile);
$cache->load();

$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState('testOne'));
$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $cache->getState('testFive'));
$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState(\MultiDependencyTest::class . '::testOne'));
$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $cache->getState(\MultiDependencyTest::class . '::testFive'));
}

public function testDoesClearCacheBeforeLoad(): void
Expand All @@ -32,12 +32,12 @@ public function testDoesClearCacheBeforeLoad(): void
$cache = new TestResultCache($cacheFile);
$cache->setState('someTest', BaseTestRunner::STATUS_FAILURE);

$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState('testFive'));
$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState(\MultiDependencyTest::class . '::testFive'));

$cache->load();

$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState('someTest'));
$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $cache->getState('testFive'));
$this->assertSame(BaseTestRunner::STATUS_UNKNOWN, $cache->getState(\MultiDependencyTest::class . '::someTest'));
$this->assertSame(BaseTestRunner::STATUS_SKIPPED, $cache->getState(\MultiDependencyTest::class . '::testFive'));
}

public function testShouldNotSerializePassedTestsAsDefectButTimeIsStored(): void
Expand Down