-
-
Notifications
You must be signed in to change notification settings - Fork 140
Use assertSame() instead of assertEquals() where possible #3
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
Comments
big 👍 |
Go for it. |
I started changing the documentation, but when I was testing the example output I ran into an issue with the array not being diffed when See the example from docs: <?php
use PHPUnit\Framework\TestCase;
class ArrayDiffTest extends TestCase
{
public function testEquality() {
$this->assertEquals(
[1, 2, 3, 4, 5, 6],
[1, 2, 33, 4, 5, 6]
);
}
} It outputs this, so you can easily tell what's the problem:
But when the
There is already an issue describing this sebastianbergmann/phpunit#2169 (and the older one sebastianbergmann/phpunit#1252). I will try to have a look at the diffing issue first, because it would be hard to persuade users to use a stricter assert, if it provides worse DX. EDIT: PR with the fix: sebastianbergmann/phpunit#2976 |
…omponent tests (GromNaN) This PR was merged into the 3.4 branch. Discussion ---------- Use strict assertSame instead of assertEquals in Asset component tests | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | #35762 (comment) | License | MIT | Doc PR | N/A Using `assertSame` instead of `assertEquals` is recommended when possible (see sebastianbergmann/phpunit-documentation-english#3). It is stricter and must be more efficient ([`===`](https://github.com/sebastianbergmann/phpunit/blob/8.5.2/src/Framework/Constraint/IsIdentical.php#L63) vs a [comparator class](https://github.com/sebastianbergmann/phpunit/blob/8.5.2/src/Framework/Constraint/IsEqual.php#L79)). ~~Also, removing useless string cast.~~ Commits ------- e8f3e84 Use strict assertion in asset tests
I noticed that the introductory documentation chapter uses
assertEquals()
. I suggest that it can be replaced withassertSame()
, which is stricter a may prevent unexpected type conversion bugs.What do you think?
If you think it is a good idea, I can prepare a PR.
The text was updated successfully, but these errors were encountered: