Skip to content

image/blobinfocache: do not assume time.Now() is unique in tests - #1038

Open
vtushar06 wants to merge 1 commit into
podman-container-tools:mainfrom
vtushar06:bic-test-clock-resolution
Open

image/blobinfocache: do not assume time.Now() is unique in tests#1038
vtushar06 wants to merge 1 commit into
podman-container-tools:mainfrom
vtushar06:bic-test-clock-resolution

Conversation

@vtushar06

Copy link
Copy Markdown

Closes #1035.

I could not get it to fail on my machine - it reports a 250ns tick and only 0.01% duplicates from time.Now() - so I measured the mechanism instead. A RecordKnownLocation on the memory cache takes about 172ns here, which is less than one tick on a machine with microsecond resolution, so two locations recorded one after the other cannot get distinct timestamps there.

To see which tests actually depend on it I truncated the recorded time to a millisecond to stand in for a very coarse clock. That fails six subtests, not one: RecordKnownLocations, CandidateLocations and CandidateLocations2, in both the no Open and with Open variants. So the assumption is in more places than the one you hit.

The fix is a small wrapper that records a location and then waits for time.Now() to change, used at the four ordering sensitive call sites. Waiting for the clock rather than sleeping a fixed amount means it costs one tick whatever the platform resolution is. With the same truncation applied to both the cache and the wrapper the six subtests pass again, and normally the whole thing is under the noise - memory, boltdb and sqlite all measure the same as before within a tenth of a second.

I left the single record in the unknown-location test alone since nothing depends on its order.

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Copilot AI review requested due to automatic review settings July 29, 2026 02:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the image Related to "image" package label Jul 29, 2026
@vtushar06

Copy link
Copy Markdown
Author

@mtrmac the part I was not expecting is that CandidateLocations and CandidateLocations2 depend on this too, not just RecordKnownLocationsand A/B ordering in those comes from two records right next to each other
cc @Luap99

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

Labels

image Related to "image" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlobInfoCache’s assumption that time.Now() values are unique is incorrect

2 participants