sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.69k stars 2.2k forks source link

assertEquals() hides contents of long strings #5846

Open mikkorantalainen opened 4 months ago

mikkorantalainen commented 4 months ago
Q A
PHPUnit version 10.5.20
PHP version 8.1.2-1ubuntu2.17 (cli)
Installation Method PHAR

Summary

When I have long strings in an array that's passed to assertEquals() and the values are not equal, the emitted failure message clips away important part of the string.

How to reproduce

mkdir test && cd test
wget -O phpunit https://phar.phpunit.de/phpunit-10.phar
cat << 'EOF' > UnitTest.php
<?php
class UnitTest extends PHPUnit\Framework\TestCase
{
  public function testAssertingArrays()
  {
    $mock_testresult = array(
      0 => "Some short string",
      1 => "Some really long string that just keeps going and going and going but contains important clue HERE about why this whole test failed.",
    );
    $this->assertEquals($mock_testresult, array(), "Expected empty array but got non-empty array.");
  }
}
EOF
php phpunit UnitTest.php

Current behavior

PHPUnit 10.5.20 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.2-1ubuntu2.17

F                                                                   1 / 1 (100%)

Time: 00:00.001, Memory: 24.79 MB

There was 1 failure:

1) UnitTest::testAssertingArrays
Expected empty array but got non-empty array.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'Some short string'
-    1 => 'Some really long string that ...ailed.'
 )

/home/user/test/UnitTest.php:10

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

Expected behavior

Otherwise same but the array offset 1 should show enough of the string to also see the word "HERE".

Additional information

I can see the whole string if I run php phpunit --debug UnitTest.php instead but the output is so verbose otherwise that it's hard to read. I'd rather see normal output but the whole string, but I couldn't find any documented feature to turn off this kind of string clipping/shortening.

hans-thomas commented 3 weeks ago

@sebastianbergmann It's because of this block of code in the sebastian/exporter package.

Should we remove that condition to solve this?

mfn commented 3 weeks ago

Wow, it would be super useful to not have this clipping at all.

I've so many cases where I see the clipping where I realize my "expected" actually is wrong and it would just speed up things if I can copypaste it from that output and carry on.

pscheit commented 4 days ago

Or at least would help if the exporter could be injected (into the ArrayComparator for example)

sebastianbergmann commented 3 days ago

@theseer, @localheinz, @staabm, @Schrank, @sebastianheuer, @Tesla91, and I discussed this issue during the PHPUnit Code Sprint in Munich today.

We suspect that the reproducing example you provided has been simplified too much to actually show the problem you want to report. For example, you mention that strings are shortened too much. However, in your example array elements are missing which has nothing to do with the shortening strings.

@hans-thomas pointed in https://github.com/sebastianbergmann/phpunit/issues/5846#issuecomment-2378629761 to https://github.com/sebastianbergmann/exporter/blob/575f0d22969d2a8ca2bc06ec54c215ee8feab11b/src/Exporter.php#L110-L114. We think it is worth exploring to introduce an optional argument to the shortenedExport() method to configure the cut-off length. Once this would be implemented, a configuration option for PHPUnit can be implemented to leverage that.

Another idea we think worth pursuing is to remove common prefixes and suffixes from actual and expected single-line strings.

@mikkorantalainen What do you think?

pscheit commented 1 day ago

I think, if you look at the output from phpunit, the other lines are already extending way to the side then the cut-off-diff.

e.g. image

there are at least 10 chars "available".

When researching the issue I thought setting "columns=max" configuration parameter and then settings COLUMNS to a bigger value would solve it, but was surprised it didnt.

staabm commented 1 day ago

there are at least 10 chars "available".

so whats your point? in the example screenshot the diff in the key is obvious. what should/could be improved in this case?

sebastianbergmann commented 1 day ago

The columns configuration options only affects the number of columns used by the progress printer.

pscheit commented 1 day ago

just saying that I was surprised that it didn't had any effect on the diff output - before reading the docs.

@staabm yes, you are right. I did the screenshot after I added the keys to have an easier-to-read-diff.

Schrank commented 18 hours ago

We had a discussion about extending the length of the string to the width of the shell.

I can’t repeat everything, but it is hard to implement for all edge cases in nested arrays snd other structures. But even worse, there is no code which properly tells you how wide the window is.

mikkorantalainen commented 17 hours ago

@hans-thomas pointed in #5846 (comment) to https://github.com/sebastianbergmann/exporter/blob/575f0d22969d2a8ca2bc06ec54c215ee8feab11b/src/Exporter.php#L110-L114. We think it is worth exploring to introduce an optional argument to the shortenedExport() method to configure the cut-off length. Once this would be implemented, a configuration option for PHPUnit can be implemented to leverage that.

Another idea we think worth pursuing is to remove common prefixes and suffixes from actual and expected single-line strings.

@mikkorantalainen What do you think?

I'd prefer to have a command line or config option to never cut or shorten the data at all. If it's called shortened_cut_off_length and I can set it to zero or -1 to signal that there's no limit, I'd be totally happy with it.

That said, I would prefer following Git style output even more. With Git the output is always handled by rendering full output via pager, not by cutting the output byt Git. I think Git simply launches less -SRFX pipes all output to it. And you can also use command line flag --no-pager to disable this feature and simply emit all output as-is into stdout.