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: avoid rounding microsecond to 1_000_000 #11861

Conversation

dqkqd
Copy link
Contributor

@dqkqd dqkqd commented Jan 26, 2024

I'm using pytest with python 3.9, and my log-date-format is enabled for microseconds. However, my testsuite occasionally fails with the message below:

ValueError: microsecond must be in 0..999999

I'm pretty sure the issue occurred because we're rounding up microsecond here, which make it exceeds 999999.

# Construct `datetime.datetime` object from `struct_time`
# and msecs information from `record`
dt = datetime(*ct[0:6], microsecond=round(record.msecs * 1000), tzinfo=tz)
return dt.strftime(datefmt)

As we can see msecs can take more than 4 digits after decimal points, hence it can be rounded up to 1_000_000 after multiplying with 1_000. (in python 3.9)

>>> import logging
>>> r = logging.makeLogRecord({'msg': 'Message'})
>>> r.msecs
696.0451602935791

This is "fixed" in python 3.10 and 3.11 python/cpython#89047
They floored msecs so it will always never be rounded to 1_000_000.

>>> import logging
>>> r = logging.makeLogRecord({'msg': 'Message'})
>>> r.msecs
459.0

However, since we also support 3.8 and 3.9, I think we should fix this by flooring microsecond as well.
(I don't know how to write test for this case without mocking time.time())

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Other than my comment, could you please also add a changelog entry? Thanks!

src/_pytest/logging.py Show resolved Hide resolved
@dqkqd dqkqd force-pushed the fix-bug-formatter-round-microsecond-to-1000000 branch from c489824 to 6d69288 Compare January 27, 2024 11:56
@dqkqd
Copy link
Contributor Author

dqkqd commented Jan 27, 2024

Thanks for reviewing, I added comment and changelog.

@dqkqd dqkqd requested a review from nicoddemus January 27, 2024 12:10
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks, appreciate it!

@nicoddemus nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Jan 27, 2024
@nicoddemus nicoddemus merged commit a164ed6 into pytest-dev:main Jan 27, 2024
25 checks passed
@nicoddemus nicoddemus added backport 8.0.x and removed needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch labels Jan 27, 2024
@dqkqd dqkqd deleted the fix-bug-formatter-round-microsecond-to-1000000 branch January 27, 2024 15:49
jsuchenia pushed a commit to jsuchenia/adventofcode that referenced this pull request Jan 29, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [pytest](https://docs.pytest.org/en/latest/) ([source](https://github.com/pytest-dev/pytest), [changelog](https://docs.pytest.org/en/stable/changelog.html)) | dev | major | `7.4.4` -> `8.0.0` |

---

### Release Notes

<details>
<summary>pytest-dev/pytest (pytest)</summary>

### [`v8.0.0`](https://github.com/pytest-dev/pytest/releases/tag/8.0.0): pytest 8.0.0 (2024-01-27)

[Compare Source](pytest-dev/pytest@7.4.4...8.0.0)

See [8.0.0rc1](https://github.com/pytest-dev/pytest/releases/tag/8.0.0rc1) and [8.0.0rc2](https://github.com/pytest-dev/pytest/releases/tag/8.0.0rc2) for the full changes since pytest 7.4!

#### Bug Fixes

-   [#&#8203;11842](pytest-dev/pytest#11842): Properly escape the `reason` of a `skip <pytest.mark.skip ref>`{.interpreted-text role="ref"} mark when writing JUnit XML files.
-   [#&#8203;11861](pytest-dev/pytest#11861): Avoid microsecond exceeds `1_000_000` when using `log-date-format` with `%f` specifier, which might cause the test suite to crash.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIn0=-->

Reviewed-on: https://git.apud.pl/jacek/adventofcode/pulls/52
Co-authored-by: Renovate <renovate@apud.pl>
Co-committed-by: Renovate <renovate@apud.pl>
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

2 participants