From c55edc2b81628737a99d4510c6a2252583ad66c0 Mon Sep 17 00:00:00 2001 From: Aris Tzoumas Date: Thu, 3 Nov 2022 18:14:25 +0200 Subject: [PATCH] chore: rsources flaky test - start services before all --- Makefile | 4 +- codecov.yml | 4 ++ services/rsources/handler_test.go | 75 ++++++++++++++++--------------- 3 files changed, 46 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index f4f11e9f25..84d83da769 100644 --- a/Makefile +++ b/Makefile @@ -14,10 +14,10 @@ test: install-tools test-run test-teardown test-run: ## Run all unit tests ifeq ($(filter 1,$(debug) $(RUNNER_DEBUG)),) $(eval TEST_CMD = SLOW=0 gotestsum --format pkgname-and-test-fails --) - $(eval TEST_OPTIONS = -p=1 -v -failfast -shuffle=on -coverprofile=profile.out -covermode=atomic -vet=all --timeout=15m) + $(eval TEST_OPTIONS = -p=1 -v -failfast -shuffle=on -coverprofile=profile.out -covermode=count -coverpkg=./... -vet=all --timeout=15m) else $(eval TEST_CMD = SLOW=0 go test) - $(eval TEST_OPTIONS = -p=1 -v -failfast -shuffle=on -coverprofile=profile.out -covermode=atomic -vet=all --timeout=15m) + $(eval TEST_OPTIONS = -p=1 -v -failfast -shuffle=on -coverprofile=profile.out -covermode=count -coverpkg=./... -vet=all --timeout=15m) endif ifdef package $(TEST_CMD) $(TEST_OPTIONS) $(package) && touch $(TESTFILE) || true diff --git a/codecov.yml b/codecov.yml index bfdc9877d9..e6eff6703e 100644 --- a/codecov.yml +++ b/codecov.yml @@ -6,3 +6,7 @@ coverage: patch: default: informational: true +ignore: + - "mocks" + - "proto" + - "cmd/devtool" diff --git a/services/rsources/handler_test.go b/services/rsources/handler_test.go index e6915cc155..d1190a771f 100644 --- a/services/rsources/handler_test.go +++ b/services/rsources/handler_test.go @@ -60,9 +60,7 @@ var _ = Describe("Using sources handler", Ordered, func() { }) AfterAll(func() { - if resource.resource != nil { - purgeResource(pool, resource.resource) - } + purgeResources(pool, resource.resource) }) It("should be able to add and get failed records", func() { @@ -503,7 +501,7 @@ var _ = Describe("Using sources handler", Ordered, func() { var err error pool, err = dockertest.NewPool("") Expect(err).NotTo(HaveOccurred()) - const networkId = "TestMultitenantSourcesHandler" + networkId := randomString() network, _ = pool.Client.NetworkInfo(networkId) if network == nil { network, err = pool.Client.CreateNetwork(docker.CreateNetworkOptions{Name: networkId}) @@ -532,26 +530,28 @@ var _ = Describe("Using sources handler", Ordered, func() { SharedConn: pgC.externalDSN, SubscriptionTargetConn: pgB.internalDSN, } - }) - AfterAll(func() { - if network != nil { - _ = pool.Client.RemoveNetwork(network.ID) - } - if pgA.resource != nil { - purgeResource(pool, pgA.resource, pgB.resource, pgC.resource) - } - }) - It("should be able to create two services", func() { // Start 2 JobServices // 1. js1 with local=pgA, remote=pgC // 2. js2 with local=pgB, remote=pgC - // Increment the same jobRunId from both services serviceA = createService(configA) serviceB = createService(configB) }) + AfterAll(func() { + purgeResources(pool, pgA.resource, pgB.resource, pgC.resource) + if network != nil { + _ = pool.Client.RemoveNetwork(network.ID) + } + }) + It("should be able to query both services for the same jobRunId and receive same stats", func() { + GinkgoT().Logf("configA: %+v", configA) + GinkgoT().Logf("serviceA: %+v", serviceA) + + GinkgoT().Logf("configB: %+v", configB) + GinkgoT().Logf("serviceB: %+v", serviceB) + // Status from both services should be same jobRunId := newJobRunId() statsA := Stats{ @@ -559,6 +559,7 @@ var _ = Describe("Using sources handler", Ordered, func() { Out: 4, Failed: 0, } + GinkgoT().Logf("incrementing serviceA") increment(pgA.db, jobRunId, defaultJobTargetKey, statsA, serviceA, nil) statsB := Stats{ @@ -566,6 +567,7 @@ var _ = Describe("Using sources handler", Ordered, func() { Out: 2, Failed: 1, } + GinkgoT().Logf("incrementing serviceB") increment(pgB.db, jobRunId, defaultJobTargetKey, statsB, serviceB, nil) Eventually(func() bool { totalStatsA, err := serviceA.GetStatus(context.Background(), jobRunId, JobFilter{ @@ -672,7 +674,7 @@ var _ = Describe("Using sources handler", Ordered, func() { It("shouldn't be able to create a service when wal_level=logical is not set on the local db", func() { pgD := newDBResource(pool, network.ID, "postgres-4") - defer purgeResource(pool, pgD.resource) + defer purgeResources(pool, pgD.resource) badConfig := JobServiceConfig{ LocalHostname: "postgres-4", MaxPoolSize: 1, @@ -686,7 +688,7 @@ var _ = Describe("Using sources handler", Ordered, func() { It("shouldn't be able to create a service when an invalid SubscriptionTargetConn is provided", func() { pgD := newDBResource(pool, network.ID, "postgres-4") - defer purgeResource(pool, pgD.resource) + defer purgeResources(pool, pgD.resource) badConfig := JobServiceConfig{ LocalHostname: "postgres-4", MaxPoolSize: 1, @@ -699,7 +701,7 @@ var _ = Describe("Using sources handler", Ordered, func() { }) }) - Context("monitoring lag when shared db is configured", func() { + Context("monitoring lag when shared db is configured", Ordered, func() { var ( pool *dockertest.Pool network *docker.Network @@ -712,7 +714,7 @@ var _ = Describe("Using sources handler", Ordered, func() { var err error pool, err = dockertest.NewPool("") Expect(err).NotTo(HaveOccurred()) - const networkId = "TestMultitenantSourcesHandler" + networkId := randomString() network, _ = pool.Client.NetworkInfo(networkId) if network == nil { network, err = pool.Client.CreateNetwork(docker.CreateNetworkOptions{Name: networkId}) @@ -736,12 +738,10 @@ var _ = Describe("Using sources handler", Ordered, func() { }) AfterAll(func() { + purgeResources(pool, pgA.resource, pgB.resource) if network != nil { _ = pool.Client.RemoveNetwork(network.ID) } - if pgA.resource != nil { - purgeResource(pool, pgA.resource, pgB.resource) - } }) It("should be able to monitor lag when shared db is configured", func() { @@ -779,7 +779,7 @@ var _ = Describe("Using sources handler", Ordered, func() { It("should be able to add rsources_failed_keys table to the publication and subscription seamlessly", func() { pool, err := dockertest.NewPool("") Expect(err).NotTo(HaveOccurred()) - const networkId = "TestMultitenantSourcesHandlerMigration" + networkId := randomString() network, _ := pool.Client.NetworkInfo(networkId) if network == nil { network, err = pool.Client.CreateNetwork(docker.CreateNetworkOptions{Name: networkId}) @@ -794,12 +794,10 @@ var _ = Describe("Using sources handler", Ordered, func() { pgC := newDBResource(pool, network.ID, "postgres-mig-3") defer func() { + purgeResources(pool, pgA.resource, pgB.resource, pgC.resource) if network != nil { _ = pool.Client.RemoveNetwork(network.ID) } - if pgA.resource != nil { - purgeResource(pool, pgA.resource, pgB.resource, pgC.resource) - } }() configA := JobServiceConfig{ @@ -929,26 +927,27 @@ func (g *mockGauge) wasGauged() bool { func createService(config JobServiceConfig) JobService { service, err := NewJobService(config) - Expect(err).NotTo(HaveOccurred(), "it should be able to create the service") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to create the service") return service } func addFailedRecords(db *sql.DB, jobRunId string, jobTargetKey JobTargetKey, sh JobService, records []json.RawMessage) { tx, err := db.Begin() - Expect(err).NotTo(HaveOccurred(), "it should be able to begin the transaction") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to begin the transaction") err = sh.AddFailedRecords(context.Background(), tx, jobRunId, jobTargetKey, records) - Expect(err).NotTo(HaveOccurred(), "it should be able to add failed records") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to add failed records") err = tx.Commit() - Expect(err).NotTo(HaveOccurred(), "it should be able to commit the transaction") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to commit the transaction") } func increment(db *sql.DB, jobRunId string, key JobTargetKey, stat Stats, sh JobService, wg *sync.WaitGroup) { + GinkgoT().Logf("Incrementing using service %+v, db: %+v", sh, db) tx, err := db.Begin() - Expect(err).NotTo(HaveOccurred(), "it should be able to begin the transaction") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to begin the transaction") err = sh.IncrementStats(context.Background(), tx, jobRunId, key, stat) - Expect(err).NotTo(HaveOccurred(), "it should be able to increment stats") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to increment stats") err = tx.Commit() - Expect(err).NotTo(HaveOccurred(), "it should be able to commit the transaction") + Expect(err).ShouldNot(HaveOccurred(), "it should be able to commit the transaction") if wg != nil { wg.Done() } @@ -1007,9 +1006,11 @@ func newDBResource(pool *dockertest.Pool, networkId, hostname string, params ... } } -func purgeResource(pool *dockertest.Pool, resources ...*dockertest.Resource) { +func purgeResources(pool *dockertest.Pool, resources ...*dockertest.Resource) { for _, resource := range resources { - _ = pool.Purge(resource) + if resource != nil { + _ = pool.Purge(resource) + } } } @@ -1019,3 +1020,7 @@ func getDB(conn string, maxOpenConns int) *sql.DB { Expect(err).NotTo(HaveOccurred()) return db } + +func randomString() string { + return strings.ReplaceAll(uuid.Must(uuid.NewV4()).String(), "-", "") +}