From e60bb9f102d2a2a399cc22c9144ff030f3b526dc Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 8 May 2020 14:00:29 -0400 Subject: [PATCH] test: Remove t.Parallel() from agent/structs tests go test will only run tests in parallel within a single package. In this case the package test run time is exactly the same with or without t.Parallel() (~0.7s). In generally we should avoid t.Parallel() as it causes a number of problems with `go test` not reporting failure messages correctly. I encountered one of these problems, which is what prompted this change. Since `t.Parallel` is not providing any benefit in this package, this commit removes it. The change was automated with: git grep -l 't.Parallel' | xargs sed -i -e '/t.Parallel/d' --- agent/structs/acl_cache_test.go | 9 ------- agent/structs/acl_legacy_test.go | 5 ---- agent/structs/acl_test.go | 24 ------------------- agent/structs/check_definition_test.go | 3 --- .../config_entry_discoverychain_test.go | 4 ---- agent/structs/config_entry_gateways_test.go | 6 ----- agent/structs/config_entry_test.go | 3 --- agent/structs/intention_test.go | 1 - agent/structs/service_definition_test.go | 1 - agent/structs/structs_filtering_test.go | 1 - agent/structs/structs_test.go | 10 -------- 11 files changed, 67 deletions(-) diff --git a/agent/structs/acl_cache_test.go b/agent/structs/acl_cache_test.go index 337d1860f3..2d527b7aa9 100644 --- a/agent/structs/acl_cache_test.go +++ b/agent/structs/acl_cache_test.go @@ -8,13 +8,10 @@ import ( ) func TestStructs_ACLCaches(t *testing.T) { - t.Parallel() t.Run("New", func(t *testing.T) { - t.Parallel() t.Run("Valid Sizes", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{2, 2, 2, 2, 2} @@ -28,7 +25,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("Zero Sizes", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{0, 0, 0, 0, 0} @@ -43,7 +39,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("Identities", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{Identities: 4} @@ -58,7 +53,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("Policies", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{Policies: 4} @@ -73,7 +67,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("ParsedPolicies", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{ParsedPolicies: 4} @@ -88,7 +81,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("Authorizers", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{Authorizers: 4} @@ -104,7 +96,6 @@ func TestStructs_ACLCaches(t *testing.T) { }) t.Run("Roles", func(t *testing.T) { - t.Parallel() // 1 isn't valid due to a bug in golang-lru library config := ACLCachesConfig{Roles: 4} diff --git a/agent/structs/acl_legacy_test.go b/agent/structs/acl_legacy_test.go index 68660d8e3c..343a38050a 100644 --- a/agent/structs/acl_legacy_test.go +++ b/agent/structs/acl_legacy_test.go @@ -54,7 +54,6 @@ func TestStructs_ACL_IsSame(t *testing.T) { } func TestStructs_ACL_Convert(t *testing.T) { - t.Parallel() acl := &ACL{ ID: "guid", @@ -76,10 +75,8 @@ func TestStructs_ACL_Convert(t *testing.T) { } func TestStructs_ACLToken_Convert(t *testing.T) { - t.Parallel() t.Run("Management", func(t *testing.T) { - t.Parallel() token := &ACLToken{ AccessorID: "6c4eb178-c7f3-4620-b899-91eb8696c265", SecretID: "67c29ecd-cabc-42e0-a20e-771e9a1ab70c", @@ -101,7 +98,6 @@ func TestStructs_ACLToken_Convert(t *testing.T) { }) t.Run("Client", func(t *testing.T) { - t.Parallel() token := &ACLToken{ AccessorID: "6c4eb178-c7f3-4620-b899-91eb8696c265", SecretID: "67c29ecd-cabc-42e0-a20e-771e9a1ab70c", @@ -120,7 +116,6 @@ func TestStructs_ACLToken_Convert(t *testing.T) { }) t.Run("Unconvertible", func(t *testing.T) { - t.Parallel() token := &ACLToken{ AccessorID: "6c4eb178-c7f3-4620-b899-91eb8696c265", SecretID: "67c29ecd-cabc-42e0-a20e-771e9a1ab70c", diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go index 87fff974e1..1916af829e 100644 --- a/agent/structs/acl_test.go +++ b/agent/structs/acl_test.go @@ -11,10 +11,8 @@ import ( ) func TestStructs_ACLToken_PolicyIDs(t *testing.T) { - t.Parallel() t.Run("Basic", func(t *testing.T) { - t.Parallel() token := &ACLToken{ Policies: []ACLTokenPolicyLink{ @@ -38,7 +36,6 @@ func TestStructs_ACLToken_PolicyIDs(t *testing.T) { }) t.Run("Legacy Management", func(t *testing.T) { - t.Parallel() a := &ACL{ ID: "root", @@ -57,7 +54,6 @@ func TestStructs_ACLToken_PolicyIDs(t *testing.T) { }) t.Run("Legacy Management With Rules", func(t *testing.T) { - t.Parallel() a := &ACL{ ID: "root", @@ -77,7 +73,6 @@ func TestStructs_ACLToken_PolicyIDs(t *testing.T) { }) t.Run("No Policies", func(t *testing.T) { - t.Parallel() token := &ACLToken{} @@ -87,17 +82,14 @@ func TestStructs_ACLToken_PolicyIDs(t *testing.T) { } func TestStructs_ACLToken_EmbeddedPolicy(t *testing.T) { - t.Parallel() t.Run("No Rules", func(t *testing.T) { - t.Parallel() token := &ACLToken{} require.Nil(t, token.EmbeddedPolicy()) }) t.Run("Legacy Client", func(t *testing.T) { - t.Parallel() // None of the other fields should be considered token := &ACLToken{ @@ -116,7 +108,6 @@ func TestStructs_ACLToken_EmbeddedPolicy(t *testing.T) { }) t.Run("Same Policy for Tokens with same Rules", func(t *testing.T) { - t.Parallel() token1 := &ACLToken{ AccessorID: "f55b260c-5e05-418e-ab19-d421d1ab4b52", @@ -141,7 +132,6 @@ func TestStructs_ACLToken_EmbeddedPolicy(t *testing.T) { } func TestStructs_ACLServiceIdentity_SyntheticPolicy(t *testing.T) { - t.Parallel() cases := []struct { serviceName string @@ -183,7 +173,6 @@ func TestStructs_ACLServiceIdentity_SyntheticPolicy(t *testing.T) { } func TestStructs_ACLToken_SetHash(t *testing.T) { - t.Parallel() token := ACLToken{ AccessorID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", @@ -228,7 +217,6 @@ func TestStructs_ACLToken_SetHash(t *testing.T) { } func TestStructs_ACLToken_EstimateSize(t *testing.T) { - t.Parallel() // estimated size here should token := ACLToken{ @@ -254,10 +242,8 @@ func TestStructs_ACLToken_EstimateSize(t *testing.T) { } func TestStructs_ACLToken_Stub(t *testing.T) { - t.Parallel() t.Run("Basic", func(t *testing.T) { - t.Parallel() token := ACLToken{ AccessorID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", @@ -290,7 +276,6 @@ func TestStructs_ACLToken_Stub(t *testing.T) { }) t.Run("Legacy", func(t *testing.T) { - t.Parallel() token := ACLToken{ AccessorID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", SecretID: "65e98e67-9b29-470c-8ffa-7c5a23cc67c8", @@ -313,7 +298,6 @@ func TestStructs_ACLToken_Stub(t *testing.T) { } func TestStructs_ACLTokens_Sort(t *testing.T) { - t.Parallel() tokens := ACLTokens{ &ACLToken{ @@ -338,7 +322,6 @@ func TestStructs_ACLTokens_Sort(t *testing.T) { } func TestStructs_ACLTokenListStubs_Sort(t *testing.T) { - t.Parallel() tokens := ACLTokenListStubs{ &ACLTokenListStub{ @@ -363,7 +346,6 @@ func TestStructs_ACLTokenListStubs_Sort(t *testing.T) { } func TestStructs_ACLPolicy_Stub(t *testing.T) { - t.Parallel() policy := &ACLPolicy{ ID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", @@ -384,7 +366,6 @@ func TestStructs_ACLPolicy_Stub(t *testing.T) { } func TestStructs_ACLPolicy_SetHash(t *testing.T) { - t.Parallel() policy := &ACLPolicy{ ID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", @@ -419,7 +400,6 @@ func TestStructs_ACLPolicy_SetHash(t *testing.T) { } func TestStructs_ACLPolicy_EstimateSize(t *testing.T) { - t.Parallel() policy := ACLPolicy{ ID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", @@ -436,7 +416,6 @@ func TestStructs_ACLPolicy_EstimateSize(t *testing.T) { } func TestStructs_ACLPolicies_Sort(t *testing.T) { - t.Parallel() policies := ACLPolicies{ &ACLPolicy{ @@ -461,7 +440,6 @@ func TestStructs_ACLPolicies_Sort(t *testing.T) { } func TestStructs_ACLPolicyListStubs_Sort(t *testing.T) { - t.Parallel() policies := ACLPolicyListStubs{ &ACLPolicyListStub{ @@ -486,7 +464,6 @@ func TestStructs_ACLPolicyListStubs_Sort(t *testing.T) { } func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) { - t.Parallel() config := ACLCachesConfig{ Identities: 0, @@ -579,7 +556,6 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) { } func TestStructs_ACLPolicies_Compile(t *testing.T) { - t.Parallel() config := ACLCachesConfig{ Identities: 0, diff --git a/agent/structs/check_definition_test.go b/agent/structs/check_definition_test.go index 7120b34b8e..9622479d90 100644 --- a/agent/structs/check_definition_test.go +++ b/agent/structs/check_definition_test.go @@ -12,7 +12,6 @@ import ( ) func TestCheckDefinition_Defaults(t *testing.T) { - t.Parallel() def := CheckDefinition{} check := def.HealthCheck("node1") @@ -47,7 +46,6 @@ func mapFields(t *testing.T, obj interface{}) map[string]reflect.Value { } func TestCheckDefinition_CheckType(t *testing.T) { - t.Parallel() // Fuzz a definition to fill all its fields with data. var def CheckDefinition @@ -77,7 +75,6 @@ func TestCheckDefinition_CheckType(t *testing.T) { } func TestCheckDefinitionToCheckType(t *testing.T) { - t.Parallel() got := &CheckDefinition{ ID: "id", Name: "name", diff --git a/agent/structs/config_entry_discoverychain_test.go b/agent/structs/config_entry_discoverychain_test.go index d7afa75823..0695a21965 100644 --- a/agent/structs/config_entry_discoverychain_test.go +++ b/agent/structs/config_entry_discoverychain_test.go @@ -13,7 +13,6 @@ import ( func TestConfigEntries_ListRelatedServices_AndACLs(t *testing.T) { // This test tests both of these because they are related functions. - t.Parallel() newServiceACL := func(t *testing.T, canRead, canWrite []string) acl.Authorizer { var buf bytes.Buffer @@ -248,7 +247,6 @@ func TestConfigEntries_ListRelatedServices_AndACLs(t *testing.T) { } func TestServiceResolverConfigEntry(t *testing.T) { - t.Parallel() type testcase struct { name string @@ -538,7 +536,6 @@ func TestServiceResolverConfigEntry(t *testing.T) { } func TestServiceSplitterConfigEntry(t *testing.T) { - t.Parallel() makesplitter := func(splits ...ServiceSplit) *ServiceSplitterConfigEntry { return &ServiceSplitterConfigEntry{ @@ -723,7 +720,6 @@ func TestServiceSplitterConfigEntry(t *testing.T) { } func TestServiceRouterConfigEntry(t *testing.T) { - t.Parallel() httpMatch := func(http *ServiceRouteHTTPMatch) *ServiceRouteMatch { return &ServiceRouteMatch{HTTP: http} diff --git a/agent/structs/config_entry_gateways_test.go b/agent/structs/config_entry_gateways_test.go index e00ae0a2d7..e6179846d8 100644 --- a/agent/structs/config_entry_gateways_test.go +++ b/agent/structs/config_entry_gateways_test.go @@ -7,7 +7,6 @@ import ( ) func TestIngressConfigEntry_Normalize(t *testing.T) { - t.Parallel() cases := []struct { name string @@ -83,7 +82,6 @@ func TestIngressConfigEntry_Normalize(t *testing.T) { // tests in parallel. tc := test t.Run(tc.name, func(t *testing.T) { - t.Parallel() err := tc.entry.Normalize() require.NoError(t, err) require.Equal(t, tc.expected, tc.entry) @@ -92,7 +90,6 @@ func TestIngressConfigEntry_Normalize(t *testing.T) { } func TestIngressConfigEntry_Validate(t *testing.T) { - t.Parallel() cases := []struct { name string @@ -323,7 +320,6 @@ func TestIngressConfigEntry_Validate(t *testing.T) { // tests in parallel. tc := test t.Run(tc.name, func(t *testing.T) { - t.Parallel() err := tc.entry.Validate() if tc.expectErr != "" { require.Error(t, err) @@ -336,7 +332,6 @@ func TestIngressConfigEntry_Validate(t *testing.T) { } func TestTerminatingConfigEntry_Validate(t *testing.T) { - t.Parallel() cases := []struct { name string @@ -435,7 +430,6 @@ func TestTerminatingConfigEntry_Validate(t *testing.T) { // tests in parallel. tc := test t.Run(tc.name, func(t *testing.T) { - t.Parallel() err := tc.entry.Validate() if tc.expectErr != "" { diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 6e4f841317..310321ae9f 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -14,7 +14,6 @@ import ( // TestDecodeConfigEntry is the 'structs' mirror image of // command/config/write/config_write_test.go:TestParseConfigEntry func TestDecodeConfigEntry(t *testing.T) { - t.Parallel() for _, tc := range []struct { name string @@ -782,7 +781,6 @@ func TestServiceConfigResponse_MsgPack(t *testing.T) { } func TestConfigEntryResponseMarshalling(t *testing.T) { - t.Parallel() cases := map[string]ConfigEntryResponse{ "nil entry": ConfigEntryResponse{}, @@ -809,7 +807,6 @@ func TestConfigEntryResponseMarshalling(t *testing.T) { name := name tcase := tcase t.Run(name, func(t *testing.T) { - t.Parallel() data, err := tcase.MarshalBinary() require.NoError(t, err) diff --git a/agent/structs/intention_test.go b/agent/structs/intention_test.go index bae87c6e3b..fa0cbb75a6 100644 --- a/agent/structs/intention_test.go +++ b/agent/structs/intention_test.go @@ -11,7 +11,6 @@ import ( ) func TestIntention_ACLs(t *testing.T) { - t.Parallel() type testCase struct { intention Intention rules string diff --git a/agent/structs/service_definition_test.go b/agent/structs/service_definition_test.go index 81199f09c4..7e51e3dffa 100644 --- a/agent/structs/service_definition_test.go +++ b/agent/structs/service_definition_test.go @@ -11,7 +11,6 @@ import ( ) func TestAgentStructs_CheckTypes(t *testing.T) { - t.Parallel() svc := new(ServiceDefinition) // Singular Check field works diff --git a/agent/structs/structs_filtering_test.go b/agent/structs/structs_filtering_test.go index 63ecf0a14b..ad981b8bd2 100644 --- a/agent/structs/structs_filtering_test.go +++ b/agent/structs/structs_filtering_test.go @@ -733,7 +733,6 @@ func (d *fieldDumper) DumpFields(name string, fields bexpr.FieldConfigurations) } func TestStructs_FilterFieldConfigurations(t *testing.T) { - t.Parallel() var d *fieldDumper if *dumpFieldConfig { diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index 2917753e8f..54847b3c0c 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -1549,7 +1549,6 @@ func TestSpecificServiceRequest_CacheInfo(t *testing.T) { } func TestNodeService_JSON_OmitTaggedAdddresses(t *testing.T) { - t.Parallel() cases := []struct { name string ns NodeService @@ -1572,7 +1571,6 @@ func TestNodeService_JSON_OmitTaggedAdddresses(t *testing.T) { name := tc.name ns := tc.ns t.Run(name, func(t *testing.T) { - t.Parallel() data, err := json.Marshal(ns) require.NoError(t, err) var raw map[string]interface{} @@ -1585,7 +1583,6 @@ func TestNodeService_JSON_OmitTaggedAdddresses(t *testing.T) { } func TestServiceNode_JSON_OmitServiceTaggedAdddresses(t *testing.T) { - t.Parallel() cases := []struct { name string sn ServiceNode @@ -1608,7 +1605,6 @@ func TestServiceNode_JSON_OmitServiceTaggedAdddresses(t *testing.T) { name := tc.name sn := tc.sn t.Run(name, func(t *testing.T) { - t.Parallel() data, err := json.Marshal(sn) require.NoError(t, err) var raw map[string]interface{} @@ -1621,7 +1617,6 @@ func TestServiceNode_JSON_OmitServiceTaggedAdddresses(t *testing.T) { } func TestNode_BestAddress(t *testing.T) { - t.Parallel() type testCase struct { input Node @@ -1658,7 +1653,6 @@ func TestNode_BestAddress(t *testing.T) { name := name tc := tc t.Run(name, func(t *testing.T) { - t.Parallel() require.Equal(t, tc.lanAddr, tc.input.BestAddress(false)) require.Equal(t, tc.wanAddr, tc.input.BestAddress(true)) @@ -1667,7 +1661,6 @@ func TestNode_BestAddress(t *testing.T) { } func TestNodeService_BestAddress(t *testing.T) { - t.Parallel() type testCase struct { input NodeService @@ -1760,7 +1753,6 @@ func TestNodeService_BestAddress(t *testing.T) { name := name tc := tc t.Run(name, func(t *testing.T) { - t.Parallel() addr, port := tc.input.BestAddress(false) require.Equal(t, tc.lanAddr, addr) @@ -1774,7 +1766,6 @@ func TestNodeService_BestAddress(t *testing.T) { } func TestCheckServiceNode_BestAddress(t *testing.T) { - t.Parallel() type testCase struct { input CheckServiceNode @@ -1928,7 +1919,6 @@ func TestCheckServiceNode_BestAddress(t *testing.T) { name := name tc := tc t.Run(name, func(t *testing.T) { - t.Parallel() addr, port := tc.input.BestAddress(false) require.Equal(t, tc.lanAddr, addr)