Fix timing dependent test_run_path_creation.py#12885
Fix timing dependent test_run_path_creation.py#12885jonathan-eq merged 1 commit intoequinor:mainfrom
Conversation
jonathan-eq
added
the
maintenance
There was a problem hiding this comment.
Pull request overview
This PR addresses flakiness in run-path creation unit tests by making <DATE> substitution deterministic, avoiding failures when tests span a midnight boundary.
Changes:
- Replace runtime-derived
<DATE>expectations with a fixedEXPECTED_DATE. - Monkeypatch the config parser’s
datetime.date.today()used for default pre-defines so<DATE>is stable during the tests.
| fixed_date = datetime.fromisoformat(EXPECTED_DATE).date() | ||
| monkeypatch.setattr( | ||
| lark_parser.datetime, | ||
| "date", | ||
| MagicMock(today=MagicMock(return_value=fixed_date)), | ||
| ) |
There was a problem hiding this comment.
This test replaces lark_parser.datetime.date with a nested MagicMock. It works for date.today(), but it’s brittle if lark_parser ever starts using datetime.date(...) or other date APIs during parsing. Consider patching lark_parser.datetime.date to a small datetime.date subclass that only overrides today() (preserving constructors/isoformat/etc.) to keep the patch safer and clearer.
| fixed_date = datetime.fromisoformat(EXPECTED_DATE).date() | ||
| monkeypatch.setattr( | ||
| lark_parser.datetime, | ||
| "date", | ||
| MagicMock(today=MagicMock(return_value=fixed_date)), | ||
| ) |
There was a problem hiding this comment.
The fixed_date + monkeypatch.setattr(lark_parser.datetime, "date", …) block is duplicated in multiple tests in this file. To reduce repetition (and make future changes to the time-freezing approach less error-prone), consider extracting this into a small fixture/helper (e.g. freeze_lark_parser_date(monkeypatch)), then reuse it in both parametrized tests.
There was a problem hiding this comment.
It is just two tests, so the result will be about the same...
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12885 +/- ##
==========================================
+ Coverage 90.65% 90.68% +0.02%
==========================================
Files 441 442 +1
Lines 30488 30496 +8
==========================================
+ Hits 27640 27656 +16
+ Misses 2848 2840 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| ).read_text() == "I WANT TO REPLACE:my_custom_variable" | ||
|
|
||
|
|
||
| EXPECTED_DATE = "2026-01-01" |
There was a problem hiding this comment.
Could you put this at the top of the file
This commit fixes a potential timing dependent issue in `test_that_pre_defines_are_substituted_templates[<DATE>]` and `test_that_the_data_file_keyword_also_has_similar_behavior_to_run_template[<DATE>]`. These tests have been flaky at some point, and one of the potential sources of the flakiness was tests running while the date changes at midnight. This commit monkeypatches this to return `2026-01-01` at every time, making sure the timing issues will never occur.
jonathan-eq
force-pushed
the
fix-flaky-test-91
branch
from
ebc4179 to
9fa9336
Compare
|
Successfully created backport PR for |
Issue
Resolves timing issues
Approach
This commit fixes a potential timing dependent issue in
test_that_pre_defines_are_substituted_templates[<DATE>]andtest_that_the_data_file_keyword_also_has_similar_behavior_to_run_template[<DATE>]. These tests have been flaky at some point, and one of the potential sources of the flakiness was tests running while the date changes at midnight. This commit monkeypatches this to return2026-01-01at every time, making sure the timing issues will never occur.(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable