Skip to content

Consider mocking time in tests #3335

Open

Description

@rominf

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: #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
Image

Activity

  1. moved this to Waiting for: Product Owner in GitHub Issues with 👀 3on Jul 24, 2024
  2. sl0thentr0py commented on Jul 24, 2024

    @sl0thentr0py
    Member

    hey @rominf all great points overall, yes we could definitely write a lot of our tests better that have grown over time.
    The gentoo link is very interesting since it points to a whole bunch of flaky and weird tests.
    Feel free to pick a simple library that works for you and make a PR.
    We will probably not prioritize this internally but if you make a contribution, we'd be happy to merge it in.

  3. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 3on Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions