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

Prevent unrelated error from being displayed if a scenario step has failed #6743

Conversation

craig-mcmahon
Copy link
Contributor

This fixes #6716 which results in incorrect test failures being reported where a scenario has a failed assertion that has been caught within the test itself or via expectException

@craig-mcmahon
Copy link
Contributor Author

Unsure how to add a test for this as it only occurs on the failure of a test.

I used the following test

<?php

namespace Tests\Unit;

use Codeception\Test\Unit;
use PHPUnit\Framework\AssertionFailedError;
use Tests\Support\UnitTester;

class TestTest extends Unit {

	protected UnitTester $tester;

	public function test1(){
		static::fail(':-(');
	}

	public function test2(){
		$this->expectException(AssertionFailedError::class);
		$this->tester->assertFalse(true);
	}

}

Output before fix

Codeception PHP Testing Framework v5.1.0 https://stand-with-ukraine.pp.ua

Tests.Unit Tests (2) -------------------------------------------------------------------------------------------------------------
✖ TestTest: Test1(0.01s)
✖ TestTest: Test2(0.00s)
----------------------------------------------------------------------------------------------------------------------------------
Time: 00:00.102, Memory: 16.87 MB

There was 1 failure:
1) TestTest: Test2
 Test  tests/Unit/TestTest.php:test2
:-(
#1  /usr/src/app/tests/Unit/TestTest.php:14
#2  /usr/src/app/vendor/bin/codecept:119

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

Output after fix

Codeception PHP Testing Framework v5.1.0 https://stand-with-ukraine.pp.ua

Tests.Unit Tests (2) -------------------------------------------------------------------------------------------------------------
✖ TestTest: Test1(0.01s)
✔ TestTest: Test2(0.00s)
----------------------------------------------------------------------------------------------------------------------------------
Time: 00:00.087, Memory: 16.87 MB

There was 1 failure:
1) TestTest: Test1
 Test  tests/Unit/TestTest.php:test1
