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

Confusing failure message when assertFileNotEquals() is used on two empty files #2721

Closed
TomasVotruba opened this issue Jun 28, 2017 · 9 comments
Labels
type/bug Something is broken

Comments

@TomasVotruba
Copy link

TomasVotruba commented Jun 28, 2017

Q A
PHPUnit version 6.2.2
PHP version 7.1.6
Installation Method Composer
$this->assertFileEquals(__DIR__ . '/empty-file.html', __DIR__ . '/second-empty-file.html');

Works fine.
But...

$this->assertFileNotEquals(__DIR__ . '/empty-file.html', __DIR__ . '/second-empty-file.html');

shows

Failed asserting that '' is not equal to <string:>.

Not sure what that means. At the moment I use workaround:

$this->assertFalse(file_get_contents($normalPostLocation) === file_get_contents($ampPostLocation));
@SpacePossum
Copy link
Contributor

If both files are empty the assertNotEquals should not pass right?
So the issue here is to make the (default) fail message better?

@TomasVotruba
Copy link
Author

It's not about empty files. This happen for same-content files as well.

I don't now what is the issue is.

@SpacePossum
Copy link
Contributor

$this->assertNotEquals('', '');
Failed asserting that '' is not equal to <string:>.
$this->assertNotEquals('a', 'a');
Failed asserting that 'a' is not equal to <string:a>.
$this->assertNotEquals([], []);
Failed asserting that Array &0 () is not equal to Array &0 ().

I might be wrong here but I think there is no issue,
other than the fail message Failed asserting that '' is not equal to <string:>. might be a bit cryptic, however as it states '' equals <string: '' > so the assert fails correctly.

A nice improvement might be to fail as;

Failed asserting that content of file "/var/foo/a.txt" is not equal to content of file "/var/foo/b.txt".

@TomasVotruba
Copy link
Author

TomasVotruba commented Jul 6, 2017

This is WTF to me. How do I know what "string:a" is? I didn't event notice the "a" there.

I would expect this message:

$this->assertNotEquals('a', 'a');
Failed asserting that 'a' is not equal to 'a'.

To be consistent with the rest of PHPUnit messages.

$this->assertContains('a', $value);
// Failed $value ... contains 'a'.  # pseudo message, not sure what exactly is there

And not:

// Failed $value ... contains '<string:a>'.  # pseudo message, not sure what exactly is there

A nice improvement might be to fail as;

Failed asserting that content of file "/var/foo/a.txt" is not equal to content of file "/var/foo/b.txt".

Yes. With their content possible, I'd have to look for the real differences manually to the files otherwise.

@SpacePossum
Copy link
Contributor

ah now I see :) 👍
lets see if this can be improved

@sebastianbergmann
Copy link
Owner

I just looked at this (sorry for not looking at it sooner) and I am confused as to what this is about.

@SpacePossum
Copy link
Contributor

SpacePossum commented Oct 13, 2017

There is nothing wrong.
When comparing two empty files with assertFileNotEquals the result is:

Failed asserting that '' is not equal to <string:>.

which is fine, but confusing for some users.

@sebastianbergmann
Copy link
Owner

Can we make it less confusing?

fetmo added a commit to fetmo/phpunit that referenced this issue Oct 14, 2017
@sebastianbergmann sebastianbergmann changed the title [assertFileNotEquals] Comparing two empty html files fails Confusing failure message when assertFileNotEquals() is used on two empty files Oct 14, 2017
@sebastianbergmann sebastianbergmann added the type/bug Something is broken label Oct 14, 2017
sebastianbergmann pushed a commit that referenced this issue Oct 14, 2017
@TomasVotruba
Copy link
Author

Awesome, thank you all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

3 participants