TestCacheLen is flaky: a 1s LifeWindow expires entries at the next Unix second, not 1s after insertion
- Langage dominant
- Go
- Étoiles
- 8.2k
- Forks
- 614
- Merge moyen
- 5 j 12 h
- PR mergées (30 j)
- 1
Description
`TestCacheLen` fails intermittently on CI with counts well below 1337:
```
expected: 1337, actual: 933
```
It isn't a slow-machine problem. The loop doesn't need to take a second, it only needs to cross a second boundary.
`clock.Epoch()` returns `time.Now().Unix()`, so timestamps have whole-second resolution. `isExpired` is
```go
return currentTimestamp-oldestTimestamp >= s.lifeWindow
```
and the test sets `LifeWindow: time.Second`. An entry written at Unix second T counts as expired as soon as the clock reads T+1, which can be a millisecond later. `set` calls `onEvict` on the oldest entry, so every `Set` after the crossing evicts one. Crossing at iteration k leaves roughly k entries, and 933 is consistent with that. `t.Parallel()` means the loop starts at an arbitrary phase within the second, which is why it usually passes.
I couldn't get it to fail by just running the suite (master passed 3 times plain, and 3 more under CPU load). Phase-aligning it reproduces it every time. On master `ae1c781`, sleeping until ~5ms before the next second and then running the identical loop:
```
attempt 0: expected 1337, got 1204
attempt 1: expected 1337, got 957
attempt 2: expected 1337, got 1058
```
Happy to send a PR if you have a preference on the fix. I can pin the test to a mock clock, or raise `LifeWindow` so a boundary crossing stops mattering. There's arguably a separate question about whether a 1s `LifeWindow` should mean "until the next second" rather than "for one second", but that's a behaviour change and I didn't want to assume.
I used a coding agent to help pin down the repro.
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Évaluation
Cette issue n'a pas encore été évaluée.