Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky tst-login-logout (rotation) test #24

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

andy-bower
Copy link
Contributor

I think I've fixed the flaky tst-login-logout test which would fail on some proportion of runs if you run the tests on a loop:

4/4 tst-login-logout              FAIL            0.11s   exit status 1
>>> UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_PERTURB_=95 MESON_TEST_ITERATION=1 LD_LIBRARY_PATH=/ubuntu/home/andy/git/wtmpdb/build/ MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 /ubuntu/home/andy/git/wtmpdb/build/tests/tst-login-logout
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
wtmpdb_read_all returned 3 expected 5
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――


Summary of Failures:

4/4 tst-login-logout       FAIL            0.11s   exit status 1

The commit logs cover my findings:

rotate: don't throw away microseconds calculating threshold

Throwing away the microseconds from the rotation threshold doesn't affect real world usage but it does make tests unreliable because it means the outcome is sensitive to exactly which part of a second the test is running in and if near the rollover point then a different result is obtained.

test: schedule test logins on correct day

As the current time is moving on throught the test, the calculation of "n days ago" ends up picking a time which is slightly more than "n days ago", so reduce n by one. This error was mostly neutralised by the rotation threshold throwing away microseconds, but with that fixed, the tests always fail unless this is also fixed.

Throwing away the microseconds from the rotation threshold doesn't affect real
world usage but it does make tests unreliable because it means the outcome is
sensitive to exactly which part of a second the test is running in and if near
the rollover point then a different result is obtained.
As the current time is moving on throught the test, the calculation of "n days
ago" ends up picking a time which is slightly more than "n days ago", so reduce
n by one. This error was mostly neutralised by the rotation threshold throwing
away microseconds, but with that fixed, the tests always fail unless this is
also fixed.
@thkukuk
Copy link
Owner

thkukuk commented Feb 10, 2025

Thanks for the analysis and the fix for it.

@thkukuk thkukuk merged commit 96c775e into thkukuk:main Feb 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants