TestCacheLen is flaky: a 1s LifeWindow expires entries at the next Unix second, not 1s after insertion
- Ngôn ngữ chính
- Go
- Star
- 8.2k
- Fork
- 614
- Merge trung bình
- 5 ngày 12 giờ
- Pull request đã merge (30 ngày)
- 1
Mô tả
`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.
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Đánh giá
Issue này chưa được đánh giá.