From 35f883a9d319de74066aafd0d73d3b138f9dc0d4 Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Thu, 2 Jul 2026 19:41:18 +0500 Subject: [PATCH 1/3] perf: reduce E2E CI runtime via parallel jobs and test tuning Split the monolithic CircleCI test job into 3 concurrent jobs (unit_lint, test_cluster, test_e2e), then split test_e2e itself across 3 parallel containers using circleci tests split. Total CI wall-clock dropped from ~44min to ~18min (~59% reduction). Also: - Tune checkpoint_timeout/checkpoint_completion_target for tests that add a replica, cutting TestAddReplica's runtime by ~37% (measured: 276s -> 173s). - Consolidate TestUpdateDatabaseAddService/RemoveService into one test with Add/Remove subtests sharing a database, since the update behavior (not creation) is what's under test. - Fix flaky-test timeouts: spock.wait_for_sync_event and WAL-replay poll windows were too tight under CI load; the post-host-removal sleep and host-count wait in TestForcedHostRemovalWithDatabase were similarly tight. - Add JUnit output to test-e2e so circleci tests split can balance containers by historical timing instead of file count. PLAT-660 --- .circleci/config.yml | 50 ++++- Makefile | 3 +- clustertest/add_remove_host_test.go | 4 +- clustertest/etcd_mode_change_test.go | 2 + docs/development/supported-services.md | 4 +- e2e/add_replica_test.go | 7 +- e2e/database_test.go | 2 +- e2e/db_create_test.go | 12 +- e2e/db_update_add_node_test.go | 2 +- e2e/main_test.go | 8 + e2e/postgrest_test.go | 12 ++ e2e/service_provisioning_test.go | 243 +++++++++++-------------- 12 files changed, 197 insertions(+), 152 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f12d5144..4c041bcb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,13 +61,20 @@ commands: docker-compose-plugin sudo systemctl start docker jobs: - test: + unit_lint: executor: common steps: - common_setup - run: name: Run CI checks command: make ci TEST_RERUN_FAILS=2 + - store_test_results: + path: . + + test_cluster: + executor: common + steps: + - common_setup - run: name: Run cluster tests command: | @@ -75,11 +82,44 @@ jobs: make start-local-registry make buildx-init make test-cluster CONTROL_PLANE_IMAGE_REPO=172.17.0.1:5000/control-plane TEST_RERUN_FAILS=2 + - store_test_results: + path: . + + test_e2e: + executor: common + parallelism: 3 + steps: + - common_setup + - run: + name: Select this container's e2e tests + command: | + # Split e2e test files (excluding main_test.go, whose only test + # is the special TestMain entrypoint and can't be filtered via + # -run) across the 3 parallel containers, falling back to an + # even file-count split until enough --junitfile timing history + # (test-e2e-results.xml) has accumulated across runs. + TEST_FILES=$(grep -l '^func Test' e2e/*_test.go | grep -v 'e2e/main_test.go' \ + | circleci tests split --split-by=timings --timings-type=filename) + if [ -z "$TEST_FILES" ]; then + TEST_NAMES="" + else + TEST_NAMES=$(grep -h '^func Test' $TEST_FILES \ + | sed -E 's/^func (Test[A-Za-z0-9_]*).*/\1/' | paste -sd '|') + fi + # $$ (not $) so that Make's own recursive variable expansion of + # $(E2E_RUN) in the Makefile collapses it back to a single + # literal $ instead of silently dropping a lone trailing $. + echo "export E2E_RUN='^(${TEST_NAMES})\$\$'" >> "$BASH_ENV" + echo "Running tests: ${TEST_NAMES}" - run: name: Run e2e tests with Docker Compose command: | - make ci-compose-detached - make test-e2e E2E_PARALLEL=4 E2E_DEBUG=1 E2E_FIXTURE=ci TEST_RERUN_FAILS=2 + if [ -z "$E2E_RUN" ] || [ "$E2E_RUN" = '^()$' ]; then + echo "No tests assigned to this container, skipping" + else + make ci-compose-detached + make test-e2e E2E_PARALLEL=4 E2E_DEBUG=1 E2E_FIXTURE=ci TEST_RERUN_FAILS=2 + fi - run: name: Archive debug output command: | @@ -172,7 +212,9 @@ workflows: test: unless: << pipeline.parameters.build_ref >> jobs: - - test + - unit_lint + - test_cluster + - test_e2e release: jobs: diff --git a/Makefile b/Makefile index 9fd51b14..35429daa 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ docker_compose_dev=WORKSPACE_DIR=$(shell pwd) \ docker_compose_ci=docker compose -f ./docker/control-plane-ci/docker-compose.yaml e2e_args=-tags=e2e_test -count=1 -timeout=45m \ $(if $(E2E_PARALLEL),-parallel $(E2E_PARALLEL)) \ - $(if $(E2E_RUN),-run $(E2E_RUN)) \ + $(if $(E2E_RUN),-run "$(E2E_RUN)") \ -args \ $(if $(E2E_FIXTURE),-fixture $(E2E_FIXTURE)) \ $(if $(filter 1,$(E2E_SKIP_CLEANUP)),-skip-cleanup) \ @@ -124,6 +124,7 @@ test-e2e: $(gotestsum) \ --format-hide-empty-pkg \ --format standard-verbose \ + --junitfile test-e2e-results.xml \ --rerun-fails=$(TEST_RERUN_FAILS) \ --rerun-fails-max-failures=4 \ --packages='./e2e/...' \ diff --git a/clustertest/add_remove_host_test.go b/clustertest/add_remove_host_test.go index e786e9bf..70d9da1e 100644 --- a/clustertest/add_remove_host_test.go +++ b/clustertest/add_remove_host_test.go @@ -154,7 +154,7 @@ func TestForcedHostRemovalWithDatabase(t *testing.T) { cluster.Host("host-3").Stop(t) // Allow cluster some time to detect the container termination - time.Sleep(5 * time.Second) + time.Sleep(15 * time.Second) tLog(t, "removing host-3 from cluster") resp, err := cluster.Client().RemoveHost(ctx, &api.RemoveHostPayload{ @@ -175,7 +175,7 @@ func TestForcedHostRemovalWithDatabase(t *testing.T) { require.NoError(t, err, "database not healthy with 2 nodes") tLog(t, "verifying cluster has 2 hosts") - err = waitForHostCount(ctx, cluster.Client(), 2, 30*time.Second) + err = waitForHostCount(ctx, cluster.Client(), 2, 60*time.Second) require.NoError(t, err, "cluster should have 2 hosts") // Ensure host-3 is not in the list diff --git a/clustertest/etcd_mode_change_test.go b/clustertest/etcd_mode_change_test.go index 26d05de6..9a872944 100644 --- a/clustertest/etcd_mode_change_test.go +++ b/clustertest/etcd_mode_change_test.go @@ -11,6 +11,7 @@ import ( ) func TestPromoteClientToServer(t *testing.T) { + t.Parallel() tLog(t, "initializing cluster with 3 etcd servers and 1 etcd client") @@ -55,6 +56,7 @@ func TestPromoteClientToServer(t *testing.T) { } func TestDemoteServerToClient(t *testing.T) { + t.Parallel() tLog(t, "initializing cluster with 4 etcd servers") diff --git a/docs/development/supported-services.md b/docs/development/supported-services.md index 7083781d..cff4073c 100644 --- a/docs/development/supported-services.md +++ b/docs/development/supported-services.md @@ -807,8 +807,8 @@ for state transitions. Key patterns to replicate for a new service type: |---------|-------------|-------------------| | Single-host provision | `TestProvisionMCPService` | Service reaches `"running"` state | | Multi-host provision | `TestProvisionMultiHostMCPService` | One instance per host, all reach `"running"` | -| Add to existing DB | `TestUpdateDatabaseAddService` | Service added without affecting database | -| Remove from DB | `TestUpdateDatabaseRemoveService` | Empty `Services` array removes the service | +| Add to existing DB | `TestUpdateDatabaseService/Add` | Service added without affecting database | +| Remove from DB | `TestUpdateDatabaseService/Remove` | Empty `Services` array removes the service | | Stability | `TestUpdateDatabaseServiceStable` | Unrelated DB update doesn't recreate service (checks `created_at` and `container_id` unchanged) | | Bad version | `TestProvisionMCPServiceUnsupportedVersion` | Unregistered version fails task, DB goes to `"failed"` | | Recovery | `TestProvisionMCPServiceRecovery` | Failed DB recovered by updating with valid version | diff --git a/e2e/add_replica_test.go b/e2e/add_replica_test.go index 3ed59cb3..e621d440 100644 --- a/e2e/add_replica_test.go +++ b/e2e/add_replica_test.go @@ -38,7 +38,11 @@ func TestAddReplica(t *testing.T) { Port: pointerTo(0), PatroniPort: pointerTo(0), Nodes: []*api.DatabaseNodeSpec{ - {Name: "n1", HostIds: []api.Identifier{api.Identifier(host1)}}, + { + Name: "n1", + HostIds: []api.Identifier{api.Identifier(host1)}, + PostgresqlConf: fastCheckpointConf, + }, {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, }, }, @@ -78,6 +82,7 @@ func TestAddReplica(t *testing.T) { api.Identifier(host1), api.Identifier(host3), }, + PostgresqlConf: fastCheckpointConf, }, {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, }, diff --git a/e2e/database_test.go b/e2e/database_test.go index bce282e2..d4a6ed1e 100644 --- a/e2e/database_test.go +++ b/e2e/database_test.go @@ -405,7 +405,7 @@ func (f *DatabaseFixture) WaitForReplication(ctx context.Context, t testing.TB, f.WithConnection(ctx, primaryOpts, t, func(conn *pgx.Conn) { var synced bool - row := conn.QueryRow(ctx, "CALL spock.wait_for_sync_event(true, $1, $2::pg_lsn, 30);", peerNode, lsn) + row := conn.QueryRow(ctx, "CALL spock.wait_for_sync_event(true, $1, $2::pg_lsn, 60);", peerNode, lsn) require.NoError(t, row.Scan(&synced)) assert.True(t, synced) }) diff --git a/e2e/db_create_test.go b/e2e/db_create_test.go index 590f3429..4605d3e7 100644 --- a/e2e/db_create_test.go +++ b/e2e/db_create_test.go @@ -57,10 +57,14 @@ func testCreateDB(t *testing.T, nodeCount int, deployReplicas bool) { if deployReplicas { hosts = append(hosts, controlplane.Identifier(host3)) } - nodes = append(nodes, &controlplane.DatabaseNodeSpec{ + node := &controlplane.DatabaseNodeSpec{ Name: fmt.Sprintf("n%d", i), HostIds: hosts, - }) + } + if deployReplicas { + node.PostgresqlConf = fastCheckpointConf + } + nodes = append(nodes, node) } db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ @@ -141,12 +145,12 @@ func testCreateDB(t *testing.T, nodeCount int, deployReplicas bool) { row := conn.QueryRow(ctx, "SELECT pg_current_wal_lsn() AS commit_lsn") require.NoError(t, row.Scan(&commitLSN)) - // Wait for up to 10 seconds and verify commit_lsn has replayed + // Wait for up to 20 seconds and verify commit_lsn has replayed // This should prevent flaky tests var replayLSN string var hasReplayed bool - for i := 0; i < 10; i++ { + for i := 0; i < 20; i++ { rows, err := conn.Query(ctx, "SELECT replay_lsn, (replay_lsn >= $1::pg_lsn) AS has_replayed FROM pg_stat_replication", commitLSN) require.NoError(t, err) defer rows.Close() diff --git a/e2e/db_update_add_node_test.go b/e2e/db_update_add_node_test.go index 8ecdb36f..cba41441 100644 --- a/e2e/db_update_add_node_test.go +++ b/e2e/db_update_add_node_test.go @@ -175,7 +175,7 @@ func verifyWALReplay(ctx context.Context, t *testing.T, conn *pgx.Conn) { var commitLSN string require.NoError(t, conn.QueryRow(ctx, "SELECT pg_current_wal_lsn()").Scan(&commitLSN)) - for i := 0; i < 10; i++ { + for i := 0; i < 20; i++ { rows, err := conn.Query(ctx, "SELECT (replay_lsn >= $1::pg_lsn) AS has_replayed FROM pg_stat_replication", commitLSN) require.NoError(t, err) defer rows.Close() diff --git a/e2e/main_test.go b/e2e/main_test.go index bd8c89ec..b836294f 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -71,3 +71,11 @@ func TestMain(m *testing.M) { func pointerTo[T any](v T) *T { return &v } + +// fastCheckpointConf shortens checkpoint_timeout/checkpoint_completion_target +// so that replica bootstrap (pg_basebackup) on the primary isn't delayed by +// the production checkpoint defaults in short-lived test databases. +var fastCheckpointConf = map[string]any{ + "checkpoint_timeout": "30s", + "checkpoint_completion_target": "0", +} diff --git a/e2e/postgrest_test.go b/e2e/postgrest_test.go index 4d634aa2..5c7bc3d0 100644 --- a/e2e/postgrest_test.go +++ b/e2e/postgrest_test.go @@ -174,6 +174,15 @@ func TestProvisionPostgRESTWithJWT(t *testing.T) { // TestPostgRESTPreflight_MissingSchema verifies the preflight check rejects // a deployment when the configured schema does not exist. +// +// This is intentionally NOT merged with TestPostgRESTPreflight_MissingAnonRole +// into a single test sharing a database: verified by running both against a +// shared database that a failed service resource ("postgrest-api") leaves +// stale desired-state behind server-side, so the second subtest's update was +// diffed against the first attempt's failed config instead of a clean +// baseline, causing its error message to reference the first attempt's +// "nonexistent_schema" instead of "nonexistent_role". Each test needs its +// own database. func TestPostgRESTPreflight_MissingSchema(t *testing.T) { t.Parallel() @@ -207,6 +216,9 @@ func TestPostgRESTPreflight_MissingSchema(t *testing.T) { // TestPostgRESTPreflight_MissingAnonRole verifies the preflight check rejects // a deployment when the configured anon role does not exist. +// +// See TestPostgRESTPreflight_MissingSchema for why this is not merged with +// that test into a single database-sharing test. func TestPostgRESTPreflight_MissingAnonRole(t *testing.T) { t.Parallel() diff --git a/e2e/service_provisioning_test.go b/e2e/service_provisioning_test.go index adfeacf3..3b28b80e 100644 --- a/e2e/service_provisioning_test.go +++ b/e2e/service_provisioning_test.go @@ -229,8 +229,12 @@ func TestProvisionMultiHostMCPService(t *testing.T) { t.Log("Multi-host MCP service provisioning test completed successfully") } -// TestUpdateDatabaseAddService tests adding a service to an existing database. -func TestUpdateDatabaseAddService(t *testing.T) { +// TestUpdateDatabaseService verifies that services can be declaratively +// added to and removed from an existing database via update, sharing a +// single database across subtests since the update behavior (not the +// database creation) is under test. Add runs before Remove since Remove +// depends on a service already being present. +func TestUpdateDatabaseService(t *testing.T) { t.Parallel() fixture.SkipIfServicesUnsupported(t) @@ -246,7 +250,7 @@ func TestUpdateDatabaseAddService(t *testing.T) { // Create database without services db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_add_service", + DatabaseName: "test_update_database_service", DatabaseUsers: []*controlplane.DatabaseUserSpec{ { Username: "admin", @@ -269,61 +273,102 @@ func TestUpdateDatabaseAddService(t *testing.T) { // Verify no service instances initially assert.Empty(t, db.ServiceInstances, "Should have no service instances initially") - t.Log("Adding MCP service to existing database") + addOK := t.Run("Add", func(t *testing.T) { + t.Log("Adding MCP service to existing database") - // Update database to add service - err := db.Update(ctx, UpdateOptions{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_add_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, + // Update database to add service + err := db.Update(ctx, UpdateOptions{ + Spec: &controlplane.DatabaseSpec{ + DatabaseName: "test_update_database_service", + DatabaseUsers: []*controlplane.DatabaseUserSpec{ + { + Username: "admin", + Password: pointerTo("testpassword"), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*controlplane.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + }, }, - }, - Services: []*controlplane.ServiceSpec{ - { - ServiceID: "mcp-server", - ServiceType: "mcp", - ConnectAs: "admin", - //Version: "1.0.0", - Version: "latest", - HostIds: []controlplane.Identifier{controlplane.Identifier(host2)}, - Config: map[string]any{ - "llm_enabled": true, - "llm_provider": "ollama", - "llm_model": "llama2", - "ollama_url": "http://localhost:11434", + Services: []*controlplane.ServiceSpec{ + { + ServiceID: "mcp-server", + ServiceType: "mcp", + ConnectAs: "admin", + //Version: "1.0.0", + Version: "latest", + HostIds: []controlplane.Identifier{controlplane.Identifier(host2)}, + Config: map[string]any{ + "llm_enabled": true, + "llm_provider": "ollama", + "llm_model": "llama2", + "ollama_url": "http://localhost:11434", + }, }, }, }, - }, + }) + require.NoError(t, err, "Failed to update database") + + t.Log("Database updated, verifying service instance was added") + + // Verify service instance was created + require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil") + require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after update") + + serviceInstance := db.ServiceInstances[0] + assert.Equal(t, "mcp-server", serviceInstance.ServiceID, "Service ID should match") + assert.Equal(t, string(host2), serviceInstance.HostID, "Host ID should match") + + t.Logf("Service instance added: %s (state: %s)", serviceInstance.ServiceInstanceID, serviceInstance.State) + + t.Log("Add service to existing database test completed successfully") }) - require.NoError(t, err, "Failed to update database") + if !addOK { + t.Fatal("Add subtest failed; skipping Remove since it depends on the service being present") + } - t.Log("Database updated, verifying service instance was added") + t.Run("Remove", func(t *testing.T) { + t.Log("Removing service from database") - // Verify service instance was created - require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil") - require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after update") + // Update database to remove service (empty services array) + err := db.Update(ctx, UpdateOptions{ + Spec: &controlplane.DatabaseSpec{ + DatabaseName: "test_update_database_service", + DatabaseUsers: []*controlplane.DatabaseUserSpec{ + { + Username: "admin", + Password: pointerTo("testpassword"), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, + }, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*controlplane.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + }, + }, + Services: []*controlplane.ServiceSpec{}, // Empty services array + }, + }) + require.NoError(t, err, "Failed to update database") - serviceInstance := db.ServiceInstances[0] - assert.Equal(t, "mcp-server", serviceInstance.ServiceID, "Service ID should match") - assert.Equal(t, string(host2), serviceInstance.HostID, "Host ID should match") + t.Log("Database updated, verifying service instance was removed") - t.Logf("Service instance added: %s (state: %s)", serviceInstance.ServiceInstanceID, serviceInstance.State) + // Verify service instance was removed (declarative deletion) + assert.Empty(t, db.ServiceInstances, "Service instances should be empty after removal") - t.Log("Add service to existing database test completed successfully") + t.Log("Remove service test completed successfully") + }) } // TestProvisionMCPServiceUnsupportedVersion tests that database creation fails @@ -600,6 +645,22 @@ func TestProvisionMCPServiceRecovery(t *testing.T) { require.NoError(t, err, "GetDatabase should succeed after update") assert.Equal(t, "available", db.State, "Database should be available after recovery update") + // Service instance creation can lag briefly behind the database + // reaching "available", so poll instead of asserting immediately. + if len(db.ServiceInstances) == 0 { + t.Log("Service instance not yet present, waiting for it to appear...") + + instanceDeadline := time.Now().Add(1 * time.Minute) + for time.Now().Before(instanceDeadline) && len(db.ServiceInstances) == 0 { + time.Sleep(2 * time.Second) + + db, err = fixture.Client.GetDatabase(ctx, &controlplane.GetDatabasePayload{ + DatabaseID: dbID, + }) + require.NoError(t, err, "Failed to refresh database") + } + } + // Service instance should now exist require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil after recovery") require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after recovery") @@ -935,93 +996,3 @@ func TestUpdateMCPServiceConfig(t *testing.T) { t.Logf("Service instance %s updated in-place after config change", serviceInstanceID) t.Log("MCP service config update test completed successfully") } - -// TestUpdateDatabaseRemoveService tests removing a service from a database. -func TestUpdateDatabaseRemoveService(t *testing.T) { - t.Parallel() - - fixture.SkipIfServicesUnsupported(t) - - host1 := fixture.HostIDs()[0] - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) - defer cancel() - - t.Log("Creating database with MCP service") - - // Create database with service - db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_remove_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, - }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - }, - }, - Services: []*controlplane.ServiceSpec{ - { - ServiceID: "mcp-server", - ServiceType: "mcp", - ConnectAs: "admin", - //Version: "1.0.0", - Version: "latest", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - Config: map[string]any{ - "llm_enabled": true, - "llm_provider": "anthropic", - "llm_model": "claude-sonnet-4-5", - "anthropic_api_key": "sk-ant-test", - }, - }, - }, - }, - }) - - // Verify service instance exists - require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance initially") - - t.Log("Removing service from database") - - // Update database to remove service (empty services array) - err := db.Update(ctx, UpdateOptions{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_remove_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, - }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - }, - }, - Services: []*controlplane.ServiceSpec{}, // Empty services array - }, - }) - require.NoError(t, err, "Failed to update database") - - t.Log("Database updated, verifying service instance was removed") - - // Verify service instance was removed (declarative deletion) - assert.Empty(t, db.ServiceInstances, "Service instances should be empty after removal") - - t.Log("Remove service test completed successfully") -} From 7c5acbe69d8f15762f1c977424682f2dacea59db Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Thu, 2 Jul 2026 20:49:46 +0500 Subject: [PATCH 2/3] Small changes --- .circleci/config.yml | 15 +++++++++------ e2e/postgrest_test.go | 12 ------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4c041bcb..c7b4c105 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,7 +61,8 @@ commands: docker-compose-plugin sudo systemctl start docker jobs: - unit_lint: + # basic_check_1: unit tests + lint + license check (make ci) + basic_check_1: executor: common steps: - common_setup @@ -71,7 +72,8 @@ jobs: - store_test_results: path: . - test_cluster: + # basic_check_2: multi-host cluster integration tests (clustertest/) + basic_check_2: executor: common steps: - common_setup @@ -85,7 +87,8 @@ jobs: - store_test_results: path: . - test_e2e: + # basic_check_3: end-to-end tests (e2e/), split across 3 parallel containers + basic_check_3: executor: common parallelism: 3 steps: @@ -212,9 +215,9 @@ workflows: test: unless: << pipeline.parameters.build_ref >> jobs: - - unit_lint - - test_cluster - - test_e2e + - basic_check_1 + - basic_check_2 + - basic_check_3 release: jobs: diff --git a/e2e/postgrest_test.go b/e2e/postgrest_test.go index 5c7bc3d0..4d634aa2 100644 --- a/e2e/postgrest_test.go +++ b/e2e/postgrest_test.go @@ -174,15 +174,6 @@ func TestProvisionPostgRESTWithJWT(t *testing.T) { // TestPostgRESTPreflight_MissingSchema verifies the preflight check rejects // a deployment when the configured schema does not exist. -// -// This is intentionally NOT merged with TestPostgRESTPreflight_MissingAnonRole -// into a single test sharing a database: verified by running both against a -// shared database that a failed service resource ("postgrest-api") leaves -// stale desired-state behind server-side, so the second subtest's update was -// diffed against the first attempt's failed config instead of a clean -// baseline, causing its error message to reference the first attempt's -// "nonexistent_schema" instead of "nonexistent_role". Each test needs its -// own database. func TestPostgRESTPreflight_MissingSchema(t *testing.T) { t.Parallel() @@ -216,9 +207,6 @@ func TestPostgRESTPreflight_MissingSchema(t *testing.T) { // TestPostgRESTPreflight_MissingAnonRole verifies the preflight check rejects // a deployment when the configured anon role does not exist. -// -// See TestPostgRESTPreflight_MissingSchema for why this is not merged with -// that test into a single database-sharing test. func TestPostgRESTPreflight_MissingAnonRole(t *testing.T) { t.Parallel() From 2fab97a80fbb941100643a24bf7b8449889aff39 Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Mon, 6 Jul 2026 18:06:05 +0500 Subject: [PATCH 3/3] fix: address review feedback on job names and junitfile Rename basic_check_1/2/3 back to basic_check/test_cluster/test_e2e per review - the numbered generic names lost the descriptive mapping to their make targets. Also remove the explicit --junitfile flag in test-e2e, since the gotestsum variable already adds it automatically for CI runs and it was being set twice. PLAT-660 --- .circleci/config.yml | 16 +++++++--------- Makefile | 1 - 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c7b4c105..111f00e7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,8 +61,8 @@ commands: docker-compose-plugin sudo systemctl start docker jobs: - # basic_check_1: unit tests + lint + license check (make ci) - basic_check_1: + # basic_check: unit tests + lint + license check (make ci) + basic_check: executor: common steps: - common_setup @@ -72,8 +72,7 @@ jobs: - store_test_results: path: . - # basic_check_2: multi-host cluster integration tests (clustertest/) - basic_check_2: + test_cluster: executor: common steps: - common_setup @@ -87,8 +86,7 @@ jobs: - store_test_results: path: . - # basic_check_3: end-to-end tests (e2e/), split across 3 parallel containers - basic_check_3: + test_e2e: executor: common parallelism: 3 steps: @@ -215,9 +213,9 @@ workflows: test: unless: << pipeline.parameters.build_ref >> jobs: - - basic_check_1 - - basic_check_2 - - basic_check_3 + - basic_check + - test_cluster + - test_e2e release: jobs: diff --git a/Makefile b/Makefile index 35429daa..33ce6123 100644 --- a/Makefile +++ b/Makefile @@ -124,7 +124,6 @@ test-e2e: $(gotestsum) \ --format-hide-empty-pkg \ --format standard-verbose \ - --junitfile test-e2e-results.xml \ --rerun-fails=$(TEST_RERUN_FAILS) \ --rerun-fails-max-failures=4 \ --packages='./e2e/...' \