assertEquals() hides contents of long strings

[mikkorantalainen]

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]

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

Should we remove that condition to solve this?

[mfn]

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]

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

[sebastianbergmann]

@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]

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.

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]

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]

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

[pscheit]

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]

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]

@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.

[dmitry-ivanov]

+1

Not having a full diff (as it was in previous PHPUnit versions) is really inconvenient.

[staabm]

as far as I read the thread, we are still missing a useful reproducer for the problem described. @dmitry-ivanov could you give more context about your concrete case?

[dmitry-ivanov]

@staabm Thank you for your reply, sure!

Here's the simplified test case:

#[Test]
public function it_modifies_strings_when_showing_diff(): void
{
    $expected = 'This is a string, which has a length of more than 40 characters -- the beginning of the string will be modified when viewing diff.';
    $actual = 'This is a string, which has a length of more than 40 characters -- the beginning of the string will be modified when viewing diff!';
    $this->assertEquals($expected, $actual);
}

Which will result in such diff:

When the full string is shown in diff, it gives the full context near the test execution result. (And IDEs like PHPStorm will make it easy to understand with a proper highlighting)

When strings are trimmed, you have to open the test implementation to see the full picture. (But, I guess, the motivation behind that change could be the fact that it makes diff less noisy in a console... 🤔)

---

Here's a real example when it felt like a pain: https://github.com/dmitry-ivanov/laravel-wikipedia-grabber/blob/dcbc7f6f7b85a78b827b19343bf6eca7bec63f64/tests/Grabber/PageTest.php#L201-L214

Particularly, this assertion, which compares two relatively long strings (HTML responses): https://github.com/dmitry-ivanov/laravel-wikipedia-grabber/blob/dcbc7f6f7b85a78b827b19343bf6eca7bec63f64/tests/Grabber/PageTest.php#L210-L213

Here's what a response could look like: https://github.com/dmitry-ivanov/laravel-wikipedia-grabber/blob/dcbc7f6f7b85a78b827b19343bf6eca7bec63f64/tests/Grabber/PageTest/page-with-images-when-enabled.txt

And here's the diff, if it fails:

In that particular case, diff wasn't very useful. The way I've solved it -- just dumped the full actual response, analyzed it, and updated the expected one.

The main idea behind this issue -- I wish diff would show me full responses, instead of the trimmed ones (so that, I don't have to dump them myself).

---

So, as I see it, the trimmed strings:

• might be useful for console users (by making diffs less noisy);
• might be inconvenient for IDE users (who have a tool for highlighting diffs, and not affected by the strings length);

I hope it helps! Thank you!

[uncaught]

Uh, I'd love for this to be configurable, too. Array-Depth and all.

This isn't a useful output 😄

[staabm]

we have shortenArraysForExportThreshold to limit the depth via phpunit.xml of how many levels arrays will be exported.

I think we could have a new shortenStringsForExportThreshold phpunit.xml configuration to modify $maxLengthForStrings in a similar way.

as I understand the use-cases, people don't want shortened output because they e.g. use a sophisticaed diff-UI which can handle bigger/longer values

@sebastianbergmann wdyt?

[staabm]

hmm I thought more about my above proposal.

since different developers work differently on the same project, I think new phpunit.xml config might be the wrong way. it seems a tool specific thing, so adding a new cli-option which the tools in question can pass by themselves would be a better fit

[rosasurfer]

I implemented my own solution using an environment variable to toggle between clipped and full output via CLI. I implemented a helper TestCase containing updated assert... methods. Now I extend all my tests from this test helper instead from the standard TestCase.

Now I call PHPUnit either the standard way or - if I need full output - with an env var. It's even properly colored.

phpunit ...                     # standard output in case of errors (potentially clipped)
PHPUNIT_FULL_DIFF=1 phpunit ... # full output in case of errors (no clipping)

So the solution to the issue would be a command line option doing the above or similar internally.