test(mr): stronger note retry and range tests

commit: 3872b20b994eb46f99a0eb01998f5daa2c145028
author: brice · 3d ago · 2026-10-19 05:54 UTC
parent: 31c9416
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +60 -20

M internal/usecase/export_test.go +0 -3


@@ -15,9 +15,6 @@ import (
15	15		"bnema.dev/ermite/internal/ports"
16	16		)
17	17
18		-	// AuthorOf exposes authorOf to the tests.
19		-	func AuthorOf(p domain.Principal) string { return domain.AuthorName(p) }
20		-
21	18		// Sweep exposes the daily sweep to the tests.
22	19		func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }
23	20


@@ -15,9 +15,6 @@ import (
15		"bnema.dev/ermite/internal/ports"	15	"bnema.dev/ermite/internal/ports"
16		)	16	)
17			17
18	-	// AuthorOf exposes authorOf to the tests.
19	-	func AuthorOf(p domain.Principal) string { return domain.AuthorName(p) }
20	-
21		// Sweep exposes the daily sweep to the tests.	18	// Sweep exposes the daily sweep to the tests.
22		func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }	19	func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }
23			20

M internal/usecase/mergerequests_integration_test.go +16 -0


@@ -1347,6 +1347,22 @@ func TestCommentsPerMergeRequestAreBounded(t *testing.T) {
1347	1347		}
1348	1348
1349	1349		func TestNoteThreadsRangesAndVerdicts(t *testing.T) {
	1350	+	t.Run("start outside diff", func(t *testing.T) {
	1351	+	r := newMRRig(t)
	1352	+	r.seed()
	1353	+	before := strings.Repeat("unchanged\n", 30)
	1354	+	r.commit(author, "long.txt", before, "long base")
	1355	+	r.push("main")
	1356	+	r.sh("checkout", "-q", "-B", "range", "main")
	1357	+	r.commit(author, "long.txt", before+"changed\n", "late hunk")
	1358	+	r.push("range")
	1359	+	mr := r.open("range")
	1360	+	_, err := r.mrs.AddComment(r.ctx, root, r.repo, mr.Number, ports.AddCommentParams{Path: "long.txt", Line: 31, Side: domain.SideNew, Text: "valid"})
	1361	+	require.NoError(t, err)
	1362	+	_, err = r.mrs.AddComment(r.ctx, root, r.repo, mr.Number, ports.AddCommentParams{Path: "long.txt", StartLine: 1, Line: 31, Side: domain.SideNew, Text: "invalid range"})
	1363	+	require.ErrorIs(t, err, domain.ErrInvalid)
	1364	+	assert.Contains(t, fieldsOf(t, err), "start_line")
	1365	+	})
1350	1366		r := newMRRig(t)
1351	1367		r.seed()
1352	1368		head := r.feature("feat")


@@ -1347,6 +1347,22 @@ func TestCommentsPerMergeRequestAreBounded(t *testing.T) {
1347	}	1347		}
1348		1348
1349	func TestNoteThreadsRangesAndVerdicts(t *testing.T) {	1349		func TestNoteThreadsRangesAndVerdicts(t *testing.T) {
		1350	+	t.Run("start outside diff", func(t *testing.T) {
		1351	+	r := newMRRig(t)
		1352	+	r.seed()
		1353	+	before := strings.Repeat("unchanged\n", 30)
		1354	+	r.commit(author, "long.txt", before, "long base")
		1355	+	r.push("main")
		1356	+	r.sh("checkout", "-q", "-B", "range", "main")
		1357	+	r.commit(author, "long.txt", before+"changed\n", "late hunk")
		1358	+	r.push("range")
		1359	+	mr := r.open("range")
		1360	+	_, err := r.mrs.AddComment(r.ctx, root, r.repo, mr.Number, ports.AddCommentParams{Path: "long.txt", Line: 31, Side: domain.SideNew, Text: "valid"})
		1361	+	require.NoError(t, err)
		1362	+	_, err = r.mrs.AddComment(r.ctx, root, r.repo, mr.Number, ports.AddCommentParams{Path: "long.txt", StartLine: 1, Line: 31, Side: domain.SideNew, Text: "invalid range"})
		1363	+	require.ErrorIs(t, err, domain.ErrInvalid)
		1364	+	assert.Contains(t, fieldsOf(t, err), "start_line")
		1365	+	})
1350	r := newMRRig(t)	1366		r := newMRRig(t)
1351	r.seed()	1367		r.seed()
1352	head := r.feature("feat")	1368		head := r.feature("feat")

M internal/usecase/mergerequests_test.go +44 -8


@@ -6,6 +6,7 @@ import (
6	6		"context"
7	7		"errors"
8	8		"testing"
	9	+	"time"
9	10
10	11		"github.com/stretchr/testify/assert"
11	12		"github.com/stretchr/testify/mock"

@@ -127,27 +128,62 @@ func TestNoteMutationRetries(t *testing.T) {
127	128		text, err := domain.ParsePlainText("before")
128	129		require.NoError(t, err)
129	130		c := domain.Comment{ID: 1, Author: "root", Text: text}
130		-	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Twice()
131		-	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Twice()
	131	+	freshMR := mr
	132	+	freshMR.Version = 2
	133	+	fresh := c
	134	+	fresh.Version = 3
	135	+	fresh.CreatedAt = now.Add(-time.Hour)
	136	+	if action != "resolve" {
	137	+	fresh.Resolved = true
	138	+	}
	139	+	want := fresh
	140	+	switch action {
	141	+	case "edit":
	142	+	want.Text, err = domain.ParsePlainText("after")
	143	+	require.NoError(t, err)
	144	+	want.EditedAt = now.UTC()
	145	+	case "delete":
	146	+	want.Text, err = domain.ParsePlainText("")
	147	+	require.NoError(t, err)
	148	+	want.Deleted = true
	149	+	want.EditedAt = now.UTC()
	150	+	case "resolve":
	151	+	want.Resolved = true
	152	+	}
	153	+	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Once()
	154	+	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(freshMR, nil).Once()
	155	+	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Once()
	156	+	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(fresh, nil).Once()
132	157		u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once()
133		-	u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
134		-	// The bus expectation must be exactly once despite the conflict.
	158	+	u.store.EXPECT().SaveComment(mock.Anything, mock.MatchedBy(func(saved domain.MergeRequest) bool {
	159	+	return saved.Version == freshMR.Version
	160	+	}), mock.MatchedBy(func(saved domain.Comment) bool { return saved == want })).Return(nil).Once()
135	161		bus := mocks.NewEventBus(t)
136	162		bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool {
137		-	return e.EventKind() == "mr.comment_"+map[string]string{"edit": "edited", "delete": "deleted", "resolve": "resolved"}[action]
	163	+	switch event := e.(type) {
	164	+	case domain.CommentEdited:
	165	+	return action == "edit" && event.Comment == want
	166	+	case domain.CommentDeleted:
	167	+	return action == "delete" && event.Comment == want
	168	+	case domain.CommentResolved:
	169	+	return action == "resolve" && event.Comment == want
	170	+	}
	171	+	return false
138	172		})).Return().Once()
139	173		clock := mocks.NewClock(t)
140	174		clock.EXPECT().Now().Return(now)
141	175		svc := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Bus: bus, Clock: clock})
	176	+	var got domain.Comment
142	177		switch action {
143	178		case "edit":
144		-	_, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")
	179	+	got, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")
145	180		case "delete":
146		-	_, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)
	181	+	got, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)
147	182		case "resolve":
148		-	_, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)
	183	+	got, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)
149	184		}
150	185		require.NoError(t, err)
	186	+	assert.Equal(t, want, got)
151	187		})
152	188		}
153	189		}


@@ -6,6 +6,7 @@ import (
6		"context"	6		"context"
7		"errors"	7		"errors"
8		"testing"	8		"testing"
			9	+	"time"
9			10
10		"github.com/stretchr/testify/assert"	11		"github.com/stretchr/testify/assert"
11		"github.com/stretchr/testify/mock"	12		"github.com/stretchr/testify/mock"

@@ -127,27 +128,62 @@ func TestNoteMutationRetries(t *testing.T) {
127		text, err := domain.ParsePlainText("before")	128		text, err := domain.ParsePlainText("before")
128		require.NoError(t, err)	129		require.NoError(t, err)
129		c := domain.Comment{ID: 1, Author: "root", Text: text}	130		c := domain.Comment{ID: 1, Author: "root", Text: text}
130	-	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Twice()	131	+	freshMR := mr
131	-	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Twice()	132	+	freshMR.Version = 2
			133	+	fresh := c
			134	+	fresh.Version = 3
			135	+	fresh.CreatedAt = now.Add(-time.Hour)
			136	+	if action != "resolve" {
			137	+	fresh.Resolved = true
			138	+	}
			139	+	want := fresh
			140	+	switch action {
			141	+	case "edit":
			142	+	want.Text, err = domain.ParsePlainText("after")
			143	+	require.NoError(t, err)
			144	+	want.EditedAt = now.UTC()
			145	+	case "delete":
			146	+	want.Text, err = domain.ParsePlainText("")
			147	+	require.NoError(t, err)
			148	+	want.Deleted = true
			149	+	want.EditedAt = now.UTC()
			150	+	case "resolve":
			151	+	want.Resolved = true
			152	+	}
			153	+	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Once()
			154	+	u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(freshMR, nil).Once()
			155	+	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Once()
			156	+	u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(fresh, nil).Once()
132		u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once()	157		u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once()
133	-	u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()	158	+	u.store.EXPECT().SaveComment(mock.Anything, mock.MatchedBy(func(saved domain.MergeRequest) bool {
134	-	// The bus expectation must be exactly once despite the conflict.	159	+	return saved.Version == freshMR.Version
			160	+	}), mock.MatchedBy(func(saved domain.Comment) bool { return saved == want })).Return(nil).Once()
135		bus := mocks.NewEventBus(t)	161		bus := mocks.NewEventBus(t)
136		bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool {	162		bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool {
137	-	return e.EventKind() == "mr.comment_"+map[string]string{"edit": "edited", "delete": "deleted", "resolve": "resolved"}[action]	163	+	switch event := e.(type) {
			164	+	case domain.CommentEdited:
			165	+	return action == "edit" && event.Comment == want
			166	+	case domain.CommentDeleted:
			167	+	return action == "delete" && event.Comment == want
			168	+	case domain.CommentResolved:
			169	+	return action == "resolve" && event.Comment == want
			170	+	}
			171	+	return false
138		})).Return().Once()	172		})).Return().Once()
139		clock := mocks.NewClock(t)	173		clock := mocks.NewClock(t)
140		clock.EXPECT().Now().Return(now)	174		clock.EXPECT().Now().Return(now)
141		svc := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Bus: bus, Clock: clock})	175		svc := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Bus: bus, Clock: clock})
			176	+	var got domain.Comment
142		switch action {	177		switch action {
143		case "edit":	178		case "edit":
144	-	_, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")	179	+	got, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")
145		case "delete":	180		case "delete":
146	-	_, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)	181	+	got, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)
147		case "resolve":	182		case "resolve":
148	-	_, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)	183	+	got, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)
149		}	184		}
150		require.NoError(t, err)	185		require.NoError(t, err)
			186	+	assert.Equal(t, want, got)
151		})	187		})
152		}	188		}
153		}	189		}

M internal/usecase/sessions_test.go +0 -9


@@ -334,12 +334,3 @@ func TestSignOutEverywhereEndsSessions(t *testing.T) {
334	334		_, err = f.sessions.Verify(ctx, c)
335	335		assert.NoError(t, err, "a new session works")
336	336		}
337		-
338		-	// The owner signed in on the web signs what it writes as "owner", where root
339		-	// signs as "root" although both hold every right.
340		-	func TestAuthorOfOwnerSession(t *testing.T) {
341		-	t.Parallel()
342		-	assert.Equal(t, "owner", usecase.AuthorOf(domain.Owner()))
343		-	assert.Equal(t, "root", usecase.AuthorOf(domain.Root()))
344		-	assert.Equal(t, "ci", usecase.AuthorOf(domain.TokenPrincipal(domain.Token{Label: "ci", Scopes: domain.AllScopes()})))
345		-	}


@@ -334,12 +334,3 @@ func TestSignOutEverywhereEndsSessions(t *testing.T) {
334		_, err = f.sessions.Verify(ctx, c)	334	_, err = f.sessions.Verify(ctx, c)
335		assert.NoError(t, err, "a new session works")	335	assert.NoError(t, err, "a new session works")
336		}	336	}
337	-
338	-	// The owner signed in on the web signs what it writes as "owner", where root
339	-	// signs as "root" although both hold every right.
340	-	func TestAuthorOfOwnerSession(t *testing.T) {
341	-	t.Parallel()
342	-	assert.Equal(t, "owner", usecase.AuthorOf(domain.Owner()))
343	-	assert.Equal(t, "root", usecase.AuthorOf(domain.Root()))
344	-	assert.Equal(t, "ci", usecase.AuthorOf(domain.TokenPrincipal(domain.Token{Label: "ci", Scopes: domain.AllScopes()})))
345	-	}