You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "lintingbin (via GitHub)" <gi...@apache.org> on 2026/05/01 17:36:21 UTC
[PR] [ams][test] Make RestCatalogService nested tests order-independent [amoro]
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ams][test] Make RestCatalogService nested tests order-independent [amoro]
Posted by "lintingbin (via GitHub)" <gi...@apache.org>.
lintingbin commented on PR #4201:
URL: https://github.com/apache/amoro/pull/4201#issuecomment-4362605632
Hi @zhoujinsong, would you mind taking a look when you have time? Thanks!
This is test-only and is meant to fix the flaky `TestInternalMixedCatalogService` / `TestInternalIcebergCatalogService` failures that have been intermittently breaking CI on `master` and on #4200. Complementary to your #4185 — same test suite, but a different (test-isolation) flavor of the same shared-state pattern.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ams][test] Make RestCatalogService nested tests order-independent [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4201:
URL: https://github.com/apache/amoro/pull/4201#issuecomment-4665316298
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ams][test] Make RestCatalogService nested tests order-independent [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4201: [ams][test] Make RestCatalogService nested tests order-independent
URL: https://github.com/apache/amoro/pull/4201
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ams][test] Make RestCatalogService nested tests order-independent [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4201:
URL: https://github.com/apache/amoro/pull/4201#issuecomment-4597612405
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org