-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add goTestUnit and goTestIntegration to magefile #7766
Conversation
Consider these targets as incubating. They are not used by any of the Makefiles yet. I added it to the Windows CI only to start testing it out and to resolve an issue where compilation could fail and success would be reported by the powershell script. ``` $ mage -h goTestUnit mage gotestunit: GoTestUnit executes the Go unit tests. Use TEST_COVERAGE=true to enable code coverage profiling. Use RACE_DETECTOR=true to enable the race detector. ``` ``` $ TEST_COVERAGE=true RACE_DETECTOR=true mage goTestUnit >> go test: Unit Testing SUMMARY: Fail: 0 Skip: 2 Pass: 807 Packages: 70 Duration: 21.459313277s Coverage Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.html JUnit Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.xml Output File: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.out >> go test: Unit Test Passed ``` ``` $ TEST_COVERAGE=true RACE_DETECTOR=true mage goTestUnit >> go test: Unit Testing FAILURES: Package: github.com/elastic/beats/libbeat/processors Test: TestDemo processor_test.go:36: Only failing tests are logged. But you can use 'mage -v goTestUnit' to see all of the go test output or just view the output file list in the summary. ---- SUMMARY: Fail: 1 Skip: 2 Pass: 807 Packages: 70 Duration: 21.53730358s Coverage Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.html JUnit Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.xml Output File: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.out >> go test: Unit Test Failed Error: go test failed: 1 test failures $ echo $? 1 ```
1aea460
to
6830908
Compare
Reminds me a bit of #4960 Perhaps we could take this opportunity to change the ENV variable names and also change a bit which commands run which tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ on this change.
@@ -96,6 +97,20 @@ func Fields() error { | |||
return mage.GenerateFieldsYAML("module") | |||
} | |||
|
|||
// GoTestUnit executes the Go unit tests. | |||
// Use TEST_COVERAGE=true to enable code coverage profiling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we call this just COVERAGE
and RACE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that they should both be prefixed in some way to avoid collisions and be self-describing. How prefixing any variables that control test parameters with TEST_
? Like...
TEST_RACE
TEST_COVERAGE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
} | ||
if goTestErr != nil && len(report.Packages) == 0 { | ||
// No packages were tested. Probably the code didn't compile. | ||
fmt.Println(bytes.NewBuffer(bufferOutput.Bytes()).String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if in here we could also use the log package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In mage
the stdlib log
package is tied to the -v
parameter so it gets used for adding debug information to build.
stdout is the main UI for the build tool. So fmt.Print
is used for messages that always need to be presented to the user. The goal is to keep verbosity down so that real warnings and problems are easily observed.
I think switching to a leveled logger would be best similar to how Gradle handles it.
And I try to only fmt.Print
when necessary such that the build only gets verbose when there is a failure.
// GoTestUnit executes the Go unit tests. | ||
// Use TEST_COVERAGE=true to enable code coverage profiling. | ||
// Use RACE_DETECTOR=true to enable the race detector. | ||
func GoTestUnit(ctx context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we call this just UnitTest
? Would be great if the names of the commands would be as intuitive and rememberable as possible (I know, very subjective topic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have targets for unitTest
and integrationTest
. The unitTest target would then have dependency on goUnitTest
and pythonUnitTest
?
Then do the same thing for with integrationTest
by making it depend on goIntegrationTest
and pythonIntegrationTest
.
Then if needed we also add benchTest
and loadTest
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. Very similar to #4960
For bench and load I wonder if they should be their own target or a ENV variable. We can discuss that in a follow up.
// GoTestUnit executes the Go unit tests. | ||
// Use TEST_COVERAGE=true to enable code coverage profiling. | ||
// Use RACE_DETECTOR=true to enable the race detector. | ||
func GoTestUnit(ctx context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit a pity that each Beat needs to have all commands inside. Is there a way that mage could "inherit" commands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully in the future this is supported by mage: magefile/mage#108
@elastic/apm-server This might be interesting for the team. |
…efile (#8294) * Add goTestUnit and goTestIntegration to magefile Consider these targets as incubating. They are not used by any of the Makefiles yet. I added it to the Windows CI only to start testing it out and to resolve an issue where compilation could fail and success would be reported by the powershell script. ``` $ mage -h goTestUnit mage gotestunit: GoTestUnit executes the Go unit tests. Use TEST_COVERAGE=true to enable code coverage profiling. Use RACE_DETECTOR=true to enable the race detector. ``` ``` $ TEST_COVERAGE=true RACE_DETECTOR=true mage goTestUnit >> go test: Unit Testing SUMMARY: Fail: 0 Skip: 2 Pass: 807 Packages: 70 Duration: 21.459313277s Coverage Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.html JUnit Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.xml Output File: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.out >> go test: Unit Test Passed ``` ``` $ TEST_COVERAGE=true RACE_DETECTOR=true mage goTestUnit >> go test: Unit Testing FAILURES: Package: github.com/elastic/beats/libbeat/processors Test: TestDemo processor_test.go:36: Only failing tests are logged. But you can use 'mage -v goTestUnit' to see all of the go test output or just view the output file list in the summary. ---- SUMMARY: Fail: 1 Skip: 2 Pass: 807 Packages: 70 Duration: 21.53730358s Coverage Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.html JUnit Report: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.xml Output File: /Users/akroh/go/src/github.com/elastic/beats/libbeat/build/TEST-go-unit.out >> go test: Unit Test Failed Error: go test failed: 1 test failures $ echo $? 1 ``` (cherry picked from commit f52f069) * Add github.com/jstemmer/go-junit-report to vendor (cherry picked from commit 5f05ac7)
Consider these targets as incubating. They are not used by any of the Makefiles yet.
I added it to the Windows CI only to start testing it out and to resolve an issue where
compilation could fail and success would be reported by the powershell script.