:-(
#1  /usr/src/app/tests/Unit/TestTest.php:14
#2  /usr/src/app/vendor/bin/codecept:119

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

@Naktibalda
Copy link
Member

Your change broke conditional assertions
Probably because condition assertion implementation passes a clone of test to FailEvent:

$failEvent = new FailEvent(clone($this->test), $f, 0);

@Naktibalda
Copy link
Member

I think that the best way to compare if 2 variables contain the same test is to compare results of \Codeception\Test\Descriptor::getTestSignatureUnique().

if ($lastFailure && Descriptor::getTestSignatureUnique($lastFailure->getTest()) === Descriptor::getTestSignatureUnique($this)

For ideas how to implement test for your change see

public function testsWithConditionalFails(CliGuy $I)
{
$I->executeCommand('run scenario ConditionalCept --no-exit');
$I->seeInShellOutput('There were 3 failures');
$I->seeInShellOutput('Fail File "not-a-file" not found');
$I->seeInShellOutput('Fail File "not-a-dir" not found');
$I->seeInShellOutput('Fail File "nothing" not found');
}
public function runTestWithAnnotationDataprovider(CliGuy $I)
{
$I->executeCommand('run scenario -g dataprovider --steps');
$I->seeInShellOutput('OK (18 tests');
}
public function runFailedTestAndCheckOutput(CliGuy $I)
{
$I->executeCommand('run scenario FailedCept', false);
$testPath = implode(DIRECTORY_SEPARATOR, ['tests', 'scenario', 'FailedCept.php']);
$I->seeInShellOutput('1) FailedCept: Fail when file is not found');
$I->seeInShellOutput('Test ' . $testPath);
$I->seeInShellOutput('Step See file found "games.zip"');
$I->seeInShellOutput('Fail File "games.zip" not found at ""');
}
public function runTestWithCustomSetupMethod(CliGuy $I)
{
$I->executeCommand('run powers PowerUpCest');
$I->dontSeeInShellOutput('FAILURES');
}
public function runCestWithTwoFailedTest(CliGuy $I)
{
$I->executeCommand('run scenario PartialFailedCest', false);
$I->seeInShellOutput('See file found "testcasetwo.txt"');
$I->seeInShellOutput('See file found "testcasethree.txt"');
$I->seeInShellOutput('Tests: 3,');
$I->seeInShellOutput('Failures: 2.');
}
public function failedTestFollowedByExceptionReportsCorrectStep(CliGuy $I)
{
$I->executeCommand('run scenario FailureAndExceptionCest', false);
$I->seeInShellOutput('1. $I->throwException("test exception")');
$I->seeInShellOutput('Step Assert same 1,2');
$I->seeInShellOutput('Failed asserting that 2 is identical to 1.');
}
public function displaysAllFailedConditionalStepsInOneCest(CliGuy $I)
{
$I->executeCommand('run scenario MultipleConditionalFailsCest --no-ansi', false);
$I->seeInShellOutput('x MultipleConditionalFailsCest: Multiple fails 3x[F]');
$I->dontSeeInShellOutput('MultipleConditionalFailsCest: Multiple fails 2x[F]');
$I->dontSeeInShellOutput('MultipleConditionalFailsCest: Multiple fails [F]');
$I->dontSeeInShellOutput('MultipleConditionalFailsCest: Multiple fails[F]');
$I->seeInShellOutput('There were 3 failures:');
$I->seeInShellOutput('Step Can see file found "not-a-file"');
$I->seeInShellOutput('Step Can see file found "not-a-dir"');
$I->seeInShellOutput('Step Can see file found "nothing"');
$filename = implode(DIRECTORY_SEPARATOR, ['tests', 'scenario','MultipleConditionalFailsCest.php']);
$I->seeInShellOutput(' 1. $I->canSeeFileFound("not-a-file") at ' . $filename . ':7');
$I->seeInShellOutput(' 7. $I->canSeeFileFound("not-a-dir") at ' . $filename . ':13');
$I->seeInShellOutput(' 13. $I->canSeeFileFound("nothing") at ' . $filename . ':19');
}

public function checkForFails(CliGuy $I)
{
$I->amInPath('tests/data/sandbox');
$I->executeCommand('run order FailedCept.php --no-exit');
$I->seeFileFound('order.txt', 'tests/_output');
$I->expect('global bootstrap, initialization, beforeSuite, before, bootstrap, test, fail, after, afterSuite');
$I->seeFileContentsEqual("BIB([STF])");
}
public function checkForCanCantFails(CliGuy $I)
{
$I->amInPath('tests/data/sandbox');
$I->executeCommand('run order CanCantFailCept.php --no-exit');
$I->seeFileFound('order.txt', 'tests/_output');
$I->expect(
'global bootstrap, initialization, beforeSuite, before, bootstrap, test,'
. ' test, fail, after, afterSuite'
);
$I->seeFileContentsEqual("BIB([STTF])");
}
public function checkForCanCantFailsInCest(CliGuy $I)
{
$I->amInPath('tests/data/sandbox');
$I->executeCommand('run order CanCantFailCest.php --no-ansi --no-exit');
$I->seeInShellOutput('x CanCantFailCest: Test one [F]');
$I->seeInShellOutput('x CanCantFailCest: Test two [F]');
$I->dontSeeInShellOutput('+ CanCantFailCest: One');
$I->dontSeeInShellOutput('+ CanCantFailCest: Two');
$I->seeFileFound('order.txt', 'tests/_output');
$I->expect(
'global bootstrap, initialization, beforeSuite, before, bootstrap, test,'
. ' test, fail, test, test, fail, after, afterSuite'
);
$I->seeFileContentsEqual("BIB([TTF][TTF])");
}
public function checkForCanCantFailsInTest(CliGuy $I)
{
$I->amInPath('tests/data/sandbox');
$I->executeCommand('run order CanCantFailTest.php --no-ansi --no-exit');
$I->seeInShellOutput('x CanCantFailTest: One');
$I->seeInShellOutput('x CanCantFailTest: Two');
$I->dontSeeInShellOutput('+ CanCantFailTest: One');
$I->dontSeeInShellOutput('+ CanCantFailTest: Two');
$I->seeFileFound('order.txt', 'tests/_output');
$I->expect(
'global bootstrap, initialization, beforeSuite, before, bootstrap, test,'
. ' test, fail, test, test, fail, after, afterSuite'
);
$I->seeFileContentsEqual("BIB([TTF][TTF])");
}
public function checkSimpleFiles(CliGuy $I)
{
$I->amInPath('tests/data/sandbox');
$I->executeCommand('run order --no-exit --group simple');
$I->seeFileFound('order.txt', 'tests/_output');
$I->seeFileContentsEqual("BIBP([ST][STTF][STF][ST])");
}
contains some similar tests too, but they are more difficult to understand.

@craig-mcmahon
Copy link
Contributor Author

Thanks for the feedback, your suggestion fixed the broken test. I have also refactored to avoid the loop when not needed.

I will look at adding a new test in the next few days

@craig-mcmahon
Copy link
Contributor Author

I have added a test to cover the new change, and fixed the code style check that failed in the last run

@craig-mcmahon
Copy link
Contributor Author

And fix pushed for the code style in the tests

@Naktibalda Naktibalda merged commit 066deb0 into Codeception:5.1 Mar 5, 2024
8 checks passed
@Naktibalda
Copy link
Member

Released as 5.1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Try-catch in the second test catches fail in the first test
2 participants