Skip to content

Don't truncate subsecond precision in run-tests.php JUNIT output #7836

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

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 27, 2021

When run-tests.php has been typed[1], the type of $time has been
chosen to be int. This, however, leads to truncation, and the
somewhat relevant subsecond precision is lost. We fix that by
changing the type to float, although int|string would be more
appropriate, but requires PHP ≥ 7.4.0. Another option would be to
move the number_format() formatting into junit_mark_test_as().

[1] 11274f5


I've noticed this due to https://dev.azure.com/phpazuredevops/PHP/_build/results?buildId=22476&view=logs&j=0d3730b8-0d59-574b-ab4a-5e189a74d53b&t=492e5677-f211-533f-274d-3a4af39d4cce&l=42.

When run-tests.php has been typed[1], the type of `$time` has been
chosen to be `int`.  This, however, leads to truncation, and the
somewhat relevant subsecond precision is lost.  We fix that by
changing the type to `float`, although `int|string` would be more
appropriate, but requires PHP ≥ 7.4.0.  Another option would be to
move the `number_format()` formatting into `junit_mark_test_as()`.

[1] <php@11274f5>
@cmb69 cmb69 requested a review from Girgias December 27, 2021 12:47
@Girgias
Copy link
Member

Girgias commented Dec 27, 2021

Hum, I seems reasonable to make this change as I don't know how invasive it is to move the formatting into junit_mark_test_as()

@cmb69 cmb69 closed this in 87d9e02 Dec 27, 2021
@cmb69 cmb69 deleted the cmb/junit-time branch December 27, 2021 21:32
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.

3 participants