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

Fix Php80Test::testFdiv() #393

Closed
wants to merge 1 commit into from
Closed

Fix Php80Test::testFdiv() #393

wants to merge 1 commit into from

Conversation

WinterSilence
Copy link
Contributor

Need using $this->assertEqualsWithDelta() to compare float values.
Why not use $this->assertEquals() or $this->assertSame()? Because (0.1 * 0.1) / 0.1 not equal 0.1, it's ~0.10000000000000002

@nicolas-grekas
Copy link
Member

This reminds me symfony/symfony#45475
This happens since sebastianbergmann/phpunit#4874

/cc @sebastianbergmann is that legit?

@WinterSilence
Copy link
Contributor Author

@nicolas-grekas no)) assertEqualsWithDelta() added specially for same cases in PhpUnit long time ago

@WinterSilence
Copy link
Contributor Author

WinterSilence commented Feb 26, 2022

@nicolas-grekas your question to Sebastian is incorrect: this package use PhpUnit 5-6, sebastianbergmann/phpunit#4874 is PhpUnit 8

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 2, 2022

This PR is definitely motivated by sebastianbergmann/phpunit#4874
The CI is currently green on versions that don't have that patch, and red on jobs that use recent PHPUnit versions.

@nicolas-grekas
Copy link
Member

Issue fixed in a932445
Thanks for submitting.

@WinterSilence
Copy link
Contributor Author

WinterSilence commented Mar 2, 2022

@nicolas-grekas this PR motivated by https://github.com/symfony/polyfill/runs/5367868196?check_suite_focus=true
I'm don't see before sebastianbergmann/phpunit#4874
It's not fixed it's f..n shame

@nicolas-grekas
Copy link
Member

It's not fixed it's f..n shame

what do you mean? yes it is, see newest CI runs as a proof.

@WinterSilence
Copy link
Contributor Author

WinterSilence commented Mar 2, 2022

@nicolas-grekas this PR too do this, but you close it and update test data instead using correct assertion.

@nicolas-grekas
Copy link
Member

This PR works around the test data, which was wrong in the first place. We want an exact check, not an approximation here.

@WinterSilence
Copy link
Contributor Author

@nicolas-grekas sorry, my fail

@WinterSilence WinterSilence deleted the patch-1 branch March 2, 2022 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants