Skip to content

Commit

Permalink
Fix mage unitTest to actually run tests (#3053)
Browse files Browse the repository at this point in the history 
* Add RunExpr parameter to fix unit tests.

The TestName parameter is intended for use in logging, passing it to
-run unconditionally sets it to 'Unit' when running mage unitTest
bypassing all tests.

* Rename TestName parameter to avoid confusion.

* Adjust gotest tests for Go 1.20 formatting changes.
  • Loading branch information
@cmacknz
cmacknz authored Jul 11, 2023
1 parent 3b6f88d commit bece71e
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 22 deletions.
17 changes: 9 additions & 8 deletions dev-tools/mage/gotest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import (
// GoTestArgs are the arguments used for the "go*Test" targets and they define
// how "go test" is invoked. "go test" is always invoked with -v for verbose.
type GoTestArgs struct {
TestName string // Test name used in logging.
LogName string // Test name used in logging.
RunExpr string // Expression to pass to the -run argument of go test.
Race bool // Enable race detector.
Tags []string // Build tags to enable.
ExtraFlags []string // Extra flags to pass to 'go test'.
Expand All @@ -49,7 +50,7 @@ type TestBinaryArgs struct {
func makeGoTestArgs(name string) GoTestArgs {
fileName := fmt.Sprintf("build/TEST-go-%s", strings.Replace(strings.ToLower(name), " ", "_", -1))
params := GoTestArgs{
TestName: name,
LogName: name,
Race: RaceDetector,
Packages: []string{"./..."},
OutputFile: fileName + ".out",
Expand All @@ -66,7 +67,7 @@ func makeGoTestArgsForModule(name, module string) GoTestArgs {
fileName := fmt.Sprintf("build/TEST-go-%s-%s", strings.Replace(strings.ToLower(name), " ", "_", -1),
strings.Replace(strings.ToLower(module), " ", "_", -1))
params := GoTestArgs{
TestName: fmt.Sprintf("%s-%s", name, module),
LogName: fmt.Sprintf("%s-%s", name, module),
Race: RaceDetector,
Packages: []string{fmt.Sprintf("./module/%s/...", module)},
OutputFile: fileName + ".out",
Expand Down Expand Up @@ -182,7 +183,7 @@ func InstallGoTestTools() {
func GoTest(ctx context.Context, params GoTestArgs) error {
mg.Deps(InstallGoTestTools)

fmt.Println(">> go test:", params.TestName, "Testing")
fmt.Println(">> go test:", params.LogName, "Testing")

// We use gotestsum to drive the tests and produce a junit report.
// The tool runs `go test -json` in order to produce a structured log which makes it easier
Expand Down Expand Up @@ -233,8 +234,8 @@ func GoTest(ctx context.Context, params GoTestArgs) error {
)
}

if params.TestName != "" {
testArgs = append(testArgs, "-run", params.TestName)
if params.RunExpr != "" {
testArgs = append(testArgs, "-run", params.RunExpr)
}

testArgs = append(testArgs, params.ExtraFlags...)
Expand Down Expand Up @@ -336,11 +337,11 @@ func GoTest(ctx context.Context, params GoTestArgs) error {

// Return an error indicating that testing failed.
if goTestErr != nil {
fmt.Println(">> go test:", params.TestName, "Test Failed")
fmt.Println(">> go test:", params.LogName, "Test Failed")
return errors.Wrap(goTestErr, "go test returned a non-zero value")
}

fmt.Println(">> go test:", params.TestName, "Test Passed")
fmt.Println(">> go test:", params.LogName, "Test Passed")
return nil
}

Expand Down
23 changes: 12 additions & 11 deletions dev-tools/mage/gotest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestGoTest_CaptureOutput(t *testing.T) {
errNonZero := "go test returned a non-zero value"
makeArgs := func(test string) GoTestArgs {
return GoTestArgs{
TestName: "asserts",
LogName: "asserts",
Packages: []string{"."},
Env: map[string]string{envGoTestHelper: "1"},
ExtraFlags: []string{"-test.run", test},
Expand Down Expand Up @@ -197,41 +197,41 @@ var wantTestAssertOutput = `(?sm:
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/assert_fails.*
--- FAIL: TestGoTest_Helper_AssertOutput/assert_fails .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/assert_with_message .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/assert_with_message.*
Messages: My message.*
--- FAIL: TestGoTest_Helper_AssertOutput/assert_with_message .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/assert_with_messagef .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/assert_with_messagef.*
Messages: My message with arguments: 42.*
--- FAIL: TestGoTest_Helper_AssertOutput/assert_with_messagef .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/require_fails .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/require_fails.*
--- FAIL: TestGoTest_Helper_AssertOutput/require_fails .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/require_with_message .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/require_with_message.*
Messages: My message.*
--- FAIL: TestGoTest_Helper_AssertOutput/require_with_message .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/require_with_messagef .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Error: Should be true.*
Test: TestGoTest_Helper_AssertOutput/require_with_messagef.*
Messages: My message with arguments: 42.*
--- FAIL: TestGoTest_Helper_AssertOutput/require_with_messagef .*
=== FAIL: dev-tools/mage TestGoTest_Helper_AssertOutput/equals_map .*
.*gotest_test.go:\d+:.*
Error Trace: .*gotest_test.go:\d+.*
Expand Down Expand Up @@ -293,17 +293,17 @@ var wantTestLogOutput = `(?sm:
gotest_test.go:\d+: printf style log message: 42.*
gotest_test.go:\d+: Log should fail.*
gotest_test.go:\d+: Log should fail with printf style log: 23.*
--- FAIL: TestGoTest_Helper_LogOutput/on_error.*
=== FAIL: dev-tools/mage TestGoTest_Helper_LogOutput/on_fatal.*
gotest_test.go:\d+: Log message should be printed.*
gotest_test.go:\d+: printf style log message: 42.*
gotest_test.go:\d+: Log should fail.*
--- FAIL: TestGoTest_Helper_LogOutput/on_fatal.*
=== FAIL: dev-tools/mage TestGoTest_Helper_LogOutput/on_fatalf.*
gotest_test.go:\d+: Log message should be printed.*
gotest_test.go:\d+: printf style log message: 42.*
gotest_test.go:\d+: Log should fail with printf style log: 42.*
--- FAIL: TestGoTest_Helper_LogOutput/on_fatalf.*
=== FAIL: dev-tools/mage TestGoTest_Helper_LogOutput/with_newlines.*
gotest_test.go:\d+: Log.*
message.*
Expand All @@ -323,8 +323,9 @@ var wantTestLogOutput = `(?sm:
style.*
log:.*
42.*
--- FAIL: TestGoTest_Helper_LogOutput/with_newlines.*
=== FAIL: dev-tools/mage TestGoTest_Helper_LogOutput.*
DONE 5 tests, 5 failures in.*
)`

Expand Down
6 changes: 3 additions & 3 deletions magefile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1336,9 +1336,9 @@ func (Integration) Local(ctx context.Context, testName string) error {
params.Tags = append(params.Tags, "local")
params.Packages = []string{"github.com/elastic/elastic-agent/testing/integration"}
if testName == "all" {
params.TestName = ""
params.RunExpr = ""
} else {
params.TestName = testName
params.RunExpr = testName
}
return devtools.GoTest(ctx, params)
}
Expand Down Expand Up @@ -1426,7 +1426,7 @@ func (Integration) TestOnRemote(ctx context.Context) error {
testName := fmt.Sprintf("remote-%s", testPrefix)
fileName := fmt.Sprintf("build/TEST-go-%s", testName)
params := mage.GoTestArgs{
TestName: testName,
LogName: testName,
OutputFile: fileName + ".out",
JUnitReportFile: fileName + ".xml",
Packages: []string{packageName},
Expand Down

0 comments on commit bece71e

Please sign in to comment.