From 31111500ad721e3bc314e5c751b8e71aba93b81d Mon Sep 17 00:00:00 2001 From: lintingbin Date: Sat, 2 May 2026 01:35:45 +0800 Subject: [PATCH] [ams][test] Make RestCatalogService tests order-independent The integration tests under amoro-ams/src/test/java/org/apache/amoro/server/TestInternal*CatalogService share a static AmsEnvironment instance and a hard-coded "test_ns" database / "test_iceberg_tbl" table name across every @Nested class. Whenever cleanup leaks (the existing @AfterEach swallows dropDatabase exceptions) or background scans race the tests, sibling @Nested classes inherit dirty catalog state and assertions that hard-code the catalog being empty start failing. Concrete fixes: * Each test instance now picks a fresh database name via a static AtomicLong counter ("test_ns_"), so unrelated tests can never collide on the same namespace even if a previous run leaked state. * TestInternalMixedCatalogService.TestDatabaseOperation.test now asserts the create / drop delta against the initial database and namespace counts instead of asserting the catalog is empty. * TestInternalIcebergCatalogService.NamespaceTests.testNamespaceOperations has the same delta-based fix. This addresses the same pre-existing flakiness seen on master CI (see Core/hadoop3 run on 2026-04-23 failing in TestInternalIcebergCatalogService$CatalogPropertiesTest's suite) and the failures reproduced on PR #4200 CI in TestInternalMixedCatalogService$TestDatabaseOperation / TestTableOperation / TestTableCommit. --- .../server/RestCatalogServiceTestBase.java | 5 ++++- .../TestInternalIcebergCatalogService.java | 18 ++++++++++++------ .../TestInternalMixedCatalogService.java | 14 +++++++++----- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/amoro-ams/src/test/java/org/apache/amoro/server/RestCatalogServiceTestBase.java b/amoro-ams/src/test/java/org/apache/amoro/server/RestCatalogServiceTestBase.java index 5a3c4aa82d..9bf32f904f 100644 --- a/amoro-ams/src/test/java/org/apache/amoro/server/RestCatalogServiceTestBase.java +++ b/amoro-ams/src/test/java/org/apache/amoro/server/RestCatalogServiceTestBase.java @@ -43,13 +43,16 @@ import java.io.IOException; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; public abstract class RestCatalogServiceTestBase { static AmsEnvironment ams = AmsEnvironment.getIntegrationInstances(); static String restCatalogUri = RestCatalogService.ICEBERG_REST_API_PREFIX; - protected final String database = "test_ns"; + private static final AtomicLong TEST_INSTANCE_COUNTER = new AtomicLong(); + + protected final String database = "test_ns_" + TEST_INSTANCE_COUNTER.incrementAndGet(); protected final String table = "test_iceberg_tbl"; protected final TableIdentifier tableIdentifier = TableIdentifier.of(catalogName(), database, table); diff --git a/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalIcebergCatalogService.java b/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalIcebergCatalogService.java index 19d4ace8a7..3a5ea9e1c0 100644 --- a/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalIcebergCatalogService.java +++ b/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalIcebergCatalogService.java @@ -104,12 +104,18 @@ public void testCatalogProperties() { public class NamespaceTests { @Test public void testNamespaceOperations() throws IOException { - Assertions.assertTrue(nsCatalog.listNamespaces().isEmpty()); - nsCatalog.createNamespace(Namespace.of(database)); - Assertions.assertEquals(1, nsCatalog.listNamespaces().size()); - Assertions.assertEquals(0, nsCatalog.listNamespaces(Namespace.of(database)).size()); - nsCatalog.dropNamespace(Namespace.of(database)); - Assertions.assertTrue(nsCatalog.listNamespaces().isEmpty()); + Namespace dbNamespace = Namespace.of(database); + int initialNamespaceCount = nsCatalog.listNamespaces().size(); + Assertions.assertFalse(nsCatalog.listNamespaces().contains(dbNamespace)); + + nsCatalog.createNamespace(dbNamespace); + Assertions.assertEquals(initialNamespaceCount + 1, nsCatalog.listNamespaces().size()); + Assertions.assertTrue(nsCatalog.listNamespaces().contains(dbNamespace)); + Assertions.assertEquals(0, nsCatalog.listNamespaces(dbNamespace).size()); + + nsCatalog.dropNamespace(dbNamespace); + Assertions.assertEquals(initialNamespaceCount, nsCatalog.listNamespaces().size()); + Assertions.assertFalse(nsCatalog.listNamespaces().contains(dbNamespace)); } } diff --git a/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalMixedCatalogService.java b/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalMixedCatalogService.java index 3e24e42b61..65e8d387bd 100644 --- a/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalMixedCatalogService.java +++ b/amoro-ams/src/test/java/org/apache/amoro/server/TestInternalMixedCatalogService.java @@ -165,16 +165,20 @@ public void test() { MixedFormatCatalog catalog = loadMixedIcebergCatalog(); Assertions.assertEquals( InternalMixedIcebergCatalog.class.getName(), catalog.getClass().getName()); - Assertions.assertTrue(catalog.listDatabases().isEmpty()); + int initialDatabaseCount = catalog.listDatabases().size(); + int initialNamespaceCount = nsCatalog.listNamespaces(Namespace.of()).size(); + Assertions.assertFalse(catalog.listDatabases().contains(database)); catalog.createDatabase(database); - Assertions.assertEquals(1, catalog.listDatabases().size()); + Assertions.assertEquals(initialDatabaseCount + 1, catalog.listDatabases().size()); Assertions.assertTrue(catalog.listDatabases().contains(database)); - Assertions.assertEquals(1, nsCatalog.listNamespaces(Namespace.of()).size()); + Assertions.assertEquals( + initialNamespaceCount + 1, nsCatalog.listNamespaces(Namespace.of()).size()); catalog.dropDatabase(database); - Assertions.assertTrue(catalog.listDatabases().isEmpty()); - Assertions.assertTrue(nsCatalog.listNamespaces().isEmpty()); + Assertions.assertEquals(initialDatabaseCount, catalog.listDatabases().size()); + Assertions.assertFalse(catalog.listDatabases().contains(database)); + Assertions.assertEquals(initialNamespaceCount, nsCatalog.listNamespaces().size()); } }