Skip to content

[Stopwatch] Correct compare float data in tests #47704

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 28, 2022
Merged

[Stopwatch] Correct compare float data in tests #47704

merged 1 commit into from
Sep 28, 2022

Conversation

peter-gribanov
Copy link
Contributor

@peter-gribanov peter-gribanov commented Sep 27, 2022 •
edited by nicolas-grekas
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
License MIT

Fix erorr in unit tests

1) Symfony\Component\Stopwatch\Tests\StopwatchPeriodTest::testGetDuration with data set #8 (2.71, 3.14, true, 0.43)
Failed asserting that 0.43000000000000016 is identical to 0.43.

@carsonbot carsonbot added this to the 6.2 milestone Sep 27, 2022
@alexander-schranz
Copy link
Contributor

alexander-schranz commented Sep 28, 2022 •
edited
Loading

The calculation of PHP itself seems to be false because of usage of floats.
If it is important to be correct the microtime could be first converted into bigger values:

var_dump(3.14-2.71); // incorrect: float(0.43000000000000016)

var_dump((3.14 * 1_000_000 - 2.71 * 1_000_000) / 1_000_000); // correct: float(0.43)

https://3v4l.org/oi0o0

@nicolas-grekas
Copy link
Member

I opened #47713 as an alternative approach.

@nicolas-grekas nicolas-grekas changed the base branch from 6.2 to 4.4 September 28, 2022 12:52
@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 4.4 Sep 28, 2022
nicolas-grekas
nicolas-grekas approved these changes Sep 28, 2022
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My alternative approach failed, let's go :)

@nicolas-grekas nicolas-grekas merged commit 89ca1c0 into symfony:4.4 Sep 28, 2022
@peter-gribanov peter-gribanov deleted the stopwatch_test branch September 29, 2022 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh Awaiting requested review from xabbuh

@lyrixx lyrixx Awaiting requested review from lyrixx

@dunglas dunglas Awaiting requested review from dunglas

@yceruto yceruto Awaiting requested review from yceruto

@wouterj wouterj Awaiting requested review from wouterj

@chalasr chalasr Awaiting requested review from chalasr

@jderusse jderusse Awaiting requested review from jderusse

Assignees
No one assigned
Labels
Bug Status: Reviewed Stopwatch
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
5 participants
@peter-gribanov @alexander-schranz @nicolas-grekas @stof @carsonbot