Closed windaishi closed 3 years ago
I do not think so.
Is there a reason?
I do not think so.
That is not a great reaction.
I was just about to raise this myself actually, so thanks @windaishi for doing so already.
The nature of BDD-style testing is that the text is supposed to be human-readable, and there is no good reason why a @testdox
description should not be as long as it needs to be, and to that end be split over multiple lines for the sake of readability in a text editor, at the same time staying in-keeping with linting rules like PHPCS's line-length check (which is an entirely valid rule to work by). I'd go as far as to say it's inappropriate for it to not work like this.
The phpdoc recommendations allow for multiple-line descriptions:
If meta-data is provided, it MAY span multiple lines and COULD follow a strict format, and as such provide parameters, as dictated by the type of tag. The type of the tag can be derived from its name.
(from https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md#5-the-phpdoc-format)
Arguably it's a violation of the recommendation to not allow multiline metadata here.
Another alternative would be to remove the internal hamstring in PHPUnit's own code that predicts the possibility of multiple separate @testDox
annotations, but then ignores all but the first one!
if (isset($annotations['method']['testdox'][0])) {
$result = $annotations['method']['testdox'][0];
(https://github.com/sebastianbergmann/phpunit/blob/9.5.2/src/Util/TestDox/NamePrettifier.php#L155)
That could just be $result = implode (' ', $annotations['method']['testdox']);
However abiding by the intent of the phpdoc standard would be better.
My current work around is to suppress PHPCS's warning re long comments:
<rule ref="Generic.Files.LineLength">
<properties>
<property name="ignoreComments" value="true"/>
</properties>
</rule>
But that is a workaround, whereas this valid request ought to be reconsidered and addressed, I think.
I am also considering if my case could be separated into smaller cases, but having assessed it, my initial instinct is that the case is fine, but I will see how my tests evolve as to whether it makes sense to simplify them so the case description ought to be shorter.
Cheers all.
Just to reinforce the point made by @windaishi and @adamcameron, here is the @testdox
description for a test I am writing.
Sync an in-memory city with the database, where: the in-memory city does originate from the database; it is French; its zipcode and name match a city in the database. The database city derives from the official referential, or the in-memory city does not. Consequently, the in-memory city will be refreshed from the database city.
217 chars, nearly 3 times the line length allowed by my linter.
Arguably the method I am testing does too many things, but that's beyond the scope of my task: for clarity's sake, I need my description to be accurate.
+1
In the Before Times I did quite a bit of development on TestDox and I hope you're still enjoying the trippy colours. A few things that come to mind:
@testdox
to setup and teardown methods. 🤐 Regarding @pablomalo 's long line: that seems more like a full description of the (internal) mechanism of the tests, rather than a succinct description of what the test itself.
To provide a sane long @testdox
example I dig in our codebase and I relatively quickly found an example:
/**
* @testdox does not retry the callback passed to runInTransactionWithRetry when a non retryable exception is thrown inside the closure
*/
public function test_doesNotRetryTheCallbackPassedToRunInTransactionWithRetryWhenANonRetryableExceptionIsThrownInsideTheClosure(): void
{
$callbackCalled = 0;
$this->dbMock->method('transactional')->willReturnCallback(
fn ($callback) => $callback(),
);
$this->expectException(Exception::class);
try {
$this->entityManager->runInTransactionWithRetry(
function () use (&$callbackCalled): string {
$callbackCalled += 1;
throw new Exception();
},
);
} catch (Exception $e) {
// The callback was only executed once and not retried
self::assertSame(1, $callbackCalled);
throw $e;
}
}
I wrote this function to be able to display a multiline @testdox annotation content:
public function printTestDoxAnnotation($method)
{
$docBlock = (new \ReflectionMethod($this, $method))->getDocComment();
// Find the index of the @testdox annotation
$testDoxIndex = strpos($docBlock, '@testdox');
if ($testDoxIndex !== false) {
// Get the substring after the @testdox annotation
$testDoxContent = substr($docBlock, $testDoxIndex + strlen('@testdox'));
// Use regex to capture the lines until the next annotation or the end of the docblock
preg_match('/\s*([^@*].*?)(?=\s*(?:\*\/|@\w+\b|\z))/s', $testDoxContent, $matches);
if (isset($matches[1])) {
$testDoxData = trim($matches[1]);
// Format and print the result
$lines = explode(PHP_EOL, $testDoxData);
$maxLineLength = max(array_map('strlen', $lines));
echo "$method\n";
echo "+".str_repeat("-", $maxLineLength + 2)."+\n";
foreach ($lines as $line) {
$line = preg_replace('/^[\s\*]*(?=\b\w+\b)/', '', $line);
echo "| ".$line.str_repeat(" ", $maxLineLength - strlen($line))." |\n";
}
echo "+".str_repeat("-", $maxLineLength + 2)."+\n";
} else {
echo $method;
}
} else {
echo "No @testdox annotation found.";
}
}
You can use it like this:
/**
* @testdox This line one
* This is line 2
* This is line 3
*/
public function testCase() {
// ... opt-out code
}
Then, in your setUp
method:
protected function setUp()
{
parent::setUp();
$showTestDox = array_search('--testdox', $_SERVER['argv']) !== false;
$showTestDox && $this->printTestDoxAnnotation($this->getName());
}
Then:
./vendor/bin/phpunit --testdox
Result:
testCase
+--------------------------+
| This is line one |
| This is line 2 |
| This is line 3 |
+--------------------------+
Summary
@testdox
annotations should be allowed to be multiline.Current behavior
If you have a test with a testdox annotation, e.g.
then the testdox becomes only "some very long testdox annotation that does NOT".
Unfortunately sometimes you have very very long testdox that violate agains the maximum line length of the code style. But you cannot chop down the @textdox.
So maybe you could add a way to make the @testdox multiline?