Revert "lint: Enable paralleltest, fix issues (uber-go#1367)" (uber-g…

…o#1368)

The PR was merged prematurely.
Maintainers are not yet in agreement about this change.

This reverts commit 7b6c114.

.golangci.yml

@@ -18,8 +18,7 @@ linters:
 
     # Our own extras:
     - gofumpt
-    - nolintlint   # lints nolint directives
-    - paralleltest # requires t.Parallel on all tests
+    - nolintlint # lints nolint directives
     - revive
 
 linters-settings:

array_test.go

@@ -53,8 +53,6 @@ func BenchmarkBoolsReflect(b *testing.B) {
 }
 
 func TestArrayWrappers(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		desc     string
 		field    Field
@@ -103,16 +101,11 @@ func TestArrayWrappers(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
-			enc := zapcore.NewMapObjectEncoder()
-			tt.field.Key = "k"
-			tt.field.AddTo(enc)
-			assert.Equal(t, tt.expected, enc.Fields["k"], "unexpected map contents")
-			assert.Equal(t, 1, len(enc.Fields), "found extra keys in map: %v", enc.Fields)
-		})
+		enc := zapcore.NewMapObjectEncoder()
+		tt.field.Key = "k"
+		tt.field.AddTo(enc)
+		assert.Equal(t, tt.expected, enc.Fields["k"], "%s: unexpected map contents.", tt.desc)
+		assert.Equal(t, 1, len(enc.Fields), "%s: found extra keys in map: %v", tt.desc, enc.Fields)
 	}
 }
 

buffer/buffer_test.go

