Skip to content

Fix flaky EngineProcessor time filter tests#8058

Merged
pavoljuhas merged 7 commits intoquantumlib:mainfrom
senahazir:fix-flaky-time-filter-tests
Apr 27, 2026
Merged

Fix flaky EngineProcessor time filter tests#8058
pavoljuhas merged 7 commits intoquantumlib:mainfrom
senahazir:fix-flaky-time-filter-tests

Conversation

@senahazir
Copy link
Copy Markdown
Contributor

@senahazir senahazir commented Apr 23, 2026

This fix addresses the flaky tests in engine_processor_test.py as explained in #7851.
The tests are fixed by mocking the datetime.datetime class with a subclass
_FrozenDateTime to make datetime.datetime.now() return a constant.

I reproduced the issue by adding import time; time.sleep(1) to both test_get_schedule_time_filter_behavior and test_list_reservations_time_filter_behavior and confirmed that the fix works for both cases (though I removed the sleeps for this pr).

Fixes #7851

@github-actions github-actions Bot added the size: S 10< lines changed <50 label Apr 23, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.63%. Comparing base (b44b862) to head (b63dda1).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8058      +/-   ##
==========================================
- Coverage   99.63%   99.63%   -0.01%     
==========================================
  Files        1110     1110              
  Lines       99794    99954     +160     
==========================================
+ Hits        99433    99591     +158     
- Misses        361      363       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread cirq-google/cirq_google/engine/engine_processor.py Outdated
Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

I think we can fix this without changing engine_processor.py.

Following the idea in #7851 (comment), here is a datetime.datetime subclass that can be added to engine_processor_test.py

class _FrozenDateTime(datetime.datetime):

    _stock_datetime = datetime.datetime

    @classmethod
    @functools.cache
    def now(cls, tz=None):
        return cls._stock_datetime.now(tz)

The affected test functions can be then decorated with

@mock.patch('datetime.datetime', _FrozenDateTime)

which would replacedatetime.datetime for their execution.

@pavoljuhas pavoljuhas added the ci/no-release Use this label for pull request that should not have Cirq pre-release on PyPI. label Apr 23, 2026
Copy link
Copy Markdown
Contributor Author

@senahazir senahazir left a comment

Choose a reason for hiding this comment

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

Updated the PR with the requested changes.

It is sufficient to apply for them the mocking of datetime.datetime
with _FrozenDateTime.  It is also unnecessary to reset the
_FrozenDateTime.now, although it is good to know such option exists.
Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM after pushing some code tweaks and updating PR description.
(The description is used for git commit message so we want it to
be in sync with changes)

Thank you for the fix!

@pavoljuhas pavoljuhas added this pull request to the merge queue Apr 27, 2026
@pavoljuhas pavoljuhas removed this pull request from the merge queue due to a manual request Apr 27, 2026
@pavoljuhas pavoljuhas added this pull request to the merge queue Apr 27, 2026
Merged via the queue into quantumlib:main with commit c73d890 Apr 27, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/no-release Use this label for pull request that should not have Cirq pre-release on PyPI. size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EngineProcessor tests are flaky because of datetime.datetime.now() calls

3 participants