From bca1926cd110275a48ead7211584839a98f6d6cc Mon Sep 17 00:00:00 2001 From: Ziwen Ning Date: Fri, 29 Sep 2023 20:55:14 -0700 Subject: [PATCH] ci: add lint for e2e and unit Signed-off-by: Ziwen Ning --- .github/workflows/ci.yaml | 2 +- .golangci.yaml | 4 ++++ Makefile | 2 +- args_test.go | 20 ++++++++++---------- logger/awslogs/logger_test.go | 3 ++- logger/buffered_logger_test.go | 7 ++++--- logger/common_test.go | 30 ++++++++++++++++-------------- logger/fluentd/logger_test.go | 1 + logger/splunk/logger_test.go | 4 ++-- 9 files changed, 41 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 146e9a8..d82290a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -25,7 +25,7 @@ jobs: # There may not be an automatic way (e.g., dependabot) to update a specific parameter of a GitHub Action, # so we will just update it manually whenever it makes sense (e.g., a feature that we want is added). version: v1.54.0 - args: --fix=false --timeout=5m --out-format=colored-line-number + args: --build-tags e2e,unit --fix=false --timeout=5m --out-format=colored-line-number unit-tests: strategy: fail-fast: false diff --git a/.golangci.yaml b/.golangci.yaml index 49f840b..d877b6a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -22,6 +22,10 @@ linters-settings: stylecheck: # ST1003 is left out because it is a bit opinionated. checks: ["all", "-ST1003"] + paralleltest: + # Ignore missing calls to `t.Parallel()` and only report incorrect uses of it. + # Default: false + ignore-missing: true linters: enable: - errname diff --git a/Makefile b/Makefile index 8ed3dc5..a80a829 100644 --- a/Makefile +++ b/Makefile @@ -62,7 +62,7 @@ coverage: .PHONY: lint lint: $(SOURCES) - $(DEPSPATH)/golangci-lint run + $(DEPSPATH)/golangci-lint run --build-tags e2e,unit -i .PHONY: mdlint # Install it locally: https://github.com/igorshubovych/markdownlint-cli#installation diff --git a/args_test.go b/args_test.go index 9b818f7..e31a25e 100644 --- a/args_test.go +++ b/args_test.go @@ -51,7 +51,7 @@ func TestGetGlobalArgs(t *testing.T) { } // testGetGlobalArgsNoError is a sub-test of TestGetGlobalArgs. It tests -// getGlobalArgs function with no returned errors +// getGlobalArgs function with no returned errors. func testGetGlobalArgsNoError(t *testing.T) { // Unset all keys used for this test defer viper.Reset() @@ -67,7 +67,7 @@ func testGetGlobalArgsNoError(t *testing.T) { assert.Equal(t, args.LogDriver, testLogDriver) assert.Equal(t, args.Mode, blockingMode) assert.Equal(t, args.MaxBufferSize, 0) - assert.Equal(t, *args.CleanupTime, time.Duration(5*time.Second)) + assert.Equal(t, *args.CleanupTime, 5*time.Second) } // testGetGlobalArgsWithError is a sub-test of TestGetGlobalArgs. It tests @@ -197,7 +197,7 @@ func testGetMaxBufferSizeWithError(t *testing.T) { } } -// TestGetCleanupTime tests getCleanupTime with/without valid setting cleanup time options +// TestGetCleanupTime tests getCleanupTime with/without valid setting cleanup time options. func TestGetCleanupTime(t *testing.T) { t.Run("NoError", testGetCleanupTimeNoError) t.Run("WithError", testGetCleanupTimeWithError) @@ -213,9 +213,9 @@ func testGetCleanupTimeNoError(t *testing.T) { cleanupTime string expectedCleanupTime time.Duration }{ - {"3s", time.Duration(3 * time.Second)}, - {"10s", time.Duration(10 * time.Second)}, - {"12s", time.Duration(12 * time.Second)}, + {"3s", 3 * time.Second}, + {"10s", 10 * time.Second}, + {"12s", 12 * time.Second}, } for _, tc := range testCasesNoError { @@ -246,14 +246,14 @@ func testGetCleanupTimeWithError(t *testing.T) { } } -// TestGetDockerConfigs tests that we can correctly get the docker config input parameters +// TestGetDockerConfigs tests that we can correctly get the docker config input parameters. func TestGetDockerConfigs(t *testing.T) { t.Run("NoError", testGetDockerConfigsNoError) t.Run("Empty", testGetDockerConfigsEmpty) t.Run("WithError", testGetDockerConfigsWithError) } -// testGetDockerConfigsNoError tests that the correctly formatted input can be parsed without error +// testGetDockerConfigsNoError tests that the correctly formatted input can be parsed without error. func testGetDockerConfigsNoError(t *testing.T) { defer viper.Reset() @@ -283,7 +283,7 @@ func testGetDockerConfigsNoError(t *testing.T) { assert.Equal(t, true, reflect.DeepEqual(testContainerEnvMap, argsContainerEnvMap)) } -// testGetDockerConfigsEmpty tests that empty docker config input parameter generates no error +// testGetDockerConfigsEmpty tests that empty docker config input parameter generates no error. func testGetDockerConfigsEmpty(t *testing.T) { defer viper.Reset() @@ -291,7 +291,7 @@ func testGetDockerConfigsEmpty(t *testing.T) { require.NoError(t, err) } -// testGetDockerConfigsWithError tests that malformat docker config results in an error +// testGetDockerConfigsWithError tests that malformat docker config results in an error. func testGetDockerConfigsWithError(t *testing.T) { defer viper.Reset() testCaseWithError := "{invalidJsonMap" diff --git a/logger/awslogs/logger_test.go b/logger/awslogs/logger_test.go index 745fadf..ec634cb 100644 --- a/logger/awslogs/logger_test.go +++ b/logger/awslogs/logger_test.go @@ -16,7 +16,7 @@ const ( testGroup = "test-group" testRegion = "test-region" testStream = "test-stream" - testCredentialsEndpoint = "test-credential-endpoints" + testCredentialsEndpoint = "test-credential-endpoints" //nolint:gosec // not credentials testCreateGroup = "true" testCreateStream = "true" testMultilinePattern = "test-multiline-pattern" @@ -41,6 +41,7 @@ var ( // TestGetAWSLogsConfig tests if getAWSLogsConfig function converts log config // maps correctly. func TestGetAWSLogsConfig(t *testing.T) { + t.Parallel() expectedConfig := map[string]string{ GroupKey: testGroup, RegionKey: testRegion, diff --git a/logger/buffered_logger_test.go b/logger/buffered_logger_test.go index 9876407..809e169 100644 --- a/logger/buffered_logger_test.go +++ b/logger/buffered_logger_test.go @@ -49,9 +49,9 @@ func testEnqueue(t *testing.T) *ringBuffer { return lb } -// TestLogBufferEnqueueDequeue tests dequeue operations from -// buffer +// TestLogBufferEnqueueDequeue tests dequeue operations from buffer. func TestLogBufferEnqueueDequeue(t *testing.T) { + t.Parallel() lb := testEnqueue(t) queueLen := len(lb.queue) for i := 0; i < queueLen; i++ { @@ -63,8 +63,9 @@ func TestLogBufferEnqueueDequeue(t *testing.T) { require.Len(t, lb.queue, 0) } -// TestLogBufferEnqueueFlush tests flush messages from buffer +// TestLogBufferEnqueueFlush tests flush messages from buffer. func TestLogBufferEnqueueFlush(t *testing.T) { + t.Parallel() lb := testEnqueue(t) flushedMsg := lb.Flush() require.Len(t, lb.queue, 0) diff --git a/logger/common_test.go b/logger/common_test.go index 6f7f94e..2348f15 100644 --- a/logger/common_test.go +++ b/logger/common_test.go @@ -29,10 +29,9 @@ const ( ) var ( - dummyLogMsg = []byte("test log message") dummySource = "stdout" dummyTime = time.Date(2020, time.January, 14, 01, 59, 0, 0, time.UTC) - dummyCleanupTime = time.Duration(2 * time.Second) + dummyCleanupTime = 2 * time.Second logDestinationFileName string ) @@ -54,15 +53,15 @@ func (d *dummyClient) Log(msg *dockerlogger.Message) error { if err != nil { return err } - f, err := os.OpenFile(logDestinationFileName, os.O_APPEND|os.O_RDWR, 0644) + f, err := os.OpenFile(logDestinationFileName, os.O_APPEND|os.O_RDWR, 0644) //nolint:gosec // testing only if err != nil { return fmt.Errorf("unable to open file %s to record log message: %w", logDestinationFileName, err) } - defer f.Close() + defer f.Close() //nolint:errcheck // testing only b, err = json.Marshal(msg) require.NoError(d.t, err) - f.Write(b) - f.Write([]byte{'\n'}) + f.Write(b) //nolint:errcheck,gosec // testing only + f.Write([]byte{'\n'}) //nolint:errcheck,gosec // testing only return nil } @@ -75,9 +74,9 @@ func checkLogFile(t *testing.T, fileName string, expectedNumLines int, lastPartialID string lastPartialOrdinal int ) - file, err := os.Open(fileName) + file, err := os.Open(fileName) //nolint:gosec // testing only require.NoError(t, err) - defer file.Close() + defer file.Close() //nolint:errcheck // testing only scanner := bufio.NewScanner(file) lines := 0 @@ -111,7 +110,7 @@ func checkLogFile(t *testing.T, fileName string, expectedNumLines int, // to mock destination. In this test case, the source and destination are both tmp files that // read from and write to inside the customized Log function. func TestSendLogs(t *testing.T) { - + t.Parallel() for _, tc := range []struct { testName string bufferSizeInBytes int @@ -153,22 +152,24 @@ func TestSendLogs(t *testing.T) { expectedPartialOrdinalSequence: []int{1, 2, 3, 1, 2, 3}, }, } { + tc := tc t.Run(tc.testName, func(t *testing.T) { + t.Parallel() l := &Logger{ Info: &dockerlogger.Info{}, Stream: &dummyClient{t}, - bufferSizeInBytes: tc.bufferSizeInBytes, - maxReadBytes: tc.maxReadBytes, + bufferSizeInBytes: tc.bufferSizeInBytes, //nolint:govet // testing only + maxReadBytes: tc.maxReadBytes, //nolint:govet // testing only } // Create a tmp file that used to mock the io pipe where the logger reads log // messages from. tmpIOSource, err := os.CreateTemp("", "") require.NoError(t, err) - defer os.Remove(tmpIOSource.Name()) + defer os.Remove(tmpIOSource.Name()) //nolint:errcheck // testing only var ( testPipe bytes.Buffer ) - for _, logMessage := range tc.logMessages { + for _, logMessage := range tc.logMessages { //nolint:govet // testing only _, err := testPipe.WriteString(logMessage + "\n") require.NoError(t, err) } @@ -177,7 +178,7 @@ func TestSendLogs(t *testing.T) { // logger sends log messages to. tmpDest, err := os.CreateTemp("", "") require.NoError(t, err) - defer os.Remove(tmpDest.Name()) + defer os.Remove(tmpDest.Name()) //nolint:errcheck // testing only logDestinationFileName = tmpDest.Name() var errGroup errgroup.Group @@ -200,6 +201,7 @@ func TestSendLogs(t *testing.T) { // TestNewInfo tests if NewInfo function creates logger info correctly. func TestNewInfo(t *testing.T) { + t.Parallel() config := map[string]string{ "configKey1": "configVal1", "configKey2": "configVal2", diff --git a/logger/fluentd/logger_test.go b/logger/fluentd/logger_test.go index e3470fd..1902145 100644 --- a/logger/fluentd/logger_test.go +++ b/logger/fluentd/logger_test.go @@ -31,6 +31,7 @@ var ( ) func TestGetFluentdConfig(t *testing.T) { + t.Parallel() expectedConfig := map[string]string{ AddressKey: testAddress, AsyncConnectKey: testAsyncConnect, diff --git a/logger/splunk/logger_test.go b/logger/splunk/logger_test.go index 1d6fe64..5ce0f02 100644 --- a/logger/splunk/logger_test.go +++ b/logger/splunk/logger_test.go @@ -54,9 +54,9 @@ var ( } ) -// TestGetSplunkConfig tests if all arguments are converted in correctly -// for splunk driver configuration +// TestGetSplunkConfig tests if all arguments are converted in correctly for splunk driver configuration. func TestGetSplunkConfig(t *testing.T) { + t.Parallel() expectedConfig := map[string]string{ TokenKey: testToken, URLKey: testURL,