From b891fa83564fa0ca6673c16d5ddcfaa54cf98e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Tue, 30 Jul 2024 09:44:23 +0100 Subject: [PATCH] Update test runner to Go 1.22 (#113) * Update test runner to Go 1.22 * Fix smoke test expectation * Update golangci-lint --- .github/workflows/test.yml | 10 +- Dockerfile | 2 +- external-packages/go.mod | 2 + go.mod | 5 +- go.sum | 12 +- testrunner/execute.go | 163 ++++++++++++------ testrunner/execute_test.go | 28 ++- testrunner/testdata/expected/broken.json | 2 +- .../testdata/expected/missing_func.json | 2 +- tests/syntax-error/expected_results.json | 2 +- 10 files changed, 149 insertions(+), 79 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29b7fe2..aab6091 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,13 +9,13 @@ jobs: - name: Install Go uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 with: - go-version: 1.21.x + go-version: 1.22.x - name: Checkout code uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b - name: Run linters - uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc + uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 with: - version: v1.51 + version: v1.59.1 test: strategy: @@ -27,7 +27,7 @@ jobs: if: success() uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 with: - go-version: 1.21.x + go-version: 1.22.x - name: Checkout code uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b - name: Run tests @@ -64,7 +64,7 @@ jobs: if: success() uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 with: - go-version: 1.21.x + go-version: 1.22.x - name: Checkout code uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b - name: Calc coverage diff --git a/Dockerfile b/Dockerfile index f71d726..5283f33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.21.1-alpine3.17 +FROM golang:1.22.5-alpine3.20 # Add addtional packages needed for the race detector to work RUN apk add --update build-base make diff --git a/external-packages/go.mod b/external-packages/go.mod index 5c2abe4..4248927 100644 --- a/external-packages/go.mod +++ b/external-packages/go.mod @@ -1,5 +1,7 @@ module external_packages +// This version should be the same as the default version of go +// in the go.mod files given to the students go 1.18 require ( diff --git a/go.mod b/go.mod index db788b7..7ad9937 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,11 @@ module github.com/exercism/go-test-runner -go 1.19 +go 1.22 + +require github.com/stretchr/testify v1.9.0 require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/testify v1.8.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 658c3d6..60ce688 100644 --- a/go.sum +++ b/go.sum @@ -1,16 +1,10 @@ -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= 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/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= -github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +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.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/testrunner/execute.go b/testrunner/execute.go index ae576b1..8b98522 100644 --- a/testrunner/execute.go +++ b/testrunner/execute.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "encoding/json" - "errors" "fmt" "log" "os" @@ -54,15 +53,16 @@ func Execute(input_dir string) []byte { ver := 3 exerciseConfig := parseExerciseConfig(input_dir) + cmdres, testsOk := runTests(input_dir, exerciseConfig.TestingFlags) + testOutput, err := parseTestOutput(cmdres) + if err != nil { + log.Fatalf("parsing test output: %s", err) + } - if cmdres, ok := runTests(input_dir, exerciseConfig.TestingFlags); ok { - report = getStructure(cmdres, input_dir, ver, exerciseConfig.TaskIDsEnabled) + if testsOk { + report = getStructureForTestsOk(testOutput, input_dir, ver, exerciseConfig.TaskIDsEnabled) } else { - report = &testReport{ - Status: statErr, - Version: ver, - Message: cmdres.String(), - } + report = getStructureForTestsNotOk(testOutput, ver) } bts, err := json.MarshalIndent(report, "", "\t") @@ -72,7 +72,31 @@ func Execute(input_dir string) []byte { return bts } -func getStructure(lines bytes.Buffer, input_dir string, ver int, taskIDsEnabled bool) *testReport { +func getStructureForTestsNotOk(parsedOutput *parsedTestOutput, ver int) *testReport { + report := &testReport{ + Status: statErr, + Version: ver, + Message: parsedOutput.joinPackageMessages("\n"), + } + + report.Message += parsedOutput.joinFailMessages("\n") + + jsonOutputMessages := make([]string, 0) + + for _, line := range parsedOutput.testLines { + if line.Action == "output" { + jsonOutputMessages = append(jsonOutputMessages, line.Output) + } + } + + if len(jsonOutputMessages) > 0 { + report.Message += "\n" + strings.Join(jsonOutputMessages, "\n") + } + + return report +} + +func getStructureForTestsOk(parsedOutput *parsedTestOutput, input_dir string, ver int, taskIDsEnabled bool) *testReport { report := &testReport{ Status: statPass, Version: ver, @@ -84,10 +108,17 @@ func getStructure(lines bytes.Buffer, input_dir string, ver int, taskIDsEnabled } }() - tests, err := processTestResults(lines, input_dir, taskIDsEnabled) - if err != nil { + tests := processTestResults(parsedOutput, input_dir, taskIDsEnabled) + + if parsedOutput.hasFailMessages() { + report.Status = statErr + report.Message = parsedOutput.joinFailMessages("\n") + return report + } + + if len(tests) == 0 && parsedOutput.hasPackageMessages() { report.Status = statErr - report.Message = err.Error() + report.Message = parsedOutput.joinPackageMessages("") return report } @@ -117,49 +148,88 @@ func getStructure(lines bytes.Buffer, input_dir string, ver int, taskIDsEnabled return report } -func processTestResults(lines bytes.Buffer, input_dir string, taskIDsEnabled bool) ([]testResult, error) { - var ( - results = []testResult{} - resultIdxByName = make(map[string]int) - failMsg [][]byte - pkgLevelMsg string - ) +type parsedTestOutput struct { + testLines []testLine + pkgLevelMessages []string + failMessages []string +} - testFile := FindTestFile(input_dir) - rootLevelTests := FindAllRootLevelTests(testFile) - rootLevelTestsMap := ConvertToMapByTestName(rootLevelTests) +func (out *parsedTestOutput) hasFailMessages() bool { + return len(out.failMessages) > 0 +} + +func (out *parsedTestOutput) joinFailMessages(sep string) string { + return strings.Join(out.failMessages, sep) +} + +func (out *parsedTestOutput) hasPackageMessages() bool { + return len(out.pkgLevelMessages) > 0 +} + +func (out *parsedTestOutput) joinPackageMessages(sep string) string { + return strings.Join(out.pkgLevelMessages, sep) +} + +func parseTestOutput(lines bytes.Buffer) (*parsedTestOutput, error) { + parsedOutput := &parsedTestOutput{ + testLines: make([]testLine, 0), + pkgLevelMessages: make([]string, 0), + failMessages: make([]string, 0), + } scanner := bufio.NewScanner(&lines) for scanner.Scan() { lineBytes := scanner.Bytes() var line testLine - switch { - case len(lineBytes) == 0: + if len(lineBytes) == 0 { continue - case !bytes.HasPrefix(lineBytes, []byte{'{'}): + } + + if !bytes.HasPrefix(lineBytes, []byte{'{'}) { // if the line is not a json, we need to collect the lines to gather why `go test --json` failed - failMsg = append(failMsg, lineBytes) + parsedOutput.failMessages = append(parsedOutput.failMessages, string(lineBytes)) continue } if err := json.Unmarshal(lineBytes, &line); err != nil { - log.Println(err) - continue + return nil, fmt.Errorf("parsing line starting with '{' as json: %w", err) } if line.Test == "" { // We collect messages that do not belong to an individual test and use them later // as error message in case there was no test level message found at all. - pkgLevelMsg += line.Output + if line.Output != "" { + parsedOutput.pkgLevelMessages = append(parsedOutput.pkgLevelMessages, line.Output) + } continue } - switch line.Action { + parsedOutput.testLines = append(parsedOutput.testLines, line) + } + + return parsedOutput, nil +} + +func processTestResults( + parsedOutput *parsedTestOutput, + input_dir string, + taskIDsEnabled bool, +) []testResult { + + results := make([]testResult, 0) + resultIdxByName := make(map[string]int) + + testFile := FindTestFile(input_dir) + rootLevelTests := FindAllRootLevelTests(testFile) + rootLevelTestsMap := ConvertToMapByTestName(rootLevelTests) + + for _, parsedLine := range parsedOutput.testLines { + switch parsedLine.Action { case "run": - tc, taskID := ExtractTestCodeAndTaskID(rootLevelTestsMap, line.Test) + tc, taskID := ExtractTestCodeAndTaskID(rootLevelTestsMap, parsedLine.Test) result := testResult{ - Name: line.Test, + Name: parsedLine.Test, // Use error as default state in case no other state is found later. // No state is provided e.g. when there is a stack overflow. Status: statErr, @@ -172,43 +242,34 @@ func processTestResults(lines bytes.Buffer, input_dir string, taskIDsEnabled boo results = append(results, result) resultIdxByName[result.Name] = len(results) - 1 case "output": - if idx, found := resultIdxByName[line.Test]; found { - results[idx].Message += "\n" + line.Output + if idx, found := resultIdxByName[parsedLine.Test]; found { + results[idx].Message += "\n" + parsedLine.Output } else { - log.Printf("cannot extend message for unknown test: %s\n", line.Test) + log.Printf("cannot extend message for unknown test: %s\n", parsedLine.Test) continue } case statFail: - if idx, found := resultIdxByName[line.Test]; found { + if idx, found := resultIdxByName[parsedLine.Test]; found { results[idx].Status = statFail } else { - log.Printf("cannot set failed status for unknown test: %s\n", line.Test) + log.Printf("cannot set failed status for unknown test: %s\n", parsedLine.Test) continue } case statPass: - if idx, found := resultIdxByName[line.Test]; found { + if idx, found := resultIdxByName[parsedLine.Test]; found { results[idx].Status = statPass } else { - log.Printf("cannot set passing status for unknown test: %s\n", line.Test) + log.Printf("cannot set passing status for unknown test: %s\n", parsedLine.Test) continue } case statSkip: - if idx, found := resultIdxByName[line.Test]; found { + if idx, found := resultIdxByName[parsedLine.Test]; found { results[idx].Status = statSkip } else { - log.Printf("cannot set skipped status for unknown test: %s\n", line.Test) + log.Printf("cannot set skipped status for unknown test: %s\n", parsedLine.Test) continue } } - - } - - if len(failMsg) != 0 { - return nil, errors.New(string(bytes.Join(failMsg, []byte{'\n'}))) - } - - if len(results) == 0 && pkgLevelMsg != "" { - return nil, errors.New(pkgLevelMsg) } if taskIDsEnabled { @@ -217,7 +278,7 @@ func processTestResults(lines bytes.Buffer, input_dir string, taskIDsEnabled boo results = addNonExecutedTests(rootLevelTests, results) } - return results, nil + return results } // addNonExecutedTests adds tests to the result set that were not executed. diff --git a/testrunner/execute_test.go b/testrunner/execute_test.go index 2738603..746060b 100644 --- a/testrunner/execute_test.go +++ b/testrunner/execute_test.go @@ -15,18 +15,25 @@ const version = 3 /* Ideally, tests should be performed via running the tool and checking the resulting json file matches the expected output. This is done in integration_test.go in the project root. - Tests should only be added in this file is comparing the full output is not possible for some reason. + Tests should only be added in this file if comparing the full output is not possible for some reason. */ func TestRunTests_RuntimeError(t *testing.T) { input_dir := filepath.Join("testdata", "practice", "runtime_error") + cmdres, ok := runTests(input_dir, nil) if !ok { fmt.Printf("runtime error test expected to return ok: %s", cmdres.String()) } - output := getStructure(cmdres, input_dir, version, false) - jsonBytes, err := json.MarshalIndent(output, "", "\t") + testOutput, err := parseTestOutput(cmdres) + if err != nil { + t.Fatalf("parsing test output: %s", err) + } + + report := getStructureForTestsOk(testOutput, input_dir, version, false) + + jsonBytes, err := json.MarshalIndent(report, "", "\t") if err != nil { t.Fatalf("runtime error output not valid json: %s", err) } @@ -56,13 +63,18 @@ func TestRunTests_RaceDetector(t *testing.T) { fmt.Printf("race detector test expected to return ok: %s", cmdres.String()) } - output := getStructure(cmdres, input_dir, version, false) - if output.Status != "fail" { - t.Errorf("wrong status for race detector test: got %q, want %q", output.Status, "fail") + testOutput, err := parseTestOutput(cmdres) + if err != nil { + t.Errorf("parsing test output: %s", err) + } + + report := getStructureForTestsOk(testOutput, input_dir, version, false) + if report.Status != "fail" { + t.Errorf("wrong status for race detector test: got %q, want %q", report.Status, "fail") } - if !strings.Contains(output.Tests[0].Message, "WARNING: DATA RACE") { - t.Errorf("no data race error included in message: %s", output.Tests[0].Message) + if !strings.Contains(report.Tests[0].Message, "WARNING: DATA RACE") { + t.Errorf("no data race error included in message: %s", report.Tests[0].Message) } } diff --git a/testrunner/testdata/expected/broken.json b/testrunner/testdata/expected/broken.json index 8e26499..a84bb3a 100644 --- a/testrunner/testdata/expected/broken.json +++ b/testrunner/testdata/expected/broken.json @@ -1,6 +1,6 @@ { "status": "error", "version": 3, - "message": "# gigasecond [gigasecond.test]\n./broken.go: undefined: unknownVar\n./broken.go: undefined: UnknownFunction\nFAIL\tgigasecond [build failed]\n'PATH_PLACEHOLDER test --short --json .' returned exit code 1: exit status 1", + "message": "FAIL\tgigasecond [build failed]\n# gigasecond [gigasecond.test]\n./broken.go: undefined: unknownVar\n./broken.go: undefined: UnknownFunction\n'PATH_PLACEHOLDER test --short --json .' returned exit code 1: exit status 1", "tests": null } \ No newline at end of file diff --git a/testrunner/testdata/expected/missing_func.json b/testrunner/testdata/expected/missing_func.json index 1a402ed..a58224f 100644 --- a/testrunner/testdata/expected/missing_func.json +++ b/testrunner/testdata/expected/missing_func.json @@ -1,6 +1,6 @@ { "status": "error", "version": 3, - "message": "# gigasecond [gigasecond.test]\n./missing_func_test.go: undefined: AddGigasecond\n./missing_func_test.go: undefined: AddGigasecond\nFAIL\tgigasecond [build failed]\n'PATH_PLACEHOLDER test --short --json .' returned exit code 1: exit status 1", + "message": "FAIL\tgigasecond [build failed]\n# gigasecond [gigasecond.test]\n./missing_func_test.go: undefined: AddGigasecond\n./missing_func_test.go: undefined: AddGigasecond\n'PATH_PLACEHOLDER test --short --json .' returned exit code 1: exit status 1", "tests": null } \ No newline at end of file diff --git a/tests/syntax-error/expected_results.json b/tests/syntax-error/expected_results.json index e10b97c..227d84c 100644 --- a/tests/syntax-error/expected_results.json +++ b/tests/syntax-error/expected_results.json @@ -1,6 +1,6 @@ { "status": "error", "version": 3, - "message": "# leap [leap.test]\n./leap.go:3:16: invalid character U+0024 '$'\n./leap.go:3:17: invalid character U+0040 '@'\n./leap.go:3:18: invalid character U+0023 '#'\n./leap.go:3:19: invalid character U+0024 '$'\n./leap.go:3:20: syntax error: unexpected ^, expected (\nFAIL\tleap [build failed]\n'/usr/local/go/bin/go test --short --json .' returned exit code 1: exit status 1", + "message": "FAIL\tleap [build failed]\n# leap [leap.test]\n./leap.go:3:16: invalid character U+0024 '$'\n./leap.go:3:17: invalid character U+0040 '@'\n./leap.go:3:18: invalid character U+0023 '#'\n./leap.go:3:19: invalid character U+0024 '$'\n./leap.go:3:20: syntax error: unexpected ^, expected (\n'/usr/local/go/bin/go test --short --json .' returned exit code 1: exit status 1", "tests": null } \ No newline at end of file