getsentry / getsentry/sentry-python

Consider mocking time in tests

未關閉
#3,335 1 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
Profiles Python Tests
主要語言
Python
星號
2.2k
分支
669
平均合併
1 天 1 小時
30 天內合併 PR
213

描述

To ensure the reliability of time-dependent tests and reduce their duration, I suggest mocking time in tests.

Currently, some tests implicitly rely on underlying platform performance by setting arbitrary sleep interval duration that would be enough for tests to pass (for example, in https://github.com/getsentry/sentry-python/blame/081285897e4471690ae52b3afe81a6a495f75ec8/tests/profiler/test_continuous_profiler.py#L232). While it is possible to empirically come up with values that would make tests complete on the infrastructures used by Python SDK for Sentry.io developers, there are other infrastructures that run these tests, and their performance will be different. On faster platforms, tests will execute slower than they would if sleep delays were optimal for this specific platform. Considering thousands of tests that run a few minutes per testing workflow (most recent runs on Test Common on GitHub Actions take 6-7 minutes to complete: https://github.com/getsentry/sentry-python/actions/workflows/test-integrations-common.yml, see screenshot), speeding up tests might be valuable. On slower platforms, tests might fail and this is the original reason for filling this issue. Building Sentry SDK on Fedora infrastructure occasionally fails (https://koji.fedoraproject.org/koji/taskinfo?taskID=120920902, see screenshot) because time-dependent tests do not pass:

```
=================================== FAILURES ===================================
______ test_continuous_profiler_manual_start_and_stop[experiment-thread] _______
tests/profiler/test_continuous_profiler.py:234: in test_continuous_profiler_manual_start_and_stop
assert_single_transaction_with_profile_chunks(envelopes, thread)
tests/profiler/test_continuous_profiler.py:86: in assert_single_transaction_with_profile_chunks
assert len(items["profile_chunk"]) > 0
E assert 0 > 0
E + where 0 = len([])
```

Another example: https://bugzilla.redhat.com/show_bug.cgi?id=2265822.

It is worth noting that this issue is not unique to Fedora infrastructure, and apparently occurs elsewhere. For example, while searching for `test_continuous_profiler_manual_start_and_stop` on GitHub, I noticed this test (along with other time-dependent tests) is being disabled in Gentoo package specification: https://github.com/gentoo/gentoo/blob/997938b5b7bf5deb7943b44a2dc52485d14f8076/dev-python/sentry-sdk/sentry-sdk-2.7.1.ebuild#L117-L122.

I propose mocking time. While it is possible to do using the `unittest.mock` library directly, I suggest using a library for that, such as https://pypi.org/project/freezegun/ or https://pypi.org/project/time-machine/. The latter currently works with CPython only, however, Sentry SDK is currently tested with CPython only as well (at least on GitHub Actions), PyPy was removed from the testing suite here: https://github.com/getsentry/sentry-python/pull/1602:

> Removed PyPy from the test suite because it was never run. We have to reevaluate support for PyPy.

What is your opinion on this issue? Will you accept a PR(s) that introduces mocking for time? If so, which library do you prefer me to use?

![Image](https://github.com/user-attachments/assets/720f14e9-7679-4143-9730-35e8362559be)
![Image](https://github.com/user-attachments/assets/1b04926a-1e9a-4c40-88d3-90eaa9abf90f)

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。