@@ -30,86 +30,32 @@ import (
 )
 
 func TestBufferWrites(t *testing.T) {
-	t.Parallel()
+	buf := NewPool().Get()
 
 	tests := []struct {
 		desc string
-		f    func(*Buffer)
+		f    func()
 		want string
 	}{
-		{
-			desc: "AppendByte",
-			f:    func(buf *Buffer) { buf.AppendByte('v') },
-			want: "v",
-		},
-		{
-			desc: "AppendString",
-			f:    func(buf *Buffer) { buf.AppendString("foo") },
-			want: "foo",
-		},
-		{
-			desc: "AppendIntPositive",
-			f:    func(buf *Buffer) { buf.AppendInt(42) },
-			want: "42",
-		},
-		{
-			desc: "AppendIntNegative",
-			f:    func(buf *Buffer) { buf.AppendInt(-42) },
-			want: "-42",
-		},
-		{
-			desc: "AppendUint",
-			f:    func(buf *Buffer) { buf.AppendUint(42) },
-			want: "42",
-		},
-		{
-			desc: "AppendBool",
-			f:    func(buf *Buffer) { buf.AppendBool(true) },
-			want: "true",
-		},
-		{
-			desc: "AppendFloat64",
-			f:    func(buf *Buffer) { buf.AppendFloat(3.14, 64) },
-			want: "3.14",
-		},
+		{"AppendByte", func() { buf.AppendByte('v') }, "v"},
+		{"AppendString", func() { buf.AppendString("foo") }, "foo"},
+		{"AppendIntPositive", func() { buf.AppendInt(42) }, "42"},
+		{"AppendIntNegative", func() { buf.AppendInt(-42) }, "-42"},
+		{"AppendUint", func() { buf.AppendUint(42) }, "42"},
+		{"AppendBool", func() { buf.AppendBool(true) }, "true"},
+		{"AppendFloat64", func() { buf.AppendFloat(3.14, 64) }, "3.14"},
 		// Intentionally introduce some floating-point error.
-		{
-			desc: "AppendFloat32",
-			f:    func(buf *Buffer) { buf.AppendFloat(float64(float32(3.14)), 32) },
-			want: "3.14",
-		},
-		{
-			desc: "AppendWrite",
-			f:    func(buf *Buffer) { buf.Write([]byte("foo")) },
-			want: "foo",
-		},
-		{
-			desc: "AppendTime",
-			f:    func(buf *Buffer) { buf.AppendTime(time.Date(2000, 1, 2, 3, 4, 5, 6, time.UTC), time.RFC3339) },
-			want: "2000-01-02T03:04:05Z",
-		},
-		{
-			desc: "WriteByte",
-			f:    func(buf *Buffer) { buf.WriteByte('v') },
-			want: "v",
-		},
-		{
-			desc: "WriteString",
-			f:    func(buf *Buffer) { buf.WriteString("foo") },
-			want: "foo",
-		},
+		{"AppendFloat32", func() { buf.AppendFloat(float64(float32(3.14)), 32) }, "3.14"},
+		{"AppendWrite", func() { buf.Write([]byte("foo")) }, "foo"},
+		{"AppendTime", func() { buf.AppendTime(time.Date(2000, 1, 2, 3, 4, 5, 6, time.UTC), time.RFC3339) }, "2000-01-02T03:04:05Z"},
+		{"WriteByte", func() { buf.WriteByte('v') }, "v"},
+		{"WriteString", func() { buf.WriteString("foo") }, "foo"},
 	}
 
-	pool := NewPool()
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
-			buf := pool.Get()
-			defer buf.Free()
-
-			tt.f(buf)
+			buf.Reset()
+			tt.f()
 			assert.Equal(t, tt.want, buf.String(), "Unexpected buffer.String().")
 			assert.Equal(t, tt.want, string(buf.Bytes()), "Unexpected string(buffer.Bytes()).")
 			assert.Equal(t, len(tt.want), buf.Len(), "Unexpected buffer length.")

buffer/pool_test.go

@@ -28,8 +28,6 @@ import (
 )
 
 func TestBuffers(t *testing.T) {
-	t.Parallel()
-
 	const dummyData = "dummy data"
 	p := NewPool()
 

clock_test.go

@@ -37,8 +37,6 @@ func (c constantClock) NewTicker(d time.Duration) *time.Ticker {
 }
 
 func TestWithClock(t *testing.T) {
-	t.Parallel()
-
 	date := time.Date(2077, 1, 23, 10, 15, 13, 441, time.UTC)
 	clock := constantClock(date)
 	withLogger(t, DebugLevel, []Option{WithClock(clock)}, func(log *Logger, logs *observer.ObservedLogs) {

config_test.go

@@ -32,8 +32,6 @@ import (
 )
 
 func TestConfig(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		desc     string
 		cfg      Config
@@ -59,10 +57,7 @@ func TestConfig(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
 			logOut := filepath.Join(t.TempDir(), "test.log")
 
 			tt.cfg.OutputPaths = []string{logOut}
@@ -91,8 +86,6 @@ func TestConfig(t *testing.T) {
 }
 
 func TestConfigWithInvalidPaths(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		desc      string
 		output    string
@@ -104,10 +97,7 @@ func TestConfigWithInvalidPaths(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
 			cfg := NewProductionConfig()
 			cfg.OutputPaths = []string{tt.output}
 			cfg.ErrorOutputPaths = []string{tt.errOutput}
@@ -118,8 +108,6 @@ func TestConfigWithInvalidPaths(t *testing.T) {
 }
 
 func TestConfigWithMissingAttributes(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		desc      string
 		cfg       Config
@@ -147,10 +135,7 @@ func TestConfigWithMissingAttributes(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
 			cfg := tt.cfg
 			_, err := cfg.Build()
 			assert.EqualError(t, err, tt.expectErr)
@@ -174,8 +159,6 @@ func makeSamplerCountingHook() (h func(zapcore.Entry, zapcore.SamplingDecision),
 }
 
 func TestConfigWithSamplingHook(t *testing.T) {
-	t.Parallel()
-
 	shook, dcount, scount := makeSamplerCountingHook()
 	cfg := Config{
 		Level:       NewAtomicLevelAt(InfoLevel),

encoder_test.go

@@ -29,20 +29,16 @@ import (
 )
 
 func TestRegisterDefaultEncoders(t *testing.T) {
-	t.Parallel()
-
 	testEncodersRegistered(t, "console", "json")
 }
 
-//nolint:paralleltest // modifies global registry
 func TestRegisterEncoder(t *testing.T) {
 	testEncoders(func() {
 		assert.NoError(t, RegisterEncoder("foo", newNilEncoder), "expected to be able to register the encoder foo")
 		testEncodersRegistered(t, "foo")
 	})
 }
 
-//nolint:paralleltest // modifies global registry
 func TestDuplicateRegisterEncoder(t *testing.T) {
 	testEncoders(func() {
 		assert.NoError(t, RegisterEncoder("foo", newNilEncoder), "expected to be able to register the encoder foo")
@@ -51,12 +47,9 @@ func TestDuplicateRegisterEncoder(t *testing.T) {
 }
 
 func TestRegisterEncoderNoName(t *testing.T) {
-	t.Parallel()
-
 	assert.Equal(t, errNoEncoderNameSpecified, RegisterEncoder("", newNilEncoder), "expected an error when registering an encoder with no name")
 }
 
-//nolint:paralleltest // modifies global registry
 func TestNewEncoder(t *testing.T) {
 	testEncoders(func() {
 		assert.NoError(t, RegisterEncoder("foo", newNilEncoder), "expected to be able to register the encoder foo")
@@ -67,15 +60,11 @@ func TestNewEncoder(t *testing.T) {
 }
 
 func TestNewEncoderNotRegistered(t *testing.T) {
-	t.Parallel()
-
 	_, err := newEncoder("foo", zapcore.EncoderConfig{})
 	assert.Error(t, err, "expected an error when trying to create an encoder of an unregistered name")
 }
 
 func TestNewEncoderNoName(t *testing.T) {
-	t.Parallel()
-
 	_, err := newEncoder("", zapcore.EncoderConfig{})
 	assert.Equal(t, errNoEncoderNameSpecified, err, "expected an error when creating an encoder with no name")
 }

error_test.go

@@ -32,8 +32,6 @@ import (
 )
 
 func TestErrorConstructors(t *testing.T) {
-	t.Parallel()
-
 	fail := errors.New("fail")
 
 	tests := []struct {
@@ -50,21 +48,14 @@ func TestErrorConstructors(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			if !assert.Equal(t, tt.expect, tt.field, "Unexpected output from convenience field constructor %s.", tt.name) {
-				t.Logf("type expected: %T\nGot: %T", tt.expect.Interface, tt.field.Interface)
-			}
-			assertCanBeReused(t, tt.field)
-		})
+		if !assert.Equal(t, tt.expect, tt.field, "Unexpected output from convenience field constructor %s.", tt.name) {
+			t.Logf("type expected: %T\nGot: %T", tt.expect.Interface, tt.field.Interface)
+		}
+		assertCanBeReused(t, tt.field)
 	}
 }
 
 func TestErrorArrayConstructor(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		desc     string
 		field    Field
@@ -79,22 +70,15 @@ func TestErrorArrayConstructor(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.desc, func(t *testing.T) {
-			t.Parallel()
-
-			enc := zapcore.NewMapObjectEncoder()
-			tt.field.Key = "k"
-			tt.field.AddTo(enc)
-			assert.Equal(t, tt.expected, enc.Fields["k"], "%s: unexpected map contents.", tt.desc)
-			assert.Equal(t, 1, len(enc.Fields), "%s: found extra keys in map: %v", tt.desc, enc.Fields)
-		})
+		enc := zapcore.NewMapObjectEncoder()
+		tt.field.Key = "k"
+		tt.field.AddTo(enc)
+		assert.Equal(t, tt.expected, enc.Fields["k"], "%s: unexpected map contents.", tt.desc)
+		assert.Equal(t, 1, len(enc.Fields), "%s: found extra keys in map: %v", tt.desc, enc.Fields)
 	}
 }
 
 func TestErrorsArraysHandleRichErrors(t *testing.T) {
-	t.Parallel()
-
 	errs := []error{fmt.Errorf("egad")}
 
 	enc := zapcore.NewMapObjectEncoder()

exp/zapfield/zapfield_test.go

@@ -36,8 +36,6 @@ type (
 )
 
 func TestFieldConstructors(t *testing.T) {
-	t.Parallel()
-
 	var (
 		key    = MyKey("test key")
 		value  = MyValue("test value")
@@ -57,15 +55,10 @@ func TestFieldConstructors(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			if !assert.Equal(t, tt.expect, tt.field, "Unexpected output from convenience field constructor %s.", tt.name) {
-				t.Logf("type expected: %T\nGot: %T", tt.expect.Interface, tt.field.Interface)
-			}
-			assertCanBeReused(t, tt.field)
-		})
+		if !assert.Equal(t, tt.expect, tt.field, "Unexpected output from convenience field constructor %s.", tt.name) {
+			t.Logf("type expected: %T\nGot: %T", tt.expect.Interface, tt.field.Interface)
+		}
+		assertCanBeReused(t, tt.field)
 	}
 }