Skip to content

[17.0][FIX] mail_notify_employee_leave: make absence tests time independent - #260

Merged
OCA-git-bot merged 2 commits into
OCA:17.0from
Jarsa-dev:17.0-fix-mail_notify_employee_leave-flaky-tests
Aug 28, 2026
Merged

[17.0][FIX] mail_notify_employee_leave: make absence tests time independent#260
OCA-git-bot merged 2 commits into
OCA:17.0from
Jarsa-dev:17.0-fix-mail_notify_employee_leave-flaky-tests

Conversation

@alan196

@alan196 alan196 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

TestNotifyEmployeeLeave fails depending on the time of day the CI runs.

_create_validated_leave() creates a full-day leave (00:00-23:59), but hr.leave
narrows it to the working schedule of the employee, so with the standard company
calendar (08:00-17:00 Europe/Brussels) the stored leave is 06:00-15:00 UTC. The
tests then rely on employee.is_absent, which is computed as
date_from <= now <= date_to, so they only pass while the runner clock happens
to be inside that window.

Reproduced locally: with the clock frozen at 20:00 the two tests fail exactly as
in CI; inside the window they pass. Recent runs match: the 17.0 post-merge run at
07:10 UTC was green, PR runs at 19:20 and 21:02 UTC were red (e.g. #258).

Fix: freeze the clock at a working hour of a working day, so the tests no longer
depend on when they run.

# (08:00-17:00 in the timezone of the company calendar). Freeze the clock inside
# that window, otherwise the result depends on the time of the day at which the
# tests happen to run.
@freeze_time("2023-05-15 10:00:00")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At class level, there are problems with this having side effects. Please do it at method level (including if neeeded setUpClass).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, moved to method level in 939d360.

@freeze_time is now applied only on the two tests that need the employee to be absent (test_notify_when_user_absent and test_only_one_notification_per_day), with the timestamp in a WORKING_HOUR constant. setUpClass is no longer frozen, so the setup has no side effects.

_create_validated_leave() is no longer a @classmethod, as it's now called from the test methods (inside the frozen window) instead of the class setup.

Tests pass locally at a time of day that used to break them (23:17 UTC): 0 failed, 0 error(s) of 3 tests.

Apply ``freeze_time`` on the tests that need an absent employee instead of
on the whole class, to avoid side effects on the rest of the test setup.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWjFDR8XPZuNbFHKm1aJEc

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-260-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 0c3c557 into OCA:17.0 Aug 28, 2026
7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at f107f79. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants