lintingbin opened a new pull request, #4201:
URL: https://github.com/apache/amoro/pull/4201
## What changes were proposed in this pull request?
`amoro-ams/src/test/java/org/apache/amoro/server/RestCatalogServiceTestBase`
and its subclasses (`TestInternalMixedCatalogService`,
`TestInternalIcebergCatalogService`) share a single static `AmsEnvironment` and
a hard-coded `database = \"test_ns\"` / `table = \"test_iceberg_tbl\"` across
every `@Nested` class. Whenever cleanup leaks (the existing `@AfterEach`
swallows `dropDatabase` exceptions) or background catalog scans race the tests,
sibling nested classes inherit dirty catalog state and assertions that
hard-code the catalog being empty start failing.
This PR makes the tests order-independent without restructuring the suite:
1. **Unique database name per test instance** —
`RestCatalogServiceTestBase.database` is now `\"test_ns_\" +
COUNTER.incrementAndGet()`. Each test method gets a fresh test instance (JUnit
5 default `Lifecycle.PER_METHOD`), so each test gets a fresh database name.
Pollution between tests can no longer cause `AlreadyExists` on `createDatabase`
or pre-populated catalog snapshots.
2. **Delta-based catalog/namespace assertions** in
`TestInternalMixedCatalogService.TestDatabaseOperation.test` and
`TestInternalIcebergCatalogService.NamespaceTests.testNamespaceOperations`. The
previous code asserted `catalog.listDatabases().isEmpty()` which only holds in
a single-test-class-per-JVM execution.
## Why are the changes needed?
These tests have been flaky on `master` for a while — for example, the
`Core/hadoop3 CI` run on 2026-04-23 failed inside
`TestInternalIcebergCatalogService\$CatalogPropertiesTest`'s suite, and PR
#4200's CI just hit the same family of failures, all surfacing the same root
cause:
```
TestInternalMixedCatalogService\$TestDatabaseOperation.test:168
expected: <true> but was: <false> // listDatabases().isEmpty()
failed because a previous test leaked test_ns
TestInternalMixedCatalogService\$TestTableOperation.before:186
AlreadyExists Database: test_ns already exists // previous @AfterEach
swallowed dropDatabase failure
TestInternalMixedCatalogService\$TestTableCommit.testTableCommit:295
NoSuchTableException: ...test_iceberg_tbl not exists // sibling test's
@AfterEach.dropTable raced into our just-created table
```
The shared `test_ns` / `test_iceberg_tbl` names are the load-bearing reason
these races collide. With unique per-instance database names, every test gets
its own namespace, so concurrent or leaked cleanup from a sibling test no
longer touches it.
This is complementary to #4185, which fixed a `Version-N-already-exists`
race in `InternalMixedIcebergHandler.newTableOperations` triggered by the same
shared-state pattern.
## How was this patch tested?
Ran the affected suites locally on Java 11:
```
./mvnw test -pl amoro-ams -am -Pspark-3.5,support-all-formats,openapi-sdk \\
-Dtest='TestInternalMixedCatalogService,TestInternalIcebergCatalogService'
# Tests run: 16, Failures: 0, Errors: 0, Skipped: 0
```
`./mvnw spotless:check checkstyle:check -pl amoro-ams -am` is clean.
## Does this PR introduce any user-facing change?
No — test-only change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
