Test-suite - Show warning when ".fixed" file not found #336
Conversation
|
@fredden Looks like this change is not compatible with PHPUnit 4.x.... |
| @@ -197,8 +197,10 @@ final public function testSniff() | |||
| $fixedFilename = basename($fixedFile); | |||
| $failureMessages[] = "Fixed version of $filename does not match expected version in $fixedFilename; the diff is\n$diff"; | |||
| } | |||
| } else { | |||
| $this->addWarning("Unable to verify auto-fixer results. File not found: $fixedFile"); | |||
There was a problem hiding this comment.
I wonder if this message could be made more specific as it could now give the impression that all test case files for sniffs containing fixers are expected to have a .fixed file, even when the fixers should/will not run for the test case file.
What about something like:
"Missing fixed version of $filename to verify the accuracy of fixes, while the sniff is making fixes against the test case file"
There was a problem hiding this comment.
I don't understand the scenario that you're referring to. When would the test suite not need a ".fixed" file for a fixer?
I can understand when a test file only contains errors which cannot be fixed automatically, but that code path does not reach here.
I've updated the text as suggested.
There was a problem hiding this comment.
I don't understand the scenario that you're referring to. When would the test suite not need a ".fixed" file for a fixer?
I can understand when a test file only contains errors which cannot be fixed automatically, but that code path does not reach here.
I agree that the code path without fixable errors would not throw the notification, but the person running the test suite will not necessarily know that, which is why I suggested the slightly different text.
|
Important note about this change: this will only work on PHPUnit 7.3.0 or higher (PHP 7.1+) as the While I don't particularly like adding a feature which is not cross-version compatible with the PHP versions supported by PHPCS, I'm going to reluctantly accept it anyway as it will ease the path to this becoming a test failure in PHPCS 4.0 and the intention is to release PHPCS 4.0 within a few months anyway (which will drop support for PHP < 7.2). |
This commit updates the `AbstractSniffUnitTest` class used for the sniff tests to show a non-build-failing warning when a test case file would lead to fixable errors/warnings, but there is no `.fixed` version available for the test case file to verify the fixes being made. This is a preliminary step towards making missing `.fixed` files a test failing error in PHPCS 4.0. Note: the warning will only show when tests are being run on PHPUnit 7.3.0 or higher.
fredden
deleted the
test-suite/auto-fixer-reliability/warning-when-file-missing
branch
…s#336) This commit updates the `AbstractSniffUnitTest` class used for the sniff tests to show a non-build-failing warning when a test case file would lead to fixable errors/warnings, but there is no `.fixed` version available for the test case file to verify the fixes being made. This is a preliminary step towards making missing `.fixed` files a test failing error in PHPCS 4.0. Note: the warning will only show when tests are being run on PHPUnit 7.3.0 or higher.
Description
This is one step closer to #300. The changes proposed in #300 are breaking as the test suite will start failing for cases where it currently does not. This pull request gives users of the test suite some information that there is a problem, before the breaking change gets introduced into 4.x.
Suggested changelog entry
Test-suite - Show warning when ".fixed" file not found
Types of changes
PR checklist