@Depends file will be run if not ran yet + Full namespace bug when using @depends [FEATURE&BUG-FIX] #3449
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,11 @@ abstract class TestCase extends Assert implements Test, SelfDescribing | |
| { | ||
| private const LOCALE_CATEGORIES = [\LC_ALL, \LC_COLLATE, \LC_CTYPE, \LC_MONETARY, \LC_NUMERIC, \LC_TIME]; | ||
|
|
||
| private const DEPENDENCY_PREFIX = 'Dependency_'; | ||
|
|
||
| /** @var TestResult \PHPUnit\Framework\TestResult */ | ||
| private static $DEPENDENCY_TASK_RESULTS; | ||
|
|
||
| /** | ||
| * @var bool | ||
| */ | ||
|
|
@@ -383,6 +388,10 @@ public function __construct($name = null, array $data = [], $dataName = '') | |
| $this->setName($name); | ||
| } | ||
|
|
||
| if (!isset(self::$DEPENDENCY_TASK_RESULTS)) { | ||
| self::$DEPENDENCY_TASK_RESULTS = new TestResult; | ||
NikoGrano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| $this->data = $data; | ||
| $this->dataName = $dataName; | ||
| } | ||
|
|
@@ -1308,11 +1317,11 @@ protected function setLocale(...$args): void | |
| protected function createMock($originalClassName): MockObject | ||
| { | ||
| return $this->getMockBuilder($originalClassName) | ||
| ->disableOriginalConstructor() | ||
| ->disableOriginalClone() | ||
| ->disableArgumentCloning() | ||
| ->disallowMockingUnknownTypes() | ||
| ->getMock(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1346,12 +1355,12 @@ protected function createConfiguredMock($originalClassName, array $configuration | |
| protected function createPartialMock($originalClassName, array $methods): MockObject | ||
| { | ||
| return $this->getMockBuilder($originalClassName) | ||
| ->disableOriginalConstructor() | ||
| ->disableOriginalClone() | ||
| ->disableArgumentCloning() | ||
| ->disallowMockingUnknownTypes() | ||
| ->setMethods(empty($methods) ? null : $methods) | ||
| ->getMock(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1363,9 +1372,9 @@ protected function createPartialMock($originalClassName, array $methods): MockOb | |
| protected function createTestProxy(string $originalClassName, array $constructorArguments = []): MockObject | ||
| { | ||
| return $this->getMockBuilder($originalClassName) | ||
| ->setConstructorArgs($constructorArguments) | ||
| ->enableProxyingToOriginalMethods() | ||
| ->getMock(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1456,12 +1465,12 @@ protected function getMockFromWsdl($wsdlFile, $originalClassName = '', $mockClas | |
|
|
||
| if (!\class_exists($originalClassName)) { | ||
| eval( | ||
| $this->getMockObjectGenerator()->generateClassFromWsdl( | ||
| $wsdlFile, | ||
| $originalClassName, | ||
| $methods, | ||
| $options | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -1682,23 +1691,97 @@ private function verifyMockObjects(): void | |
| } | ||
| } | ||
|
|
||
| private function processFailed(array $failed): array | ||
| { | ||
| /** @var TestFailure $failure */ | ||
| foreach ($failed as $failure) { | ||
| $pos = \strpos($failure->getTestName(), ' with data set'); | ||
|
|
||
| if ($pos !== false) { | ||
| $failedKeys[] = \substr($failure->getTestName(), 0, $pos); | ||
| } else { | ||
| $failedKeys[] = $failure->getTestName(); | ||
| } | ||
| } | ||
|
|
||
| return \array_flip(\array_unique($failedKeys ?? [])); | ||
| } | ||
|
|
||
| private function processPassedKeys(array $passedKeys): array | ||
| { | ||
| foreach (\array_keys($passedKeys) as $key) { | ||
| $pos = \strpos($key, ' with data set'); | ||
|
|
||
| if ($pos !== false) { | ||
| $newPassedKeys[] = \substr($key, 0, $pos); | ||
| } | ||
| } | ||
|
|
||
| return \array_merge(\array_flip(\array_unique($newPassedKeys ?? [])), $passedKeys); | ||
| } | ||
|
|
||
| private function processPassed(array $passed): array | ||
| { | ||
| foreach (\array_reverse($passed) as $key => $val) { | ||
| $pos = \strpos($key, ' with data set'); | ||
|
|
||
| if ($pos !== false) { | ||
| $newPassed[\substr($key, 0, $pos)] = $val; | ||
| } | ||
| } | ||
|
|
||
| return \array_merge($passed, $newPassed ?? []); | ||
| } | ||
|
|
||
| private function processSkipped(array $skipped): array | ||
| { | ||
| /** @var TestFailure $skip */ | ||
| foreach ($skipped as $skip) { | ||
| $pos = \strpos($skip->getTestName(), ' with data set'); | ||
|
|
||
| if ($pos !== false) { | ||
| $skippedKeys[] = \substr($skip->getTestName(), 0, $pos); | ||
| } else { | ||
| $skippedKeys[] = $skip->getTestName(); | ||
| } | ||
| } | ||
|
|
||
| return \array_flip(\array_unique($skippedKeys ?? [])); | ||
| } | ||
|
|
||
| private function getSkippedFailedPassedKeys(&$skipped, &$failed, &$passedKeys, &$passed): void | ||
| { | ||
| $skipped = \array_merge($this->processSkipped($this->result->skipped()), $this->processSkipped(self::$DEPENDENCY_TASK_RESULTS->skipped())); | ||
| $failed = \array_merge($this->processFailed($this->result->failures()), $this->processFailed(self::$DEPENDENCY_TASK_RESULTS->failures())); | ||
| $passed = \array_merge($this->result->passed(), self::$DEPENDENCY_TASK_RESULTS->passed()); | ||
| $passedKeys = $this->processPassedKeys($passed); | ||
| $passed = $this->processPassed($passed); | ||
| } | ||
|
|
||
| private function setSkippedDependsOn(string $dependsOn): void | ||
| { | ||
| $this->status = BaseTestRunner::STATUS_SKIPPED; | ||
|
|
||
| $this->result->startTest($this); | ||
|
|
||
| $this->result->addError( | ||
| $this, | ||
| new SkippedTestError( | ||
| \sprintf( | ||
| 'This test depends on "%s" to pass.', | ||
| $dependsOn | ||
| ) | ||
| ), | ||
| 0 | ||
| ); | ||
| $this->result->endTest($this, 0); | ||
| } | ||
|
|
||
| private function handleDependencies(): bool | ||
| { | ||
| if (!empty($this->dependencies) && !$this->inIsolation) { | ||
| $className = \get_class($this); | ||
| $this->getSkippedFailedPassedKeys($skippedKeys, $failedKeys, $passedKeys, $passed); | ||
|
|
||
| foreach ($this->dependencies as $dependency) { | ||
| $deepClone = false; | ||
|
|
@@ -1724,31 +1807,68 @@ private function handleDependencies(): bool | |
| $dependency = $className . '::' . $dependency; | ||
| } | ||
|
|
||
| if (\strpos($dependency, '\\') === 0) { | ||
| $dependency = \substr($dependency, 1); | ||
| } | ||
|
|
||
| if (!isset($passedKeys[$dependency])) { | ||
| if ( | ||
| !isset($skippedKeys[$dependency]) | ||
| && !isset($failedKeys[$dependency]) | ||
| && \strpos($dependency, '::') > 0 | ||
| ) { | ||
| if ($className !== \explode('::', $dependency, 2)[0]) { | ||
| $dependencyClass = \explode('::', $dependency, 2)[0]; | ||
| $dependencyKey = self::DEPENDENCY_PREFIX . \ucfirst($dependencyClass); | ||
| $oldErrorsCount = self::$DEPENDENCY_TASK_RESULTS->errorCount(); | ||
| (new TestSuite($dependencyClass, $dependencyKey))->run(self::$DEPENDENCY_TASK_RESULTS); | ||
|
There was a problem hiding this comment. Before you put any more work into this, wait for a reply for @sebastianbergmann The design of this feature still worries me. Creating a new
There was a problem hiding this comment.
It will be ran twice, if it is not in correct order.
Not possible to run it over twice. The result will be put into dependency-only testResult in case it's required more than one time before it is ran in correct order.
I'll test this. Should not affect that. |
||
|
|
||
| if ($oldErrorsCount < self::$DEPENDENCY_TASK_RESULTS->errorCount()) { | ||
| $this->setSkippedDependsOn($dependency); | ||
|
|
||
| return false; | ||
| } | ||
| } else { | ||
| $this->status = BaseTestRunner::STATUS_SKIPPED; | ||
|
|
||
| $this->result->startTest($this); | ||
|
|
||
| $this->result->addError( | ||
| $this, | ||
| new SkippedTestError( | ||
| \sprintf( | ||
| 'Reordering same class dependency function is not implemented.' . | ||
| ' Please reorder "%s" before "%s".', | ||
| \explode('::', $dependency, 2)[1], | ||
| $this->getName(false) | ||
| ) | ||
| ), | ||
| 0 | ||
| ); | ||
| $this->result->endTest($this, 0); | ||
|
|
||
| return false; | ||
| } | ||
| } else { | ||
| $this->setSkippedDependsOn($dependency); | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
|
|
||
| $this->getSkippedFailedPassedKeys($skippedKeys, $failedKeys, $passedKeys, $passed); | ||
| $dependencyName = false; | ||
|
|
||
| if (isset($passed[$dependency . $this->getDataSetAsString(false)])) { | ||
| $dependencyName = $dependency . $this->getDataSetAsString(false); | ||
| } elseif (isset($passed[$dependency])) { | ||
| $dependencyName = $dependency; | ||
| } | ||
|
|
||
| if ($dependencyName) { | ||
| if ($passed[$dependencyName]['size'] != \PHPUnit\Util\Test::UNKNOWN && | ||
| $this->getSize() != \PHPUnit\Util\Test::UNKNOWN && | ||
| $passed[$dependencyName]['size'] > $this->getSize()) { | ||
| $this->result->addError( | ||
| $this, | ||
| new SkippedTestError( | ||
|
|
@@ -1764,11 +1884,11 @@ private function handleDependencies(): bool | |
| $deepCopy = new DeepCopy; | ||
| $deepCopy->skipUncloneable(false); | ||
|
|
||
| $this->dependencyInput[$dependency] = $deepCopy->copy($passed[$dependencyName]['result']); | ||
| } elseif ($shallowClone) { | ||
| $this->dependencyInput[$dependency] = clone $passed[$dependencyName]['result']; | ||
| } else { | ||
| $this->dependencyInput[$dependency] = $passed[$dependencyName]['result']; | ||
| } | ||
| } else { | ||
| $this->dependencyInput[$dependency] = null; | ||
|
|
@@ -2117,6 +2237,6 @@ private function checkExceptionExpectations(Throwable $throwable): bool | |
| private function runInSeparateProcess(): bool | ||
| { | ||
| return ($this->runTestInSeparateProcess === true || $this->runClassInSeparateProcess === true) && | ||
| $this->inIsolation !== true && !$this instanceof PhptTestCase; | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This data structure is only needed for the edge case of automatically running dependencies located in another class. Does this need to be a class-wide variable?
Also: why
static, it belongs to an actualTestCaseinstance?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's static for optimization reasons.
Meaning, if multiple functions require this same dependency, the results will be used from this class property instead of running dependency again and again, maybe even 100 times, before it will run the test.