allegro / allegro/bigcache

TestCacheLen is flaky: a 1s LifeWindow expires entries at the next Unix second, not 1s after insertion

オープン
#433 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Go
スター
8.2k
フォーク
614
平均マージ
5日 12時間
マージ済み PR(30日)
1

説明

`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.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。