From 461ccda5022dfb185ead9fe445137f4fe6c9da84 Mon Sep 17 00:00:00 2001 From: Oliver Braun Date: Thu, 23 Apr 2026 21:05:57 +0200 Subject: [PATCH] feat: add merge request checks and update configuration options Co-authored-by: Copilot --- config/assignment.go | 9 +++++- config/assignment_test.go | 44 ++++++++++++++++++++++++++++ config/show.go | 24 ++++++++++++++-- config/types.go | 8 ++++-- config/urls_show_test.go | 26 +++++++++++++++++ docs/configuration.md | 12 ++++++++ docs/getting-started.md | 4 +++ gitlab/projects.go | 35 +++++++++++++++-------- gitlab/projects_contract_test.go | 49 ++++++++++++++++++++++++++++++++ 9 files changed, 194 insertions(+), 17 deletions(-) diff --git a/config/assignment.go b/config/assignment.go index 225a0c8..fbb7069 100644 --- a/config/assignment.go +++ b/config/assignment.go @@ -159,5 +159,12 @@ func mergeRequest(assignmentKey string) *MergeRequest { squashOption = SquashDefaultOff } - return &MergeRequest{MergeMethod: mergeMethod, SquashOption: squashOption} + return &MergeRequest{ + MergeMethod: mergeMethod, + SquashOption: squashOption, + PipelineMustSucceed: viper.GetBool(assignmentKey + ".mergeRequest.pipeline"), + SkippedPipelinesAreSuccessful: viper.GetBool(assignmentKey + ".mergeRequest.skippedPipelinesAreSuccessful"), + AllThreadsMustBeResolved: viper.GetBool(assignmentKey + ".mergeRequest.allThreadsMustBeResolved"), + StatusChecksMustSucceed: viper.GetBool(assignmentKey + ".mergeRequest.statusChecksMustSucceed"), + } } diff --git a/config/assignment_test.go b/config/assignment_test.go index c7562ed..e6d0c6f 100644 --- a/config/assignment_test.go +++ b/config/assignment_test.go @@ -150,6 +150,18 @@ func TestGetAssignmentConfig_DefaultMergeRequest(t *testing.T) { if cfg.MergeRequest.SquashOption != SquashDefaultOff { t.Fatalf("MergeRequest.SquashOption = %q, want %q", cfg.MergeRequest.SquashOption, SquashDefaultOff) } + if cfg.MergeRequest.PipelineMustSucceed { + t.Fatal("MergeRequest.PipelineMustSucceed = true, want false") + } + if cfg.MergeRequest.SkippedPipelinesAreSuccessful { + t.Fatal("MergeRequest.SkippedPipelinesAreSuccessful = true, want false") + } + if cfg.MergeRequest.AllThreadsMustBeResolved { + t.Fatal("MergeRequest.AllThreadsMustBeResolved = true, want false") + } + if cfg.MergeRequest.StatusChecksMustSucceed { + t.Fatal("MergeRequest.StatusChecksMustSucceed = true, want false") + } } func TestGetAssignmentConfig_SquashOption(t *testing.T) { @@ -183,3 +195,35 @@ func TestGetAssignmentConfig_SquashOption(t *testing.T) { }) } } + +func TestGetAssignmentConfig_MergeRequestChecks(t *testing.T) { + resetViper(t) + + viper.Set("gitlab.host", "https://gitlab.example.org") + viper.Set("course", true) + viper.Set("course.coursepath", "mpd") + viper.Set("course.a1", true) + viper.Set("course.a1.assignmentpath", "blatt-01") + viper.Set("course.a1.mergeRequest.pipeline", true) + viper.Set("course.a1.mergeRequest.skippedPipelinesAreSuccessful", true) + viper.Set("course.a1.mergeRequest.allThreadsMustBeResolved", true) + viper.Set("course.a1.mergeRequest.statusChecksMustSucceed", true) + + cfg := GetAssignmentConfig("course", "a1") + + if cfg.MergeRequest == nil { + t.Fatal("MergeRequest should not be nil") + } + if !cfg.MergeRequest.PipelineMustSucceed { + t.Fatal("PipelineMustSucceed = false, want true") + } + if !cfg.MergeRequest.SkippedPipelinesAreSuccessful { + t.Fatal("SkippedPipelinesAreSuccessful = false, want true") + } + if !cfg.MergeRequest.AllThreadsMustBeResolved { + t.Fatal("AllThreadsMustBeResolved = false, want true") + } + if !cfg.MergeRequest.StatusChecksMustSucceed { + t.Fatal("StatusChecksMustSucceed = false, want true") + } +} diff --git a/config/show.go b/config/show.go index f90d4e6..26be505 100644 --- a/config/show.go +++ b/config/show.go @@ -15,17 +15,37 @@ func (cfg *AssignmentConfig) Show() { mergeMethod := MergeCommit squashOption := SquashDefaultOff + pipelineMustSucceed := false + skippedPipelinesAreSuccessful := false + allThreadsMustBeResolved := false + statusChecksMustSucceed := false if cfg.MergeRequest != nil { mergeMethod = cfg.MergeRequest.MergeMethod squashOption = cfg.MergeRequest.SquashOption + pipelineMustSucceed = cfg.MergeRequest.PipelineMustSucceed + skippedPipelinesAreSuccessful = cfg.MergeRequest.SkippedPipelinesAreSuccessful + allThreadsMustBeResolved = cfg.MergeRequest.AllThreadsMustBeResolved + statusChecksMustSucceed = cfg.MergeRequest.StatusChecksMustSucceed } mergeRequestCfg := aurora.Sprintf(aurora.Cyan(` - %s %s - %s %s`), + %s %s + %s %s + %s %t + %s %t + %s %t + %s %t`), aurora.Cyan("MergeMethod:"), aurora.Yellow(mergeMethod), aurora.Cyan("SquashOption:"), aurora.Yellow(squashOption), + aurora.Cyan("PipelineMustSucceed:"), + aurora.Yellow(pipelineMustSucceed), + aurora.Cyan("SkippedPipelinesAreSuccessful:"), + aurora.Yellow(skippedPipelinesAreSuccessful), + aurora.Cyan("AllThreadsMustBeResolved:"), + aurora.Yellow(allThreadsMustBeResolved), + aurora.Cyan("StatusChecksMustSucceed:"), + aurora.Yellow(statusChecksMustSucceed), ) startercode := aurora.Sprintf(aurora.Red("not defined")) diff --git a/config/types.go b/config/types.go index c72c834..93029c1 100644 --- a/config/types.go +++ b/config/types.go @@ -70,8 +70,12 @@ type Release struct { } type MergeRequest struct { - MergeMethod MergeMethod - SquashOption SquashOption + MergeMethod MergeMethod + SquashOption SquashOption + PipelineMustSucceed bool + SkippedPipelinesAreSuccessful bool + AllThreadsMustBeResolved bool + StatusChecksMustSucceed bool } type ReleaseMergeRequest struct { diff --git a/config/urls_show_test.go b/config/urls_show_test.go index cd27e78..cffe120 100644 --- a/config/urls_show_test.go +++ b/config/urls_show_test.go @@ -254,3 +254,29 @@ func TestShow_WithSquashOption(t *testing.T) { t.Fatalf("Show() output does not contain squash option 'always': %q", out) } } + +func TestShow_WithMergeChecks(t *testing.T) { + cfg := &AssignmentConfig{ + MergeRequest: &MergeRequest{ + MergeMethod: MergeCommit, + SquashOption: SquashDefaultOff, + PipelineMustSucceed: true, + SkippedPipelinesAreSuccessful: true, + AllThreadsMustBeResolved: true, + StatusChecksMustSucceed: true, + }, + } + out := captureStdout(t, func() { cfg.Show() }) + if !strings.Contains(out, "PipelineMustSucceed:") || !strings.Contains(out, "true") { + t.Fatalf("Show() output does not contain PipelineMustSucceed=true: %q", out) + } + if !strings.Contains(out, "AllThreadsMustBeResolved:") { + t.Fatalf("Show() output does not contain AllThreadsMustBeResolved key: %q", out) + } + if !strings.Contains(out, "SkippedPipelinesAreSuccessful:") { + t.Fatalf("Show() output does not contain SkippedPipelinesAreSuccessful key: %q", out) + } + if !strings.Contains(out, "StatusChecksMustSucceed:") { + t.Fatalf("Show() output does not contain StatusChecksMustSucceed key: %q", out) + } +} diff --git a/docs/configuration.md b/docs/configuration.md index e196df2..a457a2f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -324,6 +324,10 @@ Control how merge requests are merged in every generated project: mergeRequest: mergeMethod: merge # merge | semi_linear | ff squashOption: default_off # never | always | default_off | default_on + pipeline: false # Require successful pipeline before merge + skippedPipelinesAreSuccessful: false + allThreadsMustBeResolved: false + statusChecksMustSucceed: false ``` ### Merge Request keys @@ -332,6 +336,10 @@ Control how merge requests are merged in every generated project: |---|---|---|---| | `mergeRequest.mergeMethod` | Merge strategy for all MRs | `merge` | Applied at project creation | | `mergeRequest.squashOption` | Squash-on-merge setting | `default_off` | Applied at project creation | +| `mergeRequest.pipeline` | Pipelines must succeed | `false` | Maps to GitLab merge check | +| `mergeRequest.skippedPipelinesAreSuccessful` | Treat skipped pipelines as successful | `false` | Only relevant when `mergeRequest.pipeline=true` | +| `mergeRequest.allThreadsMustBeResolved` | All threads must be resolved | `false` | Maps to GitLab merge check | +| `mergeRequest.statusChecksMustSucceed` | Status checks must succeed | `false` | Maps to GitLab merge check | ### Merge method values @@ -357,6 +365,10 @@ blatt01: mergeRequest: mergeMethod: ff # Students must keep a linear history squashOption: always # All commits squashed into one on merge + pipeline: true # Block merge until latest pipeline succeeds + skippedPipelinesAreSuccessful: false + allThreadsMustBeResolved: true + statusChecksMustSucceed: true ``` ## Full example diff --git a/docs/getting-started.md b/docs/getting-started.md index 1ddb752..e05bc7f 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -79,6 +79,10 @@ mpd: | Assignment | `description` | `generated by glabs` | | Assignment | `accesslevel` | `developer` | | Assignment | `containerRegistry` | `false` | +| Assignment | `mergeRequest.pipeline` | `false` | +| Assignment | `mergeRequest.skippedPipelinesAreSuccessful` | `false` | +| Assignment | `mergeRequest.allThreadsMustBeResolved` | `false` | +| Assignment | `mergeRequest.statusChecksMustSucceed` | `false` | | Startercode | `fromBranch` | `main` | | Startercode | `toBranch` | `main` | | Startercode | `devBranch` | value of `toBranch` | diff --git a/gitlab/projects.go b/gitlab/projects.go index a086340..4c67721 100644 --- a/gitlab/projects.go +++ b/gitlab/projects.go @@ -16,9 +16,17 @@ func (c *Client) generateProject(assignmentCfg *config.AssignmentConfig, name st // Keep this fallback for safety when AssignmentConfig is constructed manually. mergeMethod := config.MergeCommit squashOption := config.SquashDefaultOff + pipelineMustSucceed := false + skippedPipelinesAreSuccessful := false + allThreadsMustBeResolved := false + statusChecksMustSucceed := false if assignmentCfg.MergeRequest != nil { mergeMethod = assignmentCfg.MergeRequest.MergeMethod squashOption = assignmentCfg.MergeRequest.SquashOption + pipelineMustSucceed = assignmentCfg.MergeRequest.PipelineMustSucceed + skippedPipelinesAreSuccessful = assignmentCfg.MergeRequest.SkippedPipelinesAreSuccessful + allThreadsMustBeResolved = assignmentCfg.MergeRequest.AllThreadsMustBeResolved + statusChecksMustSucceed = assignmentCfg.MergeRequest.StatusChecksMustSucceed } // Convert glabs MergeMethod to GitLab API MergeMethodValue @@ -46,18 +54,21 @@ func (c *Client) generateProject(assignmentCfg *config.AssignmentConfig, name st } p := &gitlab.CreateProjectOptions{ - Name: gitlab.Ptr(name), - Description: gitlab.Ptr(assignmentCfg.Description), - NamespaceID: gitlab.Ptr(inID), - MergeRequestsAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), - IssuesAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), - BuildsAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), - JobsEnabled: gitlab.Ptr(true), - Visibility: gitlab.Ptr(gitlab.PrivateVisibility), - ContainerRegistryEnabled: gitlab.Ptr(assignmentCfg.ContainerRegistry), - OnlyAllowMergeIfAllStatusChecksPassed: gitlab.Ptr(false), - MergeMethod: gitlab.Ptr(gitlabMergeMethod), - SquashOption: gitlab.Ptr(gitlabSquashOption), + Name: gitlab.Ptr(name), + Description: gitlab.Ptr(assignmentCfg.Description), + NamespaceID: gitlab.Ptr(inID), + MergeRequestsAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), + IssuesAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), + BuildsAccessLevel: gitlab.Ptr(gitlab.EnabledAccessControl), + JobsEnabled: gitlab.Ptr(true), + Visibility: gitlab.Ptr(gitlab.PrivateVisibility), + ContainerRegistryEnabled: gitlab.Ptr(assignmentCfg.ContainerRegistry), + OnlyAllowMergeIfPipelineSucceeds: gitlab.Ptr(pipelineMustSucceed), + AllowMergeOnSkippedPipeline: gitlab.Ptr(skippedPipelinesAreSuccessful), + OnlyAllowMergeIfAllDiscussionsAreResolved: gitlab.Ptr(allThreadsMustBeResolved), + OnlyAllowMergeIfAllStatusChecksPassed: gitlab.Ptr(statusChecksMustSucceed), + MergeMethod: gitlab.Ptr(gitlabMergeMethod), + SquashOption: gitlab.Ptr(gitlabSquashOption), } project, _, err := c.Projects.CreateProject(p) diff --git a/gitlab/projects_contract_test.go b/gitlab/projects_contract_test.go index cfaa061..884a9c1 100644 --- a/gitlab/projects_contract_test.go +++ b/gitlab/projects_contract_test.go @@ -135,3 +135,52 @@ func TestGenerateProject_FallsBackToExistingProject(t *testing.T) { t.Fatalf("project = %#v", project) } } + +func TestGenerateProject_SetsMergeRequestChecks(t *testing.T) { + var createBody string + + client := newContractClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/api/v4/projects" { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("ReadAll failed: %v", err) + } + createBody = string(body) + _, _ = w.Write([]byte(`{"id":31,"name":"repo-a","path_with_namespace":"mpd/ss26/repo-a"}`)) + return + } + w.WriteHeader(http.StatusNotFound) + }) + + assignmentCfg := &config.AssignmentConfig{ + Description: "desc", + Path: "mpd/ss26", + MergeRequest: &config.MergeRequest{ + PipelineMustSucceed: true, + SkippedPipelinesAreSuccessful: true, + AllThreadsMustBeResolved: true, + StatusChecksMustSucceed: true, + }, + } + + _, _, err := client.generateProject(assignmentCfg, "repo-a", 123) + if err != nil { + t.Fatalf("generateProject() returned error: %v", err) + } + + checks := []string{ + `"only_allow_merge_if_pipeline_succeeds":true`, + `"allow_merge_on_skipped_pipeline":true`, + `"only_allow_merge_if_all_discussions_are_resolved":true`, + `"only_allow_merge_if_all_status_checks_passed":true`, + "only_allow_merge_if_pipeline_succeeds=true", + "allow_merge_on_skipped_pipeline=true", + "only_allow_merge_if_all_discussions_are_resolved=true", + "only_allow_merge_if_all_status_checks_passed=true", + } + for i := 0; i < 4; i++ { + if !strings.Contains(createBody, checks[i]) && !strings.Contains(createBody, checks[i+4]) { + t.Fatalf("create project request body missing merge check field %q: %q", checks[i], createBody) + } + } +}