-
Notifications
You must be signed in to change notification settings - Fork 23
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
Type error when using Codeception 5th version #60
Comments
It looks like 3 separate implementations will be required to support Codeception 5, PHPUnit 10 and older versions. Signature of TestCase::run method is different in PHPUnit 10 and TestResult class is marked for removal. |
Removing |
I don't think that removing private function runSpec(string $specification, Closure $callable = null, $params = [])
{
// other code
$test = new SpecifyTest($callable->bindTo($this));
// other code
// run test
$test->run($this->getTestResultObject());
// done
} Looks like there is no way to use Specify in Codeception 5. I tried to change implementation of SpecifyHooks to force them to use ResultAggregator instead of TestResult, But it's not impossible. ResultAggregator methods works with FailureEvent which needs abstract class Test, which is TestCaseWrapper. |
|
Sorry, it was a very bad guess, I haven't even looked at the code and guessed that it calls Do you understand what's the point of catching According to #53 failed
Removing
And it would actually fix #53 Do you see any downsides? It may be necessary to move cleanup code in SpecifyHook::runSpec method to Specify/src/Codeception/Specify/SpecifyHooks.php Lines 79 to 86 in 7dc67c1
|
It's good temporary solution. It makes all my tests run on 5th Codeception, but at same time it has some cons. // SpecifyTest::run()
public function run(TestResult $result = null): TestResult
{
call_user_func_array($this->test, $this->example);
return new TestResult();
}
// SpecifyHooks::runSpec()
try {
$test->run($this->getTestResultObject());
} finally {
$this->specifyCheckMockObjects();
// restore object properties
$this->specifyRestoreProperties($properties);
if (!empty($this->afterSpecify) && is_array($this->afterSpecify)) {
foreach ($this->afterSpecify as $closure) {
if ($closure instanceof Closure) {
$closure->__invoke();
}
}
}
} Pros:
Cons:
As you mentioned it also solves problem of #53 issue.
I think we need to create another different solution for 5th version, to implement specification output and correct work with new environment. |
The output difference could be solved by prepending specify name to exception message. Continuing execution of other examples is more complicated. To support Specify, Codeception 5 must expose ResultAggregator. Probably the best option would be to make Codecept::getResultAggregator method static. try {
call_user_func_array($this->test, $this->example);
} catch (AssertionFailedError $error) {
if (class_exists(\Codeception\Codecept::class) && \Codeception\Codecept::getResultAggregator() !== null
) {
\Codeception\Codecept::getResultAggregator()->addFailure(
new \Codeception\Event\FailEvent(
new \Codeception\Test\TestCaseWrapper(
clone($this)
),
$error,
0
);
);
} else {
$result->addFailure(clone($this), $error, $result->time());
}
} |
I see Codeception's Make this: final public function realRun(ResultAggregator $result): void
{
$this->resultAggregator = $result; Like this final public function realRun(ResultAggregator $result): void
{
$this->setResultAggregator($result); And in implementation in public function setResultAggregator(?ResultAggregator $resultAggregator): void
{
$this->resultAggregator = $resultAggregator;
$this->testCase->setResultAggregator($resultAggregator)
} And we will be able to use ResultAggregator in our tests |
I understand that it may be not perfect solution but i just want to show direction of my mind. I don't think making ResultAggregator global accessible is good idea |
This solution wouldn't support tests extending |
Bad situation( At least your solution helped me to run Specify using Codeception 5. I'll hope framework will choose stable concepts that can be used natively for this lib |
There will be no TestResult in PHPUnit 10: https://github.com/sebastianbergmann/phpunit/blob/c7c1903ce2995f36a016a245d43ca21ea8ea352f/src/Framework/Test.php#L19 |
More information:
If you uses specify out of box (as described in github page), it will throw an TypeError:
Codeception\Specify\SpecifyTest::run(): Return value must be of type PHPUnit\Framework\TestResult, null returned
I have looked in and noticed that
TestResult
object was got fromTestCase
object. That how it works in 4th version. And codeception runs test usingTestCase::run
method. But in 5th version it usesTestCaseWrapper
. That's why there is notTestResult
object inTestCase
to give it in SpecifyCodeception has default configurations (just after codecept bootstrap)
The text was updated successfully, but these errors were encountered: