test(diff): comment on a generated file and collapse it in commit windows
- commit
- c3d039187e85bdb716f9e20132ab3b3a87f9a3e2
- author
- brice · · 2026-10-20 07:41 UTC
- parent
- c6f1c49
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +34 -8
M internal/usecase/mergerequests.go +3 -2
| @@ -780,8 +780,9 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||
| 780 | 780 | // file's hunks within the default total limits would reject a comment on a | |
| 781 | 781 | // file the earlier ones left no budget for. So a first read lists the files | |
| 782 | 782 | // with no hunks (From past the end: cheap, git is stopped at once), and a | |
| 783 | - | // second reads the hunks of the one file within the default per-file limit | |
| 784 | - | // and total limits, as the web view does for a window holding it. | |
| 783 | + | // second loads the one file in full (a Full window: no line cap, the byte | |
| 784 | + | // guard only, generated files included), as the web view does when it loads | |
| 785 | + | // a collapsed file: any line the page can show can be commented on. | |
| 785 | 786 | list := domain.DefaultDiffLimits() | |
| 786 | 787 | list.MaxFiles, list.From = 0, math.MaxInt | |
| 787 | 788 | _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list) | |
| @@ -780,8 +780,9 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||||
| 780 | // file's hunks within the default total limits would reject a comment on a | 780 | // file's hunks within the default total limits would reject a comment on a | ||
| 781 | // file the earlier ones left no budget for. So a first read lists the files | 781 | // file the earlier ones left no budget for. So a first read lists the files | ||
| 782 | // with no hunks (From past the end: cheap, git is stopped at once), and a | 782 | // with no hunks (From past the end: cheap, git is stopped at once), and a | ||
| 783 | - | // second reads the hunks of the one file within the default per-file limit | 783 | + | // second loads the one file in full (a Full window: no line cap, the byte |
| 784 | - | // and total limits, as the web view does for a window holding it. | 784 | + | // guard only, generated files included), as the web view does when it loads |
| 785 | + | // a collapsed file: any line the page can show can be commented on. | |||
| 785 | list := domain.DefaultDiffLimits() | 786 | list := domain.DefaultDiffLimits() | ||
| 786 | list.MaxFiles, list.From = 0, math.MaxInt | 787 | list.MaxFiles, list.From = 0, math.MaxInt | ||
| 787 | _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list) | 788 | _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list) | ||
M internal/usecase/window_test.go +31 -6
| @@ -100,17 +100,21 @@ func TestCheckLineBeyondFileLimit(t *testing.T) { | |||
| 100 | 100 | r.seed() | |
| 101 | 101 | r.sh("checkout", "-q", "-B", "feat", "main") | |
| 102 | 102 | r.commit(author, "big.txt", bigContent(3000), "big") | |
| 103 | + | r.commit(author, "go.sum", "h1:x\nh1:y\n", "sum") | |
| 103 | 104 | r.push("feat") | |
| 104 | 105 | n := r.open("feat").Number | |
| 105 | 106 | ||
| 106 | - | add := func(line int) error { | |
| 107 | - | _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: "big.txt", Line: line, Side: domain.SideNew, Text: "c"}) | |
| 107 | + | add := func(path string, line int) error { | |
| 108 | + | _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: path, Line: line, Side: domain.SideNew, Text: "c"}) | |
| 108 | 109 | return err | |
| 109 | 110 | } | |
| 110 | - | require.NoError(t, add(1)) | |
| 111 | - | require.NoError(t, add(2500)) | |
| 112 | - | require.NoError(t, add(3000)) | |
| 113 | - | require.ErrorIs(t, add(3001), domain.ErrInvalid) | |
| 111 | + | require.NoError(t, add("big.txt", 1)) | |
| 112 | + | require.NoError(t, add("big.txt", 2500)) | |
| 113 | + | require.NoError(t, add("big.txt", 3000)) | |
| 114 | + | require.ErrorIs(t, add("big.txt", 3001), domain.ErrInvalid) | |
| 115 | + | // A generated file is collapsed in the web windows, but loadable: its | |
| 116 | + | // lines can be commented on. | |
| 117 | + | require.NoError(t, add("go.sum", 2)) | |
| 114 | 118 | } | |
| 115 | 119 | ||
| 116 | 120 | func TestBrowseGetCommitFullWindow(t *testing.T) { | |
| @@ -139,3 +143,24 @@ func TestBrowseGetCommitFullWindow(t *testing.T) { | |||
| 139 | 143 | _, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}}) | |
| 140 | 144 | require.ErrorIs(t, err, domain.ErrInvalid) | |
| 141 | 145 | } | |
| 146 | + | ||
| 147 | + | func TestBrowseGetCommitWindowCollapsesGenerated(t *testing.T) { | |
| 148 | + | r := newMRRig(t) | |
| 149 | + | r.seed() | |
| 150 | + | r.commit(author, "go.sum", "h1:x\n", "sum") | |
| 151 | + | id := r.head() | |
| 152 | + | r.push("main") | |
| 153 | + | b := usecase.NewBrowse(r.store, r.git) | |
| 154 | + | rev := domain.RevisionOf(id) | |
| 155 | + | ||
| 156 | + | d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{}) | |
| 157 | + | require.NoError(t, err) | |
| 158 | + | require.Len(t, d.Diff.Files, 1) | |
| 159 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "API reads keep generated files") | |
| 160 | + | ||
| 161 | + | d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 25}}) | |
| 162 | + | require.NoError(t, err) | |
| 163 | + | require.Len(t, d.Diff.Files, 1) | |
| 164 | + | assert.Equal(t, domain.OmitGenerated, d.Diff.Files[0].Omitted, "web windows collapse them") | |
| 165 | + | assert.Empty(t, d.Diff.Files[0].Hunks) | |
| 166 | + | } | |
| @@ -100,17 +100,21 @@ func TestCheckLineBeyondFileLimit(t *testing.T) { | |||||
| 100 | r.seed() | 100 | r.seed() | ||
| 101 | r.sh("checkout", "-q", "-B", "feat", "main") | 101 | r.sh("checkout", "-q", "-B", "feat", "main") | ||
| 102 | r.commit(author, "big.txt", bigContent(3000), "big") | 102 | r.commit(author, "big.txt", bigContent(3000), "big") | ||
| 103 | + | r.commit(author, "go.sum", "h1:x\nh1:y\n", "sum") | |||
| 103 | r.push("feat") | 104 | r.push("feat") | ||
| 104 | n := r.open("feat").Number | 105 | n := r.open("feat").Number | ||
| 105 | 106 | ||||
| 106 | - | add := func(line int) error { | 107 | + | add := func(path string, line int) error { |
| 107 | - | _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: "big.txt", Line: line, Side: domain.SideNew, Text: "c"}) | 108 | + | _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: path, Line: line, Side: domain.SideNew, Text: "c"}) |
| 108 | return err | 109 | return err | ||
| 109 | } | 110 | } | ||
| 110 | - | require.NoError(t, add(1)) | 111 | + | require.NoError(t, add("big.txt", 1)) |
| 111 | - | require.NoError(t, add(2500)) | 112 | + | require.NoError(t, add("big.txt", 2500)) |
| 112 | - | require.NoError(t, add(3000)) | 113 | + | require.NoError(t, add("big.txt", 3000)) |
| 113 | - | require.ErrorIs(t, add(3001), domain.ErrInvalid) | 114 | + | require.ErrorIs(t, add("big.txt", 3001), domain.ErrInvalid) |
| 115 | + | // A generated file is collapsed in the web windows, but loadable: its | |||
| 116 | + | // lines can be commented on. | |||
| 117 | + | require.NoError(t, add("go.sum", 2)) | |||
| 114 | } | 118 | } | ||
| 115 | 119 | ||||
| 116 | func TestBrowseGetCommitFullWindow(t *testing.T) { | 120 | func TestBrowseGetCommitFullWindow(t *testing.T) { | ||
| @@ -139,3 +143,24 @@ func TestBrowseGetCommitFullWindow(t *testing.T) { | |||||
| 139 | _, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}}) | 143 | _, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}}) | ||
| 140 | require.ErrorIs(t, err, domain.ErrInvalid) | 144 | require.ErrorIs(t, err, domain.ErrInvalid) | ||
| 141 | } | 145 | } | ||
| 146 | + | ||||
| 147 | + | func TestBrowseGetCommitWindowCollapsesGenerated(t *testing.T) { | |||
| 148 | + | r := newMRRig(t) | |||
| 149 | + | r.seed() | |||
| 150 | + | r.commit(author, "go.sum", "h1:x\n", "sum") | |||
| 151 | + | id := r.head() | |||
| 152 | + | r.push("main") | |||
| 153 | + | b := usecase.NewBrowse(r.store, r.git) | |||
| 154 | + | rev := domain.RevisionOf(id) | |||
| 155 | + | ||||
| 156 | + | d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{}) | |||
| 157 | + | require.NoError(t, err) | |||
| 158 | + | require.Len(t, d.Diff.Files, 1) | |||
| 159 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "API reads keep generated files") | |||
| 160 | + | ||||
| 161 | + | d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 25}}) | |||
| 162 | + | require.NoError(t, err) | |||
| 163 | + | require.Len(t, d.Diff.Files, 1) | |||
| 164 | + | assert.Equal(t, domain.OmitGenerated, d.Diff.Files[0].Omitted, "web windows collapse them") | |||
| 165 | + | assert.Empty(t, d.Diff.Files[0].Hunks) | |||
| 166 | + | } | |||