refactor: simplify/extract logic to assert GC review due dates
The following discussion from !1100 (merged) should be addressed:
-
@jaime started a discussion: minor nit: we use
defaultReviewAfterDelay+minReviewAfterJitter
anddefaultReviewAfterDelay+ maxReviewAfterJitter
in many places so they could be defined as constants at the top of this file.We also do a bunch of assertions repeatedly so they could be extracted into an
assertGCReviews
function for example. This can probably be done later as a nice-to-have😄
Implementation Guide
The registry/datastore/gc_integration_test.go
test file currently contains several instances of the following assertions:
require.Greater(t, rr[0].ReviewAfter, b.CreatedAt.Add(defaultReviewAfterDelay+minReviewAfterJitter))
require.Less(t, rr[0].ReviewAfter, b.CreatedAt.Add(defaultReviewAfterDelay+maxReviewAfterJitter))
These assertions test that the review delay for the review queue object falls within the expected time. We use a jitter to reduce contention over the review queue since it's possible for many reviews to be queued simultaneously, and since multiple instances of manifest or blob workers may be attempting to process queue items concurrently.
We can simplify these statements in two ways, first by consolidating defaultReviewAfterDelay+minReviewAfterJitter
and defaultReviewAfterDelay+maxReviewAfterJitter
into constants representing the min and max expected ReviewAfterDelay respectively. Secondly, we can use require.WithinRange
to replace the require.Greater
and require.Less
calls with a single assertion.
Testing
The following command can be used to test locally:
go test -v -tags=integration -failfast github.com/docker/distribution/registry/datastore
The integration
tag is conventionally used in this project to enable tests that require a connection to a database. Setting up a local Postgres 12 database can speed up test cycles compared to CI jobs.
The tests will automatically handle the migrations and setup/teardown operations they require to run.
These tests use several environment variables to connect to the database.
See the .gitlab-ci.yml
for these variables, all named starting with REGISTRY_DATABASE_
.
You'll need to set these variables so that they correspond to the database you intend the tests to connect to.