Merge pull request 'test(kvstore): accept a conflict from SetRead racing with a repository delete' (#16) from fix/kvstore-delete-race-test into main

commit: 9b35c2577f2ec5dbf579ac6510908034e8d1b759
author: bnema · 9d ago · 2026-10-12 19:50 UTC
merge of: f3afb66 + a358636 · diff against the first parent
signature: SSH UnverifiedSHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivA The key is unknown

2 files · +6 -3

M internal/adapters/kvstore/mergerequests_test.go +3 -2


@@ -358,7 +358,8 @@ func assertNoMergeRequestKeys(t testing.T, mrs kvstore.MergeRequests, n domain
358	358		}
359	359
360	360		// TestDeleteRacingWithWrites: writes racing with Delete either land before it
361		-	// and are deleted, or are refused after it; nothing is left for a repository
	361	+	// and are deleted, or are refused (not found after it, or a conflict when the
	362	+	// retries run out under this contention); nothing is left for a repository
362	363		// created again under the name.
363	364		func TestDeleteRacingWithWrites(t *testing.T) {
364	365		ctx := context.Background()

@@ -376,7 +377,7 @@ func TestDeleteRacingWithWrites(t *testing.T) {
376	377		if w%2 == 0 {
377	378		err := mrs.SetRead(ctx, n, mr.Number, oid('a'))
378	379		if err != nil {
379		-	assert.ErrorIs(t, err, domain.ErrNotFound)
	380	+	assert.True(t, errors.Is(err, domain.ErrConflict) \|\| errors.Is(err, domain.ErrNotFound), "round %d: %v", round, err)
380	381		return
381	382		}
382	383		continue

 }
 // TestDeleteRacingWithWrites: writes racing with Delete either land before it
-// and are deleted, or are refused after it; nothing is left for a repository
+// and are deleted, or are refused (not found after it, or a conflict when the
+// retries run out under this contention); nothing is left for a repository
 // created again under the name.
 func TestDeleteRacingWithWrites(t *testing.T) {
 	ctx := context.Background()
 					if w%2 == 0 {
 						err := mrs.SetRead(ctx, n, mr.Number, oid('a'))
 						if err != nil {
-							assert.ErrorIs(t, err, domain.ErrNotFound)
+							assert.True(t, errors.Is(err, domain.ErrConflict) || errors.Is(err, domain.ErrNotFound), "round %d: %v", round, err)
 							return
 						}
 						continue

M internal/ports/ports.go +3 -1


@@ -255,7 +255,9 @@ type MergeRequestStore interface {
255	255		// ObjectID when there is none.
256	256		GetRead(ctx context.Context, repo domain.RepoName, number int64) (domain.ObjectID, error)
257	257		// SetRead records the head the reviewer has read. It returns
258		-	// domain.ErrNotFound when the repository does not exist.
	258	+	// domain.ErrNotFound when the repository does not exist, and
	259	+	// domain.ErrConflict when concurrent writes to the repository keep it
	260	+	// from landing.
259	261		SetRead(ctx context.Context, repo domain.RepoName, number int64, head domain.ObjectID) error
260	262		}
261	263

 	// ObjectID when there is none.
 	GetRead(ctx context.Context, repo domain.RepoName, number int64) (domain.ObjectID, error)
 	// SetRead records the head the reviewer has read. It returns
-	// domain.ErrNotFound when the repository does not exist.
+	// domain.ErrNotFound when the repository does not exist, and
+	// domain.ErrConflict when concurrent writes to the repository keep it
+	// from landing.
 	SetRead(ctx context.Context, repo domain.RepoName, number int64, head domain.ObjectID) error
 }