From 072920f5083c3a6ab6c379b29b202b736dc41935 Mon Sep 17 00:00:00 2001 From: kumarkavish Date: Fri, 23 Feb 2024 22:07:05 +0530 Subject: [PATCH 1/3] - fixing flaky tests, yay! --- dependency/catalog_node_test.go | 64 +------------- dependency/catalog_nodes_test.go | 130 ++++++++++++++++++++++++++++- dependency/catalog_service_test.go | 4 +- dependency/dependency_test.go | 9 +- dependency/health_service_test.go | 8 +- dependency/kv_get_test.go | 2 +- dependency/kv_keys_test.go | 12 +-- dependency/kv_list_test.go | 12 +-- test/tenancy_helper.go | 4 +- 9 files changed, 155 insertions(+), 90 deletions(-) diff --git a/dependency/catalog_node_test.go b/dependency/catalog_node_test.go index 75b8e0f89..49da440a8 100644 --- a/dependency/catalog_node_test.go +++ b/dependency/catalog_node_test.go @@ -136,7 +136,7 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { cases := tenancyHelper.GenerateNonDefaultTenancyTests(func(tenancy *test.Tenancy) []interface{} { return []interface{}{ testCase{ - tenancyHelper.AppendTenancyInfo("local", tenancy), + tenancyHelper.AppendTenancyInfo("local", tenancy), // the agent has nothing registered, we use fake node name for registration "", &CatalogNode{ Node: &Node{ @@ -152,20 +152,6 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { }, }, Services: []*CatalogNodeService{ - { - ID: "conn-enabled-service-default-default", - Service: "conn-enabled-service-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - Port: 12345, - }, - { - ID: "conn-enabled-service-proxy-default-default", - Service: "conn-enabled-service-proxy-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - Port: 21999, - }, { ID: "consul", Service: "consul", @@ -173,29 +159,15 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { Tags: ServiceTags([]string{}), Meta: map[string]string{}, }, - { - ID: "service-meta-default-default", - Service: "service-meta-default-default", - Tags: ServiceTags([]string{"tag1"}), - Meta: map[string]string{ - "meta1": "value1", - }, - }, - { - ID: "service-taggedAddresses-default-default", - Service: "service-taggedAddresses-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - }, }, }, }, testCase{ tenancyHelper.AppendTenancyInfo("partition and ns", tenancy), - fmt.Sprintf("%s?partition=%s&ns=%s", testConsul.Config.NodeName, tenancy.Partition, tenancy.Namespace), + fmt.Sprintf("%s?partition=%s&ns=%s", "node"+tenancy.Partition, tenancy.Partition, tenancy.Namespace), &CatalogNode{ Node: &Node{ - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, Address: testConsul.Config.Bind, Datacenter: "dc1", TaggedAddresses: map[string]string{ @@ -250,7 +222,7 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { cases = append(cases, tenancyHelper.GenerateDefaultTenancyTests(func(tenancy *test.Tenancy) []interface{} { return []interface{}{ testCase{ - tenancyHelper.AppendTenancyInfo("local", tenancy), + tenancyHelper.AppendTenancyInfo("local", tenancy), // the agent has nothing registered, we use fake node name for registration "", &CatalogNode{ Node: &Node{ @@ -266,20 +238,6 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { }, }, Services: []*CatalogNodeService{ - { - ID: "conn-enabled-service-default-default", - Service: "conn-enabled-service-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - Port: 12345, - }, - { - ID: "conn-enabled-service-proxy-default-default", - Service: "conn-enabled-service-proxy-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - Port: 21999, - }, { ID: "consul", Service: "consul", @@ -287,20 +245,6 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) { Tags: ServiceTags([]string{}), Meta: map[string]string{}, }, - { - ID: "service-meta-default-default", - Service: "service-meta-default-default", - Tags: ServiceTags([]string{"tag1"}), - Meta: map[string]string{ - "meta1": "value1", - }, - }, - { - ID: "service-taggedAddresses-default-default", - Service: "service-taggedAddresses-default-default", - Tags: ServiceTags([]string{}), - Meta: map[string]string{}, - }, }, }, }, diff --git a/dependency/catalog_nodes_test.go b/dependency/catalog_nodes_test.go index e8b14ac94..0827fab42 100644 --- a/dependency/catalog_nodes_test.go +++ b/dependency/catalog_nodes_test.go @@ -162,6 +162,18 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { //"consul-network-segment": "", }, }, + { + Node: "nodedefault", + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, }, }, testCase{ @@ -169,7 +181,7 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { fmt.Sprintf("?partition=%s&ns=%s@dc1", tenancy.Partition, tenancy.Namespace), []*Node{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, Address: testConsul.Config.Bind, Datacenter: "dc1", TaggedAddresses: map[string]string{ @@ -198,6 +210,18 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { //"consul-network-segment": "", }, }, + { + Node: "nodedefault", + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, }, }, testCase{ @@ -205,7 +229,7 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { fmt.Sprintf("?partition=%s@dc1", tenancy.Partition), []*Node{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, Address: testConsul.Config.Bind, Datacenter: "dc1", TaggedAddresses: map[string]string{ @@ -239,6 +263,108 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { //"consul-network-segment": "", }, }, + { + Node: "node" + tenancy.Partition, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + }, + }, + testCase{ + tenancyHelper.AppendTenancyInfo("partition and namespace", tenancy), + fmt.Sprintf("?partition=%s&ns=%s@dc1", tenancy.Partition, tenancy.Namespace), + []*Node{ + { + Node: testConsul.Config.NodeName, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + { + Node: "node" + tenancy.Partition, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + }, + }, + testCase{ + tenancyHelper.AppendTenancyInfo("namespace", tenancy), + fmt.Sprintf("?ns=%s@dc1", tenancy.Namespace), + []*Node{ + { + Node: testConsul.Config.NodeName, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + { + Node: "nodedefault", + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + }, + }, + testCase{ + tenancyHelper.AppendTenancyInfo("partition", tenancy), + fmt.Sprintf("?partition=%s@dc1", tenancy.Partition), + []*Node{ + { + Node: testConsul.Config.NodeName, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, + { + Node: "node" + tenancy.Partition, + Address: testConsul.Config.Bind, + Datacenter: "dc1", + TaggedAddresses: map[string]string{ + //"lan": "127.0.0.1", + //"wan": "127.0.0.1", + }, + Meta: map[string]string{ + //"consul-network-segment": "", + }, + }, }, }, } diff --git a/dependency/catalog_service_test.go b/dependency/catalog_service_test.go index 4f3d3c668..98a11c6a9 100644 --- a/dependency/catalog_service_test.go +++ b/dependency/catalog_service_test.go @@ -220,7 +220,7 @@ func TestCatalogServiceQuery_Fetch(t *testing.T) { "service-meta-default-default", []*CatalogService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, Address: testConsul.Config.Bind, Datacenter: "dc1", TaggedAddresses: map[string]string{ @@ -253,7 +253,7 @@ func TestCatalogServiceQuery_Fetch(t *testing.T) { fmt.Sprintf("service-meta-%s-%s?ns=%s&partition=%s", tenancy.Partition, tenancy.Namespace, tenancy.Namespace, tenancy.Partition), []*CatalogService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, Address: testConsul.Config.Bind, Datacenter: "dc1", TaggedAddresses: map[string]string{ diff --git a/dependency/dependency_test.go b/dependency/dependency_test.go index 08ff2a2d4..68a69c180 100644 --- a/dependency/dependency_test.go +++ b/dependency/dependency_test.go @@ -139,12 +139,6 @@ func TestMain(m *testing.M) { func (c *ClientSet) createConsulTestResources() error { catalog := testClients.Consul().Catalog() - - node, err := testClients.Consul().Agent().NodeName() - if err != nil { - return err - } - for _, tenancy := range tenancyHelper.TestTenancies() { partition := "" namespace := "" @@ -152,6 +146,7 @@ func (c *ClientSet) createConsulTestResources() error { partition = tenancy.Partition namespace = tenancy.Namespace } + node := "node" + tenancy.Partition // service with meta data serviceMetaService := &api.AgentService{ ID: fmt.Sprintf("service-meta-%s-%s", tenancy.Partition, tenancy.Namespace), @@ -241,6 +236,7 @@ func (c *ClientSet) createConsulTestResources() error { if err := testClients.createConsulPeerings(tenancy); err != nil { return err } + time.Sleep(200 * time.Millisecond) } return nil @@ -562,7 +558,6 @@ func (c *ClientSet) createConsulPartitions() error { return nil } - func (c *ClientSet) createConsulNs() error { for _, tenancy := range tenancyHelper.TestTenancies() { if tenancy.Namespace != "" && tenancy.Namespace != "default" { diff --git a/dependency/health_service_test.go b/dependency/health_service_test.go index a0b54d678..a5d1bd02d 100644 --- a/dependency/health_service_test.go +++ b/dependency/health_service_test.go @@ -448,7 +448,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) { "service-meta-default-default", []*HealthService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, NodeAddress: testConsul.Config.Bind, NodeTaggedAddresses: map[string]string{ //"lan": "127.0.0.1", @@ -477,7 +477,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) { "service-taggedAddresses-default-default", []*HealthService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, NodeAddress: testConsul.Config.Bind, NodeTaggedAddresses: map[string]string{ //"lan": "127.0.0.1", @@ -580,7 +580,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) { fmt.Sprintf("service-meta-%s-%s?partition=%s&ns=%s", tenancy.Partition, tenancy.Namespace, tenancy.Partition, tenancy.Namespace), []*HealthService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, NodeAddress: testConsul.Config.Bind, NodeTaggedAddresses: map[string]string{ //"lan": "127.0.0.1", @@ -609,7 +609,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) { fmt.Sprintf("service-taggedAddresses-%s-%s?partition=%s&ns=%s", tenancy.Partition, tenancy.Namespace, tenancy.Partition, tenancy.Namespace), []*HealthService{ { - Node: testConsul.Config.NodeName, + Node: "node" + tenancy.Partition, NodeAddress: testConsul.Config.Bind, NodeTaggedAddresses: map[string]string{ //"lan": "127.0.0.1", diff --git a/dependency/kv_get_test.go b/dependency/kv_get_test.go index 114e4e877..88dc763ca 100644 --- a/dependency/kv_get_test.go +++ b/dependency/kv_get_test.go @@ -243,7 +243,7 @@ func TestKVGetQuery_Fetch(t *testing.T) { testCase{ tenancyHelper.AppendTenancyInfo("exists", tenancy), "test-kv-get/key", - fmt.Sprintf("value-%s-%s", tenancy.Partition, tenancy.Namespace), + "value-default-default", }, testCase{ tenancyHelper.AppendTenancyInfo("exists_empty_string", tenancy), diff --git a/dependency/kv_keys_test.go b/dependency/kv_keys_test.go index 6f6174724..1c4410977 100644 --- a/dependency/kv_keys_test.go +++ b/dependency/kv_keys_test.go @@ -272,18 +272,18 @@ func TestKVKeysQuery_Fetch(t *testing.T) { tenancyHelper.AppendTenancyInfo("exists", tenancy), "test-kv-keys/prefix", []string{ - fmt.Sprintf("foo-%s-%s", tenancy.Partition, tenancy.Namespace), - fmt.Sprintf("wave/ocean-%s-%s", tenancy.Partition, tenancy.Namespace), - fmt.Sprintf("zip-%s-%s", tenancy.Partition, tenancy.Namespace), + "foo-default-default", + "wave/ocean-default-default", + "zip-default-default", }, }, testCase{ tenancyHelper.AppendTenancyInfo("trailing", tenancy), "test-kv-keys/prefix/", []string{ - fmt.Sprintf("foo-%s-%s", tenancy.Partition, tenancy.Namespace), - fmt.Sprintf("wave/ocean-%s-%s", tenancy.Partition, tenancy.Namespace), - fmt.Sprintf("zip-%s-%s", tenancy.Partition, tenancy.Namespace), + "foo-default-default", + "wave/ocean-default-default", + "zip-default-default", }, }, testCase{ diff --git a/dependency/kv_list_test.go b/dependency/kv_list_test.go index 7d6501a2e..743b8965a 100644 --- a/dependency/kv_list_test.go +++ b/dependency/kv_list_test.go @@ -299,17 +299,17 @@ func TestKVListQuery_Fetch(t *testing.T) { { Path: "test-kv-list/prefix/foo", Key: "foo", - Value: fmt.Sprintf("bar-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "bar-default-default", }, { Path: "test-kv-list/prefix/wave/ocean", Key: "wave/ocean", - Value: fmt.Sprintf("sleek-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "sleek-default-default", }, { Path: "test-kv-list/prefix/zip", Key: "zip", - Value: fmt.Sprintf("zap-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "zap-default-default", }, }, }, @@ -320,17 +320,17 @@ func TestKVListQuery_Fetch(t *testing.T) { { Path: "test-kv-list/prefix/foo", Key: "foo", - Value: fmt.Sprintf("bar-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "bar-default-default", }, { Path: "test-kv-list/prefix/wave/ocean", Key: "wave/ocean", - Value: fmt.Sprintf("sleek-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "sleek-default-default", }, { Path: "test-kv-list/prefix/zip", Key: "zip", - Value: fmt.Sprintf("zap-%s-%s", tenancy.Partition, tenancy.Namespace), + Value: "zap-default-default", }, }, }, diff --git a/test/tenancy_helper.go b/test/tenancy_helper.go index 056d26431..9c2143014 100644 --- a/test/tenancy_helper.go +++ b/test/tenancy_helper.go @@ -125,7 +125,7 @@ func (t *TenancyHelper) GenerateTenancyTests(generationFunc func(tenancy *Tenanc func (t *TenancyHelper) GenerateNonDefaultTenancyTests(generationFunc func(tenancy *Tenancy) []interface{}) []interface{} { cases := make([]interface{}, 0) for _, tenancy := range t.TestTenancies() { - if tenancy.Partition != "default" || tenancy.Namespace != "default" { + if tenancy.Partition != "default" { cases = append(cases, generationFunc(tenancy)...) } } @@ -135,7 +135,7 @@ func (t *TenancyHelper) GenerateNonDefaultTenancyTests(generationFunc func(tenan func (t *TenancyHelper) GenerateDefaultTenancyTests(generationFunc func(tenancy *Tenancy) []interface{}) []interface{} { cases := make([]interface{}, 0) for _, tenancy := range t.TestTenancies() { - if tenancy.Partition == "default" && tenancy.Namespace == "default" { + if tenancy.Partition == "default" { cases = append(cases, generationFunc(tenancy)...) } } From 4ab95bf8c5a27818560d9d45f5809da049473ad8 Mon Sep 17 00:00:00 2001 From: kumarkavish Date: Fri, 23 Feb 2024 22:10:08 +0530 Subject: [PATCH 2/3] - fixing flaky tests, yay! --- dependency/catalog_nodes_test.go | 90 -------------------------------- 1 file changed, 90 deletions(-) diff --git a/dependency/catalog_nodes_test.go b/dependency/catalog_nodes_test.go index 0827fab42..bc815925d 100644 --- a/dependency/catalog_nodes_test.go +++ b/dependency/catalog_nodes_test.go @@ -277,96 +277,6 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) { }, }, }, - testCase{ - tenancyHelper.AppendTenancyInfo("partition and namespace", tenancy), - fmt.Sprintf("?partition=%s&ns=%s@dc1", tenancy.Partition, tenancy.Namespace), - []*Node{ - { - Node: testConsul.Config.NodeName, - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - { - Node: "node" + tenancy.Partition, - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - }, - }, - testCase{ - tenancyHelper.AppendTenancyInfo("namespace", tenancy), - fmt.Sprintf("?ns=%s@dc1", tenancy.Namespace), - []*Node{ - { - Node: testConsul.Config.NodeName, - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - { - Node: "nodedefault", - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - }, - }, - testCase{ - tenancyHelper.AppendTenancyInfo("partition", tenancy), - fmt.Sprintf("?partition=%s@dc1", tenancy.Partition), - []*Node{ - { - Node: testConsul.Config.NodeName, - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - { - Node: "node" + tenancy.Partition, - Address: testConsul.Config.Bind, - Datacenter: "dc1", - TaggedAddresses: map[string]string{ - //"lan": "127.0.0.1", - //"wan": "127.0.0.1", - }, - Meta: map[string]string{ - //"consul-network-segment": "", - }, - }, - }, - }, } })...) From 0d772e41e594c8b6d644c641653271188142d565 Mon Sep 17 00:00:00 2001 From: kumarkavish Date: Fri, 23 Feb 2024 22:19:24 +0530 Subject: [PATCH 3/3] - fixing flaky tests, yay! --- test/tenancy_helper_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tenancy_helper_test.go b/test/tenancy_helper_test.go index 632611047..24a304b41 100644 --- a/test/tenancy_helper_test.go +++ b/test/tenancy_helper_test.go @@ -144,7 +144,7 @@ func TestGenerateDefaultTenancyTests(t *testing.T) { fakeTest{name: "CE2"}, } }}, - {name: "ENT", isConsulEnterprise: true, expectedTests: 2, input: func(tenancy *Tenancy) []interface{} { + {name: "ENT", isConsulEnterprise: true, expectedTests: 4, input: func(tenancy *Tenancy) []interface{} { return []interface{}{ fakeTest{name: "ENT1"}, fakeTest{name: "ENT2"}, @@ -179,7 +179,7 @@ func TestGenerateNonDefaultTenancyTests(t *testing.T) { fakeTest{name: "CE2"}, } }}, - {name: "ENT", isConsulEnterprise: true, expectedTests: 6, input: func(tenancy *Tenancy) []interface{} { + {name: "ENT", isConsulEnterprise: true, expectedTests: 4, input: func(tenancy *Tenancy) []interface{} { return []interface{}{ fakeTest{name: "ENT1"}, fakeTest{name: "ENT2"},