From f4ce26c27f0c11adde18cbfda518918ca7aa0957 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 27 Mar 2025 14:06:21 +1100 Subject: [PATCH 1/6] ci: Include JUint XML report in job artifacts We currently ask `gotestsum` to generate JUint XML reports, but we don't save them as job artifacts. The reports are primarily used to populate the GitLab unit tests UI [1]. Add the JUnit XML file to the list of job artifacts. These reports are easier to parse than the JSON report, and will be required for the flaky test quarantine automation. [1] https://docs.gitlab.com/ci/testing/unit_test_reports/ --- .gitlab-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 56ff05ff4c..070f67d109 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -165,6 +165,7 @@ workflow: paths: - ${TEST_LOG_DIR} - ${TEST_JSON_REPORT} + - ${TEST_JUNIT_REPORT} - ${TEST_COVERAGE_REPORT} - ${TEST_COVERAGE_LIST} reports: -- GitLab From 8783dbda2a21390fc2ed395eeb896224e604b125 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 27 Mar 2025 14:46:20 +1100 Subject: [PATCH 2/6] ci: Reorder linter and unit tests The linter and unit tests both execute the same run_go_tests target, but in different working directories. The TEST_JUNIT_REPORT and TEST_JSON_REPORT variables are thus shared between runs, causing the results of the linter tests to overwrite the results of the unit tests. Reorder the tests so that linter tests run first; it isn't really that important to produce a test artifact for these. Add prepare-tests as a dependency to the test-gitaly-linters target, as it sets up the coverage output directory. # Conflicts: # .gitlab-ci.yml --- .gitlab-ci.yml | 10 +++++----- Makefile | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 070f67d109..a582210e45 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -134,6 +134,11 @@ workflow: - mount none -t tmpfs -o size=10G "${TEST_TMP_DIR}" - go version script: + # Run tool tests under a privileged user. These tests sometimes trigger + # a Go toolchain to be downloaded, and that seems to cause permissions issues + # when it's executed under `setpriv` below. + - make test-gitaly-linters + - make test-mod-validator # Create the build directory for the unprivileged user that we're running # tests as. This is required because the source directory itself is owned # by `root`. @@ -144,11 +149,6 @@ workflow: # proper attention to permission bits and that we don't modify the source # directory. - setpriv --reuid=${TEST_UID} --regid=${TEST_UID} --clear-groups --no-new-privs env HOME=/tmp/home make ${TEST_TARGET} - # Run tool tests under a privileged user. These tests sometimes trigger - # a Go toolchain to be downloaded, and that seems to cause permissions issues - # when it's executed under `setpriv` below. - - make test-gitaly-linters - - make test-mod-validator coverage: /^total:\t+\(statements\)\t+\d+\.\d+%$/ after_script: &test_after_script - go run "${CI_PROJECT_DIR}/tools/panic-parser" diff --git a/Makefile b/Makefile index 8a8d252644..0d59a9c547 100644 --- a/Makefile +++ b/Makefile @@ -410,6 +410,7 @@ test: test-go .PHONY: test-gitaly-linters ## Test Go tests in tools/golangci-lint/gitaly folder ## That folder has its own go.mod file. Hence tests must run the context of that folder. +test-gitaly-linters: prepare-tests test-gitaly-linters: TEST_PACKAGES := . test-gitaly-linters: ${GOTESTSUM} ${Q}cd ${SOURCE_DIR}/tools/golangci-lint/gitaly && $(call run_go_tests) -- GitLab From 14b83aa5214c193500ec3a148255d4683a5030e7 Mon Sep 17 00:00:00 2001 From: James Liu Date: Fri, 28 Mar 2025 15:01:44 +1100 Subject: [PATCH 3/6] test: Create a flaky test detector Gitaly tests can often be flaky due to race conditions or other concurrency-related issues. These are hard to catch during development time, and can become very disruptive if left untouched. We recently introduced [1] the use of the --rerun-fails flag to our `gotestsum` invocation, which allows the tool to re-run individual tests up to three times (two retries). This reduces the number of times our pipelines are blocked due to test flake, since flaky tests are swept under the rug. This however doesn't resolve the issue of making flaky tests more visible to team members. Create the beginnings of a flake detection tool which can parse JUnit XML reports produced by `gotestsum`/`go test` within CI jobs. The idea is that if the pipeline is green, but the report contains failed tests, we have flaky tests on our hands. The JUnit file's hierarchical structure is easier to parse than the equivalent JSON output. The tool will eventually be executed in the `after_script` of the test jobs, and will be responsible for parsing the test report from the specific job it was executed in. The detector will only be invoked if the job has passed. Add a new package in the tools/ directory called `flake-detector`, and wire it up to a new Makefile target. The tool currently ingests a JUnit file, finds flakes, filters out duplicates (two failures are recorded for every failed table test; one for the specific test case, and one for the outer function), and logs them. Subsequent commits will introduce functionality to open "Flaky Test" issues automatically. [1] https://gitlab.com/gitlab-org/gitaly/-/merge_requests/7673/commits --- Makefile | 5 + tools/flake-detector/go.mod | 14 +++ tools/flake-detector/go.sum | 13 +++ tools/flake-detector/main.go | 98 +++++++++++++++++++ tools/flake-detector/main_test.go | 93 ++++++++++++++++++ .../testdata/flakes-table-reordered.junit | 16 +++ .../testdata/flakes-table.junit | 16 +++ .../testdata/flakes-toplevel.junit | 12 +++ tools/flake-detector/testdata/flakes.junit | 20 ++++ tools/flake-detector/testdata/main_test.go | 53 ++++++++++ tools/flake-detector/testdata/pass.junit | 11 +++ 11 files changed, 351 insertions(+) create mode 100644 tools/flake-detector/go.mod create mode 100644 tools/flake-detector/go.sum create mode 100644 tools/flake-detector/main.go create mode 100644 tools/flake-detector/main_test.go create mode 100644 tools/flake-detector/testdata/flakes-table-reordered.junit create mode 100644 tools/flake-detector/testdata/flakes-table.junit create mode 100644 tools/flake-detector/testdata/flakes-toplevel.junit create mode 100644 tools/flake-detector/testdata/flakes.junit create mode 100644 tools/flake-detector/testdata/main_test.go create mode 100644 tools/flake-detector/testdata/pass.junit diff --git a/Makefile b/Makefile index 0d59a9c547..971423d0c7 100644 --- a/Makefile +++ b/Makefile @@ -610,6 +610,11 @@ build-protoset: proto govulncheck: ${GOVULNCHECK} ${Q}(${GOVULNCHECK} ./... || true) | go run ${SOURCE_DIR}/tools/govulncheck-filter/main.go +.PHONY: flake-detector +flake-detector: + ${Q}cd ${SOURCE_DIR}/tools/flake-detector && \ + go run ${SOURCE_DIR}/tools/flake-detector/main.go ${TEST_JUNIT_REPORT} ${CI_JOB_NAME} + .PHONY: no-changes no-changes: ${Q}${GIT} diff --exit-code diff --git a/tools/flake-detector/go.mod b/tools/flake-detector/go.mod new file mode 100644 index 0000000000..4cd6d50625 --- /dev/null +++ b/tools/flake-detector/go.mod @@ -0,0 +1,14 @@ +module gitlab.com/gitlab-org/gitaly/tools/flake-detector + +go 1.23.6 + +require ( + github.com/joshdk/go-junit v1.0.0 + github.com/stretchr/testify v1.4.0 +) + +require ( + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v2 v2.2.2 // indirect +) diff --git a/tools/flake-detector/go.sum b/tools/flake-detector/go.sum new file mode 100644 index 0000000000..d720c6fe9e --- /dev/null +++ b/tools/flake-detector/go.sum @@ -0,0 +1,13 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/joshdk/go-junit v1.0.0 h1:S86cUKIdwBHWwA6xCmFlf3RTLfVXYQfvanM5Uh+K6GE= +github.com/joshdk/go-junit v1.0.0/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/tools/flake-detector/main.go b/tools/flake-detector/main.go new file mode 100644 index 0000000000..8331c7c3f3 --- /dev/null +++ b/tools/flake-detector/main.go @@ -0,0 +1,98 @@ +package main + +import ( + "fmt" + "github.com/joshdk/go-junit" + "log" + "os" + "strings" +) + +type FlakyTest struct { + GoVersion string + Package string + TestName string + Log string +} + +func Detect(filename string) ([]FlakyTest, error) { + suites, err := junit.IngestFile(filename) + if err != nil { + return nil, fmt.Errorf("ingest test report: %w", err) + } + + testsByBaseName := make(map[string][]int) + + var flakes []FlakyTest + for _, suite := range suites { + if suite.Totals.Failed == 0 { + continue + } + + for _, test := range suite.Tests { + if test.Status != junit.StatusFailed { + continue + } + + // We track the base name of each test so failures in table tests can be + // deduplicated later on. + // The test output records two failures for each table test failure; one + // for the specific test case, and another for the function. For example, + // if `TestFlakyTableTest/test-one` failed, the report will contain two + // failures: + // + // - TestFlakyTableTest/test-one + // - TestFlakyTableTest + // + // We want to keep only the more specific failure. + baseName := strings.Split(test.Name, "/")[0] + idx := len(flakes) + testsByBaseName[baseName] = append(testsByBaseName[baseName], idx) + + flakes = append(flakes, FlakyTest{ + GoVersion: suite.Properties["go.version"], + Package: suite.Name, + TestName: test.Name, + Log: test.Error.Error(), + }) + } + } + + var filteredFlakes []FlakyTest + for _, idxs := range testsByBaseName { + if len(idxs) == 1 { + // Not a table test + filteredFlakes = append(filteredFlakes, flakes[idxs[0]]) + continue + } + + for _, idx := range idxs { + // Keep only the tests with a name containing a slash, since + // that represents the more specific failure. + if strings.Contains(flakes[idx].TestName, "/") { + filteredFlakes = append(filteredFlakes, flakes[idx]) + } + } + } + + return filteredFlakes, nil +} + +func main() { + if len(os.Args) < 3 { + log.Fatal("Usage: flake-detector ") + } + + ciJobName := os.Args[2] + log.Printf("Processing CI job %q\n", ciJobName) + + junitReport := os.Args[1] + log.Printf("Reading report %q\n", junitReport) + + flakyTests, err := Detect(junitReport) + if err != nil { + log.Fatal(err) + } + + fmt.Println(flakyTests) +} diff --git a/tools/flake-detector/main_test.go b/tools/flake-detector/main_test.go new file mode 100644 index 0000000000..f2a742df58 --- /dev/null +++ b/tools/flake-detector/main_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "github.com/stretchr/testify/require" + "os" + "testing" +) + +func TestDetect(t *testing.T) { + tests := []struct { + name string + inputFile string + jobName string + expectedFlakes []FlakyTest + }{ + { + name: "when top-level and table test flakes are found", + inputFile: "testdata/flakes.junit", + jobName: "test CI job", + expectedFlakes: []FlakyTest{ + { + GoVersion: "go1.23.6 darwin/arm64", + Package: "gitlab.com/gitlab-org/gitaly/tools/flake-detector/testdata", + TestName: "TestFlakyTopLevel", + Log: "=== RUN TestFlakyTopLevel\n--- FAIL: TestFlakyTopLevel (0.00s)\n", + }, + { + GoVersion: "go1.23.6 darwin/arm64", + Package: "gitlab.com/gitlab-org/gitaly/tools/flake-detector/testdata", + TestName: "TestFlakyTableTest/test-one", + Log: "=== RUN TestFlakyTableTest/test-one\n--- FAIL: TestFlakyTableTest/test-one (0.00s)\n", + }, + }, + }, + { + name: "when top-level flakes are found", + inputFile: "testdata/flakes-toplevel.junit", + jobName: "test CI job", + expectedFlakes: []FlakyTest{ + { + GoVersion: "go1.23.6 darwin/arm64", + Package: "gitlab.com/gitlab-org/gitaly/tools/flake-detector/testdata", + TestName: "TestFlakyTopLevel", + Log: "=== RUN TestFlakyTopLevel\n--- FAIL: TestFlakyTopLevel (0.00s)\n", + }, + }, + }, + { + name: "when table-test flakes are found", + inputFile: "testdata/flakes-table.junit", + jobName: "test CI job", + expectedFlakes: []FlakyTest{ + { + GoVersion: "go1.23.6 darwin/arm64", + Package: "gitlab.com/gitlab-org/gitaly/tools/flake-detector/testdata", + TestName: "TestFlakyTableTest/test-one", + Log: "=== RUN TestFlakyTableTest/test-one\n--- FAIL: TestFlakyTableTest/test-one (0.00s)\n", + }, + }, + }, + { + name: "when table-test flakes are found (reordered)", + inputFile: "testdata/flakes-table-reordered.junit", + jobName: "test CI job", + expectedFlakes: []FlakyTest{ + { + GoVersion: "go1.23.6 darwin/arm64", + Package: "gitlab.com/gitlab-org/gitaly/tools/flake-detector/testdata", + TestName: "TestFlakyTableTest/test-one", + Log: "=== RUN TestFlakyTableTest/test-one\n--- FAIL: TestFlakyTableTest/test-one (0.00s)\n", + }, + }, + }, + { + name: "when no flakes are found", + inputFile: "testdata/pass.junit", + jobName: "test CI job", + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + _ = os.Setenv("CI_JOB_NAME", tc.jobName) + defer os.Unsetenv("CI_JOB_NAME") + + actualFlakes, err := Detect(tc.inputFile) + require.NoError(t, err) + + require.ElementsMatch(t, tc.expectedFlakes, actualFlakes) + }) + } +} diff --git a/tools/flake-detector/testdata/flakes-table-reordered.junit b/tools/flake-detector/testdata/flakes-table-reordered.junit new file mode 100644 index 0000000000..848c56ffc4 --- /dev/null +++ b/tools/flake-detector/testdata/flakes-table-reordered.junit @@ -0,0 +1,16 @@ + + + + + + + + === RUN TestFlakyTableTest --- FAIL: TestFlakyTableTest (0.00s) + + + === RUN TestFlakyTableTest/test-one --- FAIL: TestFlakyTableTest/test-one (0.00s) + + + + + diff --git a/tools/flake-detector/testdata/flakes-table.junit b/tools/flake-detector/testdata/flakes-table.junit new file mode 100644 index 0000000000..c975d4130c --- /dev/null +++ b/tools/flake-detector/testdata/flakes-table.junit @@ -0,0 +1,16 @@ + + + + + + + + === RUN TestFlakyTableTest/test-one --- FAIL: TestFlakyTableTest/test-one (0.00s) + + + === RUN TestFlakyTableTest --- FAIL: TestFlakyTableTest (0.00s) + + + + + \ No newline at end of file diff --git a/tools/flake-detector/testdata/flakes-toplevel.junit b/tools/flake-detector/testdata/flakes-toplevel.junit new file mode 100644 index 0000000000..db6f47034b --- /dev/null +++ b/tools/flake-detector/testdata/flakes-toplevel.junit @@ -0,0 +1,12 @@ + + + + + + + + === RUN TestFlakyTopLevel --- FAIL: TestFlakyTopLevel (0.00s) + + + + \ No newline at end of file diff --git a/tools/flake-detector/testdata/flakes.junit b/tools/flake-detector/testdata/flakes.junit new file mode 100644 index 0000000000..eb493fcfdd --- /dev/null +++ b/tools/flake-detector/testdata/flakes.junit @@ -0,0 +1,20 @@ + + + + + + + + === RUN TestFlakyTopLevel --- FAIL: TestFlakyTopLevel (0.00s) + + + === RUN TestFlakyTableTest/test-one --- FAIL: TestFlakyTableTest/test-one (0.00s) + + + === RUN TestFlakyTableTest --- FAIL: TestFlakyTableTest (0.00s) + + + + + + \ No newline at end of file diff --git a/tools/flake-detector/testdata/main_test.go b/tools/flake-detector/testdata/main_test.go new file mode 100644 index 0000000000..c75c32b76e --- /dev/null +++ b/tools/flake-detector/testdata/main_test.go @@ -0,0 +1,53 @@ +// Package main is used to generate the JUnit test data files in this directory, via +// gotestsum --rerun-fails --packages ./... --junitfile foo.junit ./... +package main + +import ( + "errors" + "os" + "testing" +) + +func TestFlakyTopLevel(t *testing.T) { + sentinel := "/tmp/flaky-top-level-sentinel" + + if _, err := os.Stat(sentinel); errors.Is(err, os.ErrNotExist) { + // Create and flake + if _, err := os.Create(sentinel); err != nil { + t.Fatal(err) + } + t.Fail() + } else if err == nil { + // Delete the file. Don't flake. + if err := os.Remove(sentinel); err != nil { + t.Fatal(err) + } + } +} + +func TestFlakyTableTest(t *testing.T) { + for _, tc := range []struct { + name string + }{ + { + name: "test-one", + }, + } { + t.Run(tc.name, func(t *testing.T) { + sentinel := "/tmp/flaky-table-test-sentinel" + + if _, err := os.Stat(sentinel); errors.Is(err, os.ErrNotExist) { + // Create and flake + if _, err := os.Create(sentinel); err != nil { + t.Fatal(err) + } + t.Fail() + } else if err == nil { + // Delete the file. Don't flake. + if err := os.Remove(sentinel); err != nil { + t.Fatal(err) + } + } + }) + } +} diff --git a/tools/flake-detector/testdata/pass.junit b/tools/flake-detector/testdata/pass.junit new file mode 100644 index 0000000000..d09328a77b --- /dev/null +++ b/tools/flake-detector/testdata/pass.junit @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file -- GitLab From cff346aa5aa9346f2c6700918cec9602fa3b92f5 Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 31 Mar 2025 18:27:26 +1100 Subject: [PATCH 4/6] test: Add a Reporter type to query Issues API The GitLab Issues API [1] can be used to retrieve and create issues programmatically. The Flake Detector will need the ability to generate issues automatically for each flaky test it discovers, and so will need access to this API. We provide authentication via a new environment variable called `GITALY_TOOLS_FLAKE_DETECTOR_PAT`. We can discuss how we will generate and maintain this PAT later on. The Reporter struct will expose two primarily functions; Get() and Create(), which will allow the tool to determine if a flaky test warrants a new issue to be created, and to actually create the issue. Implement the Reporter struct and the Get() function, which queries the "List project issues" endpoint. We assume that issues generated by the tool will follow a similar naming scheme to the current [2] flaky test issues. Additionally, we set `per_page` to 1, since we're only interested in whether an exact match is found. The corresponding unit test creates a local HTTP server that ensures the client is passing the required headers and query parameters. The server gets invoked by swapping out the base URL of the API. [1] https://docs.gitlab.com/api/issues/ [2] https://gitlab.com/gitlab-org/gitaly/-/issues/?sort=due_date&state=opened&search=%5BFlaky+Test%5D&first_page_size=20 --- tools/flake-detector/issues.go | 89 ++++++++++++++ tools/flake-detector/main_test.go | 37 ++++++ .../testdata/issues_response.json | 112 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/flake-detector/issues.go create mode 100644 tools/flake-detector/testdata/issues_response.json diff --git a/tools/flake-detector/issues.go b/tools/flake-detector/issues.go new file mode 100644 index 0000000000..e93a248826 --- /dev/null +++ b/tools/flake-detector/issues.go @@ -0,0 +1,89 @@ +package main + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os" +) + +type Issue struct { + IID int `json:"iid"` + URL string `json:"web_url"` +} + +const ( + patEnvVar = "GITALY_TOOLS_FLAKE_DETECTOR_PAT" + gitalyRepoPath = "gitlab-org/gitaly" +) + +type Reporter struct { + baseURL string + pat string + client *http.Client +} + +func NewReporter() (*Reporter, error) { + r := Reporter{ + client: &http.Client{}, + baseURL: "https://gitlab.com", + } + + pat, ok := os.LookupEnv(patEnvVar) + if !ok { + return nil, fmt.Errorf("environment variable %s is not set", patEnvVar) + } + + r.pat = pat + + return &r, nil +} + +func (r *Reporter) WithBaseURL(url string) { + r.baseURL = url +} + +func (r *Reporter) Get(flake FlakyTest) (Issue, bool, error) { + req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/v4/projects/%s/issues", r.baseURL, url.QueryEscape(gitalyRepoPath)), nil) + if err != nil { + return Issue{}, false, fmt.Errorf("form request: %w", err) + } + + q := req.URL.Query() + q.Add("search", fmt.Sprintf("[Flaky Test] %s", flake.TestName)) + q.Add("in", "title") + q.Add("labels", "type::maintenance,maintenance::pipelines,group::gitaly") + q.Add("per_page", "1") + req.URL.RawQuery = q.Encode() + + req.Header.Add("PRIVATE-TOKEN", r.pat) + + resp, err := r.client.Do(req) + if err != nil { + return Issue{}, false, fmt.Errorf("do request: %w", err) + } + + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + if err != nil { + return Issue{}, false, fmt.Errorf("read response body: %w", err) + } + + var issues []Issue + if err := json.Unmarshal(b, &issues); err != nil { + return Issue{}, false, fmt.Errorf("unmarshal response: %w", err) + } + + if len(issues) == 0 { + // Not found + return Issue{}, false, nil + } + + return issues[0], true, nil +} + +func (r *Reporter) Create(flake FlakyTest) error { + return nil +} diff --git a/tools/flake-detector/main_test.go b/tools/flake-detector/main_test.go index f2a742df58..7742b38328 100644 --- a/tools/flake-detector/main_test.go +++ b/tools/flake-detector/main_test.go @@ -2,6 +2,8 @@ package main import ( "github.com/stretchr/testify/require" + "net/http" + "net/http/httptest" "os" "testing" ) @@ -91,3 +93,38 @@ func TestDetect(t *testing.T) { }) } } + +func TestReporter_Get(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + q := req.URL.Query() + + require.Equal(t, "[Flaky Test] TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently", q.Get("search")) + require.Equal(t, "title", q.Get("in")) + require.Equal(t, "type::maintenance,maintenance::pipelines,group::gitaly", q.Get("labels")) + require.Equal(t, "1", q.Get("per_page")) + + require.NotEmpty(t, req.Header.Get("PRIVATE-TOKEN")) + + body, err := os.ReadFile("./testdata/issues_response.json") + require.NoError(t, err) + + res.Write(body) + })) + + require.NoError(t, os.Setenv("GITALY_TOOLS_FLAKE_DETECTOR_PAT", "nope")) + defer os.Unsetenv("GITALY_TOOLS_FLAKE_DETECTOR_PAT") + + r, err := NewReporter() + require.NoError(t, err) + + r.WithBaseURL(ts.URL) + + resp, found, err := r.Get(FlakyTest{TestName: "TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently"}) + require.NoError(t, err) + require.True(t, found) + + require.EqualValues(t, Issue{ + IID: 6689, + URL: "https://gitlab.com/gitlab-org/gitaly/-/issues/6689", + }, resp) +} diff --git a/tools/flake-detector/testdata/issues_response.json b/tools/flake-detector/testdata/issues_response.json new file mode 100644 index 0000000000..d0ead1e5fc --- /dev/null +++ b/tools/flake-detector/testdata/issues_response.json @@ -0,0 +1,112 @@ +[ + { + "id": 164805057, + "iid": 6689, + "project_id": 2009901, + "title": "[Flaky Test] TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently", + "description": "", + "state": "opened", + "created_at": "2025-03-27T09:32:27.376Z", + "updated_at": "2025-03-28T11:25:26.222Z", + "closed_at": null, + "closed_by": null, + "labels": [ + "Category:Gitaly", + "devops::data_access", + "failure::flaky-test", + "group::gitaly", + "maintenance::pipelines", + "priority::1", + "section::infrastructure platforms", + "type::maintenance", + "workflow::ready for development" + ], + "milestone": { + "id": 4599726, + "iid": 110, + "group_id": 9970, + "title": "17.11", + "description": "", + "state": "active", + "created_at": "2024-05-24T19:20:15.085Z", + "updated_at": "2024-05-24T19:20:15.085Z", + "due_date": "2025-04-11", + "start_date": "2025-03-15", + "expired": false, + "web_url": "https://gitlab.com/groups/gitlab-org/-/milestones/110" + }, + "assignees": [ + { + "id": 7815314, + "username": "qmnguyen0711", + "name": "Quang-Minh Nguyen", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/7815314/avatar.png", + "web_url": "https://gitlab.com/qmnguyen0711" + } + ], + "author": { + "id": 18454560, + "username": "echui-gitlab", + "name": "Emily Chui", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/18454560/avatar.png", + "web_url": "https://gitlab.com/echui-gitlab" + }, + "type": "ISSUE", + "assignee": { + "id": 7815314, + "username": "qmnguyen0711", + "name": "Quang-Minh Nguyen", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/7815314/avatar.png", + "web_url": "https://gitlab.com/qmnguyen0711" + }, + "user_notes_count": 1, + "merge_requests_count": 0, + "upvotes": 0, + "downvotes": 0, + "due_date": null, + "confidential": false, + "discussion_locked": null, + "issue_type": "issue", + "web_url": "https://gitlab.com/gitlab-org/gitaly/-/issues/6689", + "time_stats": { + "time_estimate": 0, + "total_time_spent": 0, + "human_time_estimate": null, + "human_total_time_spent": null + }, + "task_completion_status": { + "count": 3, + "completed_count": 1 + }, + "weight": null, + "blocking_issues_count": 0, + "has_tasks": false, + "_links": { + "self": "https://gitlab.com/api/v4/projects/2009901/issues/6689", + "notes": "https://gitlab.com/api/v4/projects/2009901/issues/6689/notes", + "award_emoji": "https://gitlab.com/api/v4/projects/2009901/issues/6689/award_emoji", + "project": "https://gitlab.com/api/v4/projects/2009901", + "closed_as_duplicate_of": null + }, + "references": { + "short": "#6689", + "relative": "#6689", + "full": "gitlab-org/gitaly#6689" + }, + "severity": "UNKNOWN", + "moved_to_id": null, + "imported": false, + "imported_from": "none", + "service_desk_reply_to": null, + "epic_iid": null, + "epic": null, + "iteration": null, + "health_status": null + } +] -- GitLab From ab0142c217844e1b41f8ba7f6db231bf213407e4 Mon Sep 17 00:00:00 2001 From: James Liu Date: Tue, 15 Apr 2025 10:54:56 +1000 Subject: [PATCH 5/6] test: Add a Reporter method to create issues Implements the Create function on the Reporter to create a flaky test issue. The issue follows the template we currently have. --- tools/flake-detector/issues.go | 40 ++++++- tools/flake-detector/main_test.go | 42 +++++++ .../testdata/issue_response.json | 110 ++++++++++++++++++ 3 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 tools/flake-detector/testdata/issue_response.json diff --git a/tools/flake-detector/issues.go b/tools/flake-detector/issues.go index e93a248826..37157add3e 100644 --- a/tools/flake-detector/issues.go +++ b/tools/flake-detector/issues.go @@ -84,6 +84,42 @@ func (r *Reporter) Get(flake FlakyTest) (Issue, bool, error) { return issues[0], true, nil } -func (r *Reporter) Create(flake FlakyTest) error { - return nil +func (r *Reporter) Create(flake FlakyTest) (Issue, error) { + req, err := http.NewRequest("POST", fmt.Sprintf("%s/api/v4/projects/%s/issues", r.baseURL, url.QueryEscape(gitalyRepoPath)), nil) + if err != nil { + return Issue{}, fmt.Errorf("form request: %w", err) + } + + q := req.URL.Query() + q.Add("title", fmt.Sprintf("[Flaky Test] %s", flake.TestName)) + q.Add("labels", "type::maintenance,maintenance::pipelines,group::gitaly,~failure::flaky-test,~priority::1,~workflow::ready for development") + q.Add("description", ` +## Steps + +- [ ] Additional information about the failure has been recorded. +- [ ] This issue has been assigned the current milestone. +- [ ] Flaky test has been quarantined with testhelper.SkipQuarantinedTest +- [ ] Flaky test has been fixed and quarantine removed. +`) + req.URL.RawQuery = q.Encode() + + req.Header.Add("PRIVATE-TOKEN", r.pat) + + resp, err := r.client.Do(req) + if err != nil { + return Issue{}, fmt.Errorf("do request: %w", err) + } + + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + if err != nil { + return Issue{}, fmt.Errorf("read response body: %w", err) + } + + var issue Issue + if err := json.Unmarshal(b, &issue); err != nil { + return Issue{}, fmt.Errorf("unmarshal response: %w", err) + } + + return issue, nil } diff --git a/tools/flake-detector/main_test.go b/tools/flake-detector/main_test.go index 7742b38328..ceb418ce66 100644 --- a/tools/flake-detector/main_test.go +++ b/tools/flake-detector/main_test.go @@ -128,3 +128,45 @@ func TestReporter_Get(t *testing.T) { URL: "https://gitlab.com/gitlab-org/gitaly/-/issues/6689", }, resp) } + +func TestReporter_Create(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodPost, req.Method) + + q := req.URL.Query() + + require.Equal(t, "[Flaky Test] TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently", q.Get("title")) + require.Equal(t, "type::maintenance,maintenance::pipelines,group::gitaly,~failure::flaky-test,~priority::1,~workflow::ready for development", q.Get("labels")) + require.Equal(t, ` +## Steps + +- [ ] Additional information about the failure has been recorded. +- [ ] This issue has been assigned the current milestone. +- [ ] Flaky test has been quarantined with testhelper.SkipQuarantinedTest +- [ ] Flaky test has been fixed and quarantine removed. +`, q.Get("description")) + + require.NotEmpty(t, req.Header.Get("PRIVATE-TOKEN")) + + body, err := os.ReadFile("./testdata/issue_response.json") + require.NoError(t, err) + + res.Write(body) + })) + + require.NoError(t, os.Setenv("GITALY_TOOLS_FLAKE_DETECTOR_PAT", "nope")) + defer os.Unsetenv("GITALY_TOOLS_FLAKE_DETECTOR_PAT") + + r, err := NewReporter() + require.NoError(t, err) + + r.WithBaseURL(ts.URL) + + resp, err := r.Create(FlakyTest{TestName: "TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently"}) + require.NoError(t, err) + + require.EqualValues(t, Issue{ + IID: 6689, + URL: "https://gitlab.com/gitlab-org/gitaly/-/issues/6689", + }, resp) +} diff --git a/tools/flake-detector/testdata/issue_response.json b/tools/flake-detector/testdata/issue_response.json new file mode 100644 index 0000000000..5adddc9a48 --- /dev/null +++ b/tools/flake-detector/testdata/issue_response.json @@ -0,0 +1,110 @@ +{ + "id": 164805057, + "iid": 6689, + "project_id": 2009901, + "title": "[Flaky Test] TestLogManager_PruneLogEntries/trigger_log_entry_pruning_concurrently", + "description": "", + "state": "opened", + "created_at": "2025-03-27T09:32:27.376Z", + "updated_at": "2025-03-28T11:25:26.222Z", + "closed_at": null, + "closed_by": null, + "labels": [ + "Category:Gitaly", + "devops::data_access", + "failure::flaky-test", + "group::gitaly", + "maintenance::pipelines", + "priority::1", + "section::infrastructure platforms", + "type::maintenance", + "workflow::ready for development" + ], + "milestone": { + "id": 4599726, + "iid": 110, + "group_id": 9970, + "title": "17.11", + "description": "", + "state": "active", + "created_at": "2024-05-24T19:20:15.085Z", + "updated_at": "2024-05-24T19:20:15.085Z", + "due_date": "2025-04-11", + "start_date": "2025-03-15", + "expired": false, + "web_url": "https://gitlab.com/groups/gitlab-org/-/milestones/110" + }, + "assignees": [ + { + "id": 7815314, + "username": "qmnguyen0711", + "name": "Quang-Minh Nguyen", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/7815314/avatar.png", + "web_url": "https://gitlab.com/qmnguyen0711" + } + ], + "author": { + "id": 18454560, + "username": "echui-gitlab", + "name": "Emily Chui", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/18454560/avatar.png", + "web_url": "https://gitlab.com/echui-gitlab" + }, + "type": "ISSUE", + "assignee": { + "id": 7815314, + "username": "qmnguyen0711", + "name": "Quang-Minh Nguyen", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/7815314/avatar.png", + "web_url": "https://gitlab.com/qmnguyen0711" + }, + "user_notes_count": 1, + "merge_requests_count": 0, + "upvotes": 0, + "downvotes": 0, + "due_date": null, + "confidential": false, + "discussion_locked": null, + "issue_type": "issue", + "web_url": "https://gitlab.com/gitlab-org/gitaly/-/issues/6689", + "time_stats": { + "time_estimate": 0, + "total_time_spent": 0, + "human_time_estimate": null, + "human_total_time_spent": null + }, + "task_completion_status": { + "count": 3, + "completed_count": 1 + }, + "weight": null, + "blocking_issues_count": 0, + "has_tasks": false, + "_links": { + "self": "https://gitlab.com/api/v4/projects/2009901/issues/6689", + "notes": "https://gitlab.com/api/v4/projects/2009901/issues/6689/notes", + "award_emoji": "https://gitlab.com/api/v4/projects/2009901/issues/6689/award_emoji", + "project": "https://gitlab.com/api/v4/projects/2009901", + "closed_as_duplicate_of": null + }, + "references": { + "short": "#6689", + "relative": "#6689", + "full": "gitlab-org/gitaly#6689" + }, + "severity": "UNKNOWN", + "moved_to_id": null, + "imported": false, + "imported_from": "none", + "service_desk_reply_to": null, + "epic_iid": null, + "epic": null, + "iteration": null, + "health_status": null +} -- GitLab From 536a4d78d25475612f323f376ec4fefdb99229d6 Mon Sep 17 00:00:00 2001 From: James Liu Date: Tue, 15 Apr 2025 14:43:41 +1000 Subject: [PATCH 6/6] wip: flake-detector CI job --- .gitlab-ci.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a582210e45..82f3624694 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,6 @@ stages: + - artifact-test-prepare + - artifact-test - build - test - analyze @@ -450,6 +452,33 @@ verify: - proto/go/gitalypb/* when: on_failure +artifact test: + stage: artifact-test-prepare + needs: [] + parallel: + matrix: + - ARTIFACT_TEST_NUMBER: one + - ARTIFACT_TEST_NUMBER: two + - ARTIFACT_TEST_NUMBER: three + script: + - mkdir output + - echo "This is ARTIFACT_TEST_NUMBER $ARTIFACT_TEST_NUMBER" >> ./output/artifact_test_output.txt + artifacts: + paths: + - output/artifact_test_output.txt + +flake-detector: + dependencies: + - artifact test + stage: artifact-test + script: + - echo "Hello from flake-detector" + - ls -la + - ls -la ./output + artifacts: + paths: + - output/* + vulnerability: needs: [] stage: analyze -- GitLab