test(mr): cover branch deletion over the API and the web box; document what mr:merge allows
- commit
- 0ba3d35e5aaccd68da5d0beefe3e68a4ba5e296e
- author
- brice · · 2026-10-11 05:53 UTC
- parent
- 5cb495b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +60 -3
M README.md +1 -1
| @@ -64,7 +64,7 @@ Use the admin token (`ERMITE_ADMIN_TOKEN`) or a stored token as the password; th | |||
| 64 | 64 | ||
| 65 | 65 | ## Merge requests | |
| 66 | 66 | ||
| 67 | - | A merge request proposes a branch for its target (the default branch unless set). The merge is refused when the branch moved since the reviewed head (`--head`) or when the target is not an ancestor of it. Fast-forward moves the target; squash writes one commit signed by the server's SSH key (`ermite signing-key show` prints the `allowed_signers` line for `git verify-commit`). `ermite repo edit --merge-modes` limits the modes per repository. Tokens need `mr:write` to open and comment, and `mr:merge` to merge. | |
| 67 | + | A merge request proposes a branch for its target (the default branch unless set). The merge is refused when the branch moved since the reviewed head (`--head`) or when the target is not an ancestor of it. Fast-forward moves the target; squash writes one commit signed by the server's SSH key (`ermite signing-key show` prints the `allowed_signers` line for `git verify-commit`). `ermite repo edit --merge-modes` limits the modes per repository. `--delete-branch` (a checkbox on the web) deletes the source branch after the merge, unless it is the default branch or another open request uses it. Tokens need `mr:write` to open and comment, and `mr:merge` to merge (and delete the merged branch). | |
| 68 | 68 | ||
| 69 | 69 | ## Web sign-in | |
| 70 | 70 | ||
| @@ -64,7 +64,7 @@ Use the admin token (`ERMITE_ADMIN_TOKEN`) or a stored token as the password; th | |||||
| 64 | 64 | ||||
| 65 | ## Merge requests | 65 | ## Merge requests | ||
| 66 | 66 | ||||
| 67 | - | A merge request proposes a branch for its target (the default branch unless set). The merge is refused when the branch moved since the reviewed head (`--head`) or when the target is not an ancestor of it. Fast-forward moves the target; squash writes one commit signed by the server's SSH key (`ermite signing-key show` prints the `allowed_signers` line for `git verify-commit`). `ermite repo edit --merge-modes` limits the modes per repository. Tokens need `mr:write` to open and comment, and `mr:merge` to merge. | 67 | + | A merge request proposes a branch for its target (the default branch unless set). The merge is refused when the branch moved since the reviewed head (`--head`) or when the target is not an ancestor of it. Fast-forward moves the target; squash writes one commit signed by the server's SSH key (`ermite signing-key show` prints the `allowed_signers` line for `git verify-commit`). `ermite repo edit --merge-modes` limits the modes per repository. `--delete-branch` (a checkbox on the web) deletes the source branch after the merge, unless it is the default branch or another open request uses it. Tokens need `mr:write` to open and comment, and `mr:merge` to merge (and delete the merged branch). |
| 68 | 68 | ||||
| 69 | ## Web sign-in | 69 | ## Web sign-in | ||
| 70 | 70 | ||||
M cmd/ermite/mr_api_test.go +28 -0
| @@ -365,6 +365,34 @@ func TestMergeModeNotAllowedOverTheAPI(t *testing.T) { | |||
| 365 | 365 | assert.Equal(t, r.bareGit("proj", "rev-parse", "main"), r.bareGit("proj", "rev-parse", "feat~2"), "main did not move") | |
| 366 | 366 | } | |
| 367 | 367 | ||
| 368 | + | // delete_source_branch reaches the merge and the response says what happened; | |
| 369 | + | // deleting the default branch is a failed precondition. | |
| 370 | + | func TestMergeDeletesTheSourceOverTheAPI(t *testing.T) { | |
| 371 | + | r := newMRRig(t, domain.VisibilityPublic) | |
| 372 | + | _, err := r.create(r.writer) | |
| 373 | + | require.NoError(t, err) | |
| 374 | + | res, err := r.merger.mr.MergeMergeRequest(r.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{ | |
| 375 | + | Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true, | |
| 376 | + | })) | |
| 377 | + | require.NoError(t, err) | |
| 378 | + | assert.True(t, res.Msg.GetSourceBranchDeleted()) | |
| 379 | + | assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat")) | |
| 380 | + | ||
| 381 | + | // The default branch as a source: refused before anything changes. | |
| 382 | + | r.bareGit("proj", "branch", "old", "main~1") | |
| 383 | + | main := r.bareGit("proj", "rev-parse", "main") | |
| 384 | + | _, err = r.root.mr.CreateMergeRequest(r.ctx, connect.NewRequest(&pb.CreateMergeRequestRequest{ | |
| 385 | + | Repo: "proj", Title: "back", SourceBranch: "main", TargetBranch: "old", | |
| 386 | + | })) | |
| 387 | + | require.NoError(t, err) | |
| 388 | + | _, err = r.merger.mr.MergeMergeRequest(r.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{ | |
| 389 | + | Repo: "proj", Number: 2, HeadId: main, DeleteSourceBranch: true, | |
| 390 | + | })) | |
| 391 | + | assert.Equal(t, "failed_precondition", codeName(err)) | |
| 392 | + | assert.Equal(t, main, r.bareGit("proj", "rev-parse", "main")) | |
| 393 | + | assert.NotEqual(t, main, r.bareGit("proj", "rev-parse", "old"), "not merged") | |
| 394 | + | } | |
| 395 | + | ||
| 368 | 396 | // The signing key needs no authentication, whatever the credentials, and keeps | |
| 369 | 397 | // its shape. | |
| 370 | 398 | func TestGetSigningKey(t *testing.T) { | |
| @@ -365,6 +365,34 @@ func TestMergeModeNotAllowedOverTheAPI(t *testing.T) { | |||||
| 365 | assert.Equal(t, r.bareGit("proj", "rev-parse", "main"), r.bareGit("proj", "rev-parse", "feat~2"), "main did not move") | 365 | assert.Equal(t, r.bareGit("proj", "rev-parse", "main"), r.bareGit("proj", "rev-parse", "feat~2"), "main did not move") | ||
| 366 | } | 366 | } | ||
| 367 | 367 | ||||
| 368 | + | // delete_source_branch reaches the merge and the response says what happened; | |||
| 369 | + | // deleting the default branch is a failed precondition. | |||
| 370 | + | func TestMergeDeletesTheSourceOverTheAPI(t *testing.T) { | |||
| 371 | + | r := newMRRig(t, domain.VisibilityPublic) | |||
| 372 | + | _, err := r.create(r.writer) | |||
| 373 | + | require.NoError(t, err) | |||
| 374 | + | res, err := r.merger.mr.MergeMergeRequest(r.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{ | |||
| 375 | + | Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true, | |||
| 376 | + | })) | |||
| 377 | + | require.NoError(t, err) | |||
| 378 | + | assert.True(t, res.Msg.GetSourceBranchDeleted()) | |||
| 379 | + | assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat")) | |||
| 380 | + | ||||
| 381 | + | // The default branch as a source: refused before anything changes. | |||
| 382 | + | r.bareGit("proj", "branch", "old", "main~1") | |||
| 383 | + | main := r.bareGit("proj", "rev-parse", "main") | |||
| 384 | + | _, err = r.root.mr.CreateMergeRequest(r.ctx, connect.NewRequest(&pb.CreateMergeRequestRequest{ | |||
| 385 | + | Repo: "proj", Title: "back", SourceBranch: "main", TargetBranch: "old", | |||
| 386 | + | })) | |||
| 387 | + | require.NoError(t, err) | |||
| 388 | + | _, err = r.merger.mr.MergeMergeRequest(r.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{ | |||
| 389 | + | Repo: "proj", Number: 2, HeadId: main, DeleteSourceBranch: true, | |||
| 390 | + | })) | |||
| 391 | + | assert.Equal(t, "failed_precondition", codeName(err)) | |||
| 392 | + | assert.Equal(t, main, r.bareGit("proj", "rev-parse", "main")) | |||
| 393 | + | assert.NotEqual(t, main, r.bareGit("proj", "rev-parse", "old"), "not merged") | |||
| 394 | + | } | |||
| 395 | + | ||||
| 368 | // The signing key needs no authentication, whatever the credentials, and keeps | 396 | // The signing key needs no authentication, whatever the credentials, and keeps | ||
| 369 | // its shape. | 397 | // its shape. | ||
| 370 | func TestGetSigningKey(t *testing.T) { | 398 | func TestGetSigningKey(t *testing.T) { | ||
M cmd/ermite/web_mr_test.go +29 -0
| @@ -430,6 +430,35 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) { | |||
| 430 | 430 | assert.Contains(t, stale.body, "has moved on") | |
| 431 | 431 | } | |
| 432 | 432 | ||
| 433 | + | // The box offers no deletion for the default branch; a fast-forward with the | |
| 434 | + | // box checked deletes the source. | |
| 435 | + | func TestWebMergeDeleteBranchBox(t *testing.T) { | |
| 436 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 437 | + | b := browser{r.harness} | |
| 438 | + | token := r.signIn(t) | |
| 439 | + | // old is an orphan branch: main can be proposed to it, and main stays an | |
| 440 | + | // ancestor of feat/search so that it fast-forwards. | |
| 441 | + | r.mustGit(r.wt, "checkout", "-q", "--orphan", "old") | |
| 442 | + | r.writeFile(r.wt, "old.txt", []byte("old\n")) // commits | |
| 443 | + | r.mustGit(r.wt, "push", "-q", r.url, "old") | |
| 444 | + | ||
| 445 | + | for _, mr := range []url.Values{ | |
| 446 | + | {"source": {"feat/search"}, "target": {"main"}, "title": {"Search"}}, | |
| 447 | + | {"source": {"main"}, "target": {"old"}, "title": {"Back"}}, | |
| 448 | + | } { | |
| 449 | + | require.Equal(t, http.StatusSeeOther, b.form("/proj/merge-requests", token, mr).status) | |
| 450 | + | } | |
| 451 | + | assert.NotContains(t, b.get("/proj/merge-requests/2", as(token)).body, `name="delete_branch"`, "never for the default branch") | |
| 452 | + | ||
| 453 | + | one := b.get("/proj/merge-requests/1", as(token)) | |
| 454 | + | res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}}) | |
| 455 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 456 | + | assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search")) | |
| 457 | + | done := b.get("/proj/merge-requests/1", nil) | |
| 458 | + | assert.Equal(t, 200, done.status, "the merged page reads without its branch") | |
| 459 | + | assert.NotContains(t, done.body, `href="/proj/tree/feat/search"`, "no link to a gone branch") | |
| 460 | + | } | |
| 461 | + | ||
| 433 | 462 | // The comment limit of the API is a message, not a crash. | |
| 434 | 463 | func TestWebMergeRequestLimitIsFriendly(t *testing.T) { | |
| 435 | 464 | r := newMRRepo(t, domain.VisibilityPublic) | |
| @@ -430,6 +430,35 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) { | |||||
| 430 | assert.Contains(t, stale.body, "has moved on") | 430 | assert.Contains(t, stale.body, "has moved on") | ||
| 431 | } | 431 | } | ||
| 432 | 432 | ||||
| 433 | + | // The box offers no deletion for the default branch; a fast-forward with the | |||
| 434 | + | // box checked deletes the source. | |||
| 435 | + | func TestWebMergeDeleteBranchBox(t *testing.T) { | |||
| 436 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 437 | + | b := browser{r.harness} | |||
| 438 | + | token := r.signIn(t) | |||
| 439 | + | // old is an orphan branch: main can be proposed to it, and main stays an | |||
| 440 | + | // ancestor of feat/search so that it fast-forwards. | |||
| 441 | + | r.mustGit(r.wt, "checkout", "-q", "--orphan", "old") | |||
| 442 | + | r.writeFile(r.wt, "old.txt", []byte("old\n")) // commits | |||
| 443 | + | r.mustGit(r.wt, "push", "-q", r.url, "old") | |||
| 444 | + | ||||
| 445 | + | for _, mr := range []url.Values{ | |||
| 446 | + | {"source": {"feat/search"}, "target": {"main"}, "title": {"Search"}}, | |||
| 447 | + | {"source": {"main"}, "target": {"old"}, "title": {"Back"}}, | |||
| 448 | + | } { | |||
| 449 | + | require.Equal(t, http.StatusSeeOther, b.form("/proj/merge-requests", token, mr).status) | |||
| 450 | + | } | |||
| 451 | + | assert.NotContains(t, b.get("/proj/merge-requests/2", as(token)).body, `name="delete_branch"`, "never for the default branch") | |||
| 452 | + | ||||
| 453 | + | one := b.get("/proj/merge-requests/1", as(token)) | |||
| 454 | + | res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}}) | |||
| 455 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 456 | + | assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search")) | |||
| 457 | + | done := b.get("/proj/merge-requests/1", nil) | |||
| 458 | + | assert.Equal(t, 200, done.status, "the merged page reads without its branch") | |||
| 459 | + | assert.NotContains(t, done.body, `href="/proj/tree/feat/search"`, "no link to a gone branch") | |||
| 460 | + | } | |||
| 461 | + | ||||
| 433 | // The comment limit of the API is a message, not a crash. | 462 | // The comment limit of the API is a message, not a crash. | ||
| 434 | func TestWebMergeRequestLimitIsFriendly(t *testing.T) { | 463 | func TestWebMergeRequestLimitIsFriendly(t *testing.T) { | ||
| 435 | r := newMRRepo(t, domain.VisibilityPublic) | 464 | r := newMRRepo(t, domain.VisibilityPublic) | ||
M internal/domain/token.go +1 -1
| @@ -27,7 +27,7 @@ const ( | |||
| 27 | 27 | ScopeRepoAdmin Scope = "repo:admin" | |
| 28 | 28 | // ScopeMRWrite creates, comments on and closes merge requests. | |
| 29 | 29 | ScopeMRWrite Scope = "mr:write" | |
| 30 | - | // ScopeMRMerge merges. | |
| 30 | + | // ScopeMRMerge merges, and deletes the merged source branch on request. | |
| 31 | 31 | ScopeMRMerge Scope = "mr:merge" | |
| 32 | 32 | // ScopeCIWrite re-runs or cancels CI runs. | |
| 33 | 33 | ScopeCIWrite Scope = "ci:write" | |
| @@ -27,7 +27,7 @@ const ( | |||||
| 27 | ScopeRepoAdmin Scope = "repo:admin" | 27 | ScopeRepoAdmin Scope = "repo:admin" | ||
| 28 | // ScopeMRWrite creates, comments on and closes merge requests. | 28 | // ScopeMRWrite creates, comments on and closes merge requests. | ||
| 29 | ScopeMRWrite Scope = "mr:write" | 29 | ScopeMRWrite Scope = "mr:write" | ||
| 30 | - | // ScopeMRMerge merges. | 30 | + | // ScopeMRMerge merges, and deletes the merged source branch on request. |
| 31 | ScopeMRMerge Scope = "mr:merge" | 31 | ScopeMRMerge Scope = "mr:merge" | ||
| 32 | // ScopeCIWrite re-runs or cancels CI runs. | 32 | // ScopeCIWrite re-runs or cancels CI runs. | ||
| 33 | ScopeCIWrite Scope = "ci:write" | 33 | ScopeCIWrite Scope = "ci:write" | ||
M skills/ermite/CLI.md +1 -1
| @@ -40,7 +40,7 @@ Scopes: | |||
| 40 | 40 | | `repo:write` | push, create a repository | | |
| 41 | 41 | | `repo:admin` | delete a repository, change visibility, description, default branch, merge settings | | |
| 42 | 42 | | `mr:write` | open, comment on, resolve, close, mark read | | |
| 43 | - | | `mr:merge` | merge | | |
| 43 | + | | `mr:merge` | merge, and delete the merged source branch with `--delete-branch` | | |
| 44 | 44 | | `token:admin` | create, list and revoke tokens | | |
| 45 | 45 | | `ci:write`, `secrets:write`, `runner:register` | reserved for CI | | |
| 46 | 46 | ||
| @@ -40,7 +40,7 @@ Scopes: | |||||
| 40 | | `repo:write` | push, create a repository | | 40 | | `repo:write` | push, create a repository | | ||
| 41 | | `repo:admin` | delete a repository, change visibility, description, default branch, merge settings | | 41 | | `repo:admin` | delete a repository, change visibility, description, default branch, merge settings | | ||
| 42 | | `mr:write` | open, comment on, resolve, close, mark read | | 42 | | `mr:write` | open, comment on, resolve, close, mark read | | ||
| 43 | - | | `mr:merge` | merge | | 43 | + | | `mr:merge` | merge, and delete the merged source branch with `--delete-branch` | |
| 44 | | `token:admin` | create, list and revoke tokens | | 44 | | `token:admin` | create, list and revoke tokens | | ||
| 45 | | `ci:write`, `secrets:write`, `runner:register` | reserved for CI | | 45 | | `ci:write`, `secrets:write`, `runner:register` | reserved for CI | | ||
| 46 | 46 | ||||