chl-wxp commented on code in PR #11212:
URL: https://github.com/apache/seatunnel/pull/11212#discussion_r3502698619
##########
docs/en/developer/test-coding-guide.md:
##########
@@ -0,0 +1,539 @@
+---
+title: Test Coding Guide
+---
+
+# Test Coding Guide
+
+This guide describes how to write a high-quality, stable End-to-End (E2E) or
Unit Test for Apache SeaTunnel.
+It complements the [Coding Guide](coding-guide.md): the Coding Guide covers
general PR quality, while this
+guide focuses specifically on test stability and resource safety.
+
+A good SeaTunnel test is deterministic (the same result on every run),
leak-free (it releases every
+resource it opens), and cheap (it starts the fewest containers possible). The
guidelines below apply these
+principles to the two kinds of tests SeaTunnel contributors write.
+
+SeaTunnel distinguishes two test layers:
+
+- **Unit tests** (`*Test`, run by the Surefire plugin) validate connector or
core logic in isolation,
+ without external systems. See [Unit Test Guidelines](#unit-test-guidelines).
+- **End-to-End (E2E) tests** (`*IT`, run by the Failsafe plugin) validate
source, transform, and sink
+ behavior against real services in Testcontainers. See [E2E Test
Guidelines](#e2e-test-guidelines).
+
+When a pull request changes both logic and integration behavior, add tests in
both layers and run both
+before submitting.
+
+## Unit Test Guidelines
+
+### 1. Test behavior and contract, not implementation details
+
+A unit test should verify input-output behavior and option contracts. Avoid
testing private
+method internals or temporary implementation structure.
+
+Real example from `JdbcSourceFactoryTest` (module: `connector-jdbc`):
+
+```java
+private Map<String, Object> baseConfig() {
+ Map<String, Object> cfg = new HashMap<>();
+ cfg.put("url", "jdbc:mysql://localhost:3306/test");
+ cfg.put("driver", "com.mysql.cj.jdbc.Driver");
+ return cfg;
+}
+
+@Test
+void testValidConfigWithTablePath() {
+ Map<String, Object> cfg = baseConfig();
+ cfg.put("table_path", "test.users");
+ Assertions.assertDoesNotThrow(() -> validate(cfg));
+}
+```
+
+This test validates the external configuration contract and remains stable
even if internal factory
+implementation changes.
+
+### 2. Keep unit tests deterministic and local
+
+Unit tests should run fast and deterministically:
+
+- no `Thread.sleep`
+- no random assertions without fixed seed
+- no dependency on wall-clock timing
+
+If a case introduces cross-process or external dependencies, clearly classify
it in the proper test
+layer and keep assertions deterministic and diagnosable.
+
+### 3. Isolate dependencies with mock, stub, or fake
+
+A unit test should isolate the target behavior from external IO. Use in-memory
or lightweight
+test doubles for collaborators when possible.
+
+Keep one assertion scope per test method, so failures point to a single
behavior.
+
+Real engine-side mock example from `JobInfoServiceNullSafetyTest` (module:
`seatunnel-engine-server`):
+
+```java
+@BeforeEach
+void setUp() {
+ nodeEngine = mock(NodeEngineImpl.class);
+ hazelcastInstance = mock(HazelcastInstance.class);
+ runningJobInfoMap = mock(IMap.class);
+ finishedJobStateMap = mock(IMap.class);
+ finishedJobMetricsMap = mock(IMap.class);
+ finishedJobVertexInfoMap = mock(IMap.class);
+
+ when(nodeEngine.getHazelcastInstance()).thenReturn(hazelcastInstance);
+
when(hazelcastInstance.getMap(Constant.IMAP_RUNNING_JOB_INFO)).thenReturn(runningJobInfoMap);
+
when(hazelcastInstance.getMap(Constant.IMAP_FINISHED_JOB_STATE)).thenReturn(finishedJobStateMap);
+ when(hazelcastInstance.getMap(Constant.IMAP_FINISHED_JOB_METRICS))
+ .thenReturn(finishedJobMetricsMap);
+ when(hazelcastInstance.getMap(Constant.IMAP_FINISHED_JOB_VERTEX_INFO))
+ .thenReturn(finishedJobVertexInfoMap);
+
+ jobInfoService = new JobInfoService(nodeEngine);
+}
+
+@Test
+void shouldReturnJobIdOnlyWhenFinishedMetricsIsMissing() {
+ when(runningJobInfoMap.get(jobId)).thenReturn(null);
+ when(finishedJobStateMap.get(jobId)).thenReturn(jobState);
+ when(finishedJobMetricsMap.get(jobId)).thenReturn(null);
+
+ JsonObject result = jobInfoService.getJobInfoJson(jobId);
+ Assertions.assertEquals(jobId.toString(),
result.getString(RestConstant.JOB_ID, null));
+}
+```
+
+This example illustrates three practices:
+
+- It mocks every external map dependency at the boundary instead of starting
the engine services.
+- It uses `when(...).thenReturn(...)` to shape a single business branch per
test.
+- Its assertions target the observable output (the result JSON contract), not
internal calls.
+
+### 4. Verify exception type and message for invalid input paths
+
+For negative paths, assert both exception class and core message, so the test
protects user-facing
+error quality.
+
+Real example from `MongodbIncrementalSourceFactoryTest` (module:
`connector-cdc-mongodb`):
+
+```java
+Assertions.assertThrows(
+ MongodbConnectorException.class,
+ () ->
+ MongodbSourceConfigProvider.newBuilder()
+ .startupOptions(
+ new StartupConfig(StartupMode.EARLIEST, null,
null, null)));
+```
+
+### 5. Use clear naming and Arrange-Act-Assert structure
+
+- Name classes as `*Test` and methods as explicit behavior statements.
+- Keep each test in Arrange-Act-Assert order.
+- Avoid large helper logic inside test methods; extract reusable setup into
focused helpers.
+
+## Unit Test Checklist
+
+- [ ] Class name follows `*Test`; method names describe behavior or error case
+- [ ] Unit tests keep deterministic execution and avoid non-essential runtime
dependencies
+- [ ] Assertions target behavior and contract, not internal implementation
details
+- [ ] Negative tests assert both exception type and key error message
+- [ ] Test data setup is minimal and reusable
+
+## E2E Test Guidelines
+
+An E2E test extends `TestSuiteBase`, runs the connector inside a
Testcontainers-managed engine container, and
+verifies the result of a real job. The six rules below keep such tests
deterministic, leak-free, and cheap.
+
+### 1. Use Dynamic Ports, Never Hardcode
+
+Testcontainers assigns a random host port at startup. Hardcoding a port causes
conflicts in CI, where the
+port may already be taken or multiple suites run in parallel.
+
+Resolve the host port from the container at runtime:
+
+```java
+// Bad — collides in CI
+String brokerUrl = "tcp://127.0.0.1:61616";
+
+// Good — let Testcontainers assign the host port
+String brokerUrl = "tcp://" + container.getHost() + ":" +
container.getMappedPort(61616);
+config.setPort(container.getFirstMappedPort()); // when only one port is
exposed
+```
+
+Apply the same rule to all external services:
+
+```java
+// Database
+String jdbcUrl = String.format(
+ "jdbc:mysql://%s:%d/%s",
+ mysqlContainer.getHost(), mysqlContainer.getMappedPort(3306),
DATABASE);
+
+// HTTP service
+String endpoint = String.format(
+ "http://%s:%d";, serviceContainer.getHost(),
serviceContainer.getMappedPort(8080));
+```
+
+:::caution Job config files are the exception
+
+The SeaTunnel job runs inside the Docker network, so its config (`.conf`) must
reference the container's
+network alias (`withNetworkAliases("activemq-host")`) and the internal port
(e.g. `61616`), not the
+mapped host port.
+
+:::
+
+### 2. Wait on Conditions, Never `Thread.sleep`
+
+`Thread.sleep` is non-deterministic: too short and the test becomes flaky, too
long and CI is slowed down. It
+also produces no useful error when the expected state never arrives. Use
+[Awaitility](https://github.com/awaitility/awaitility) to poll for the actual
condition instead.
+
+```java
+// Bad — flaky, wasteful, and silent on failure
+container.executeJob("/job.conf");
+Thread.sleep(30000);
+Assertions.assertIterableEquals(expected, query());
+
+// Good — returns as soon as the condition holds, fails with the last
assertion error
+Awaitility.await()
+ .atMost(60, TimeUnit.SECONDS)
+ .pollInterval(2, TimeUnit.SECONDS)
+ .pollDelay(Duration.ZERO)
+ .untilAsserted(() -> Assertions.assertIterableEquals(expected,
query()));
+```
+
+Choose the timeout from the scenario rather than picking a round number:
+
+| Scenario | `atMost` | `pollInterval` | Why
|
+|-----------------------------------|------------|----------------|--------------------------------------|
+| Container / client readiness | 2 min | 1 s | Image pull
+ service init is slow |
+| Job reaches `RUNNING` state | 1 min | 2 s | Scheduling
overhead |
+| Batch job result verified | 60 s | 2 s | Small
dataset completion |
+| Kafka / MQ message consumption | 30–60 s | 1 s | Consumer
group rebalance |
+| CDC / schema-change propagation | 60–120 s | 2–5 s | Binlog
latency + snapshot |
+
+When the client throws until the service is ready, add `.ignoreExceptions()`:
+
+```java
+Awaitility.given()
+ .ignoreExceptions()
+ .pollInterval(500, TimeUnit.MILLISECONDS)
+ .atMost(180, TimeUnit.SECONDS)
+ .untilAsserted(this::initProducer);
+```
+
+A reusable helper for the common "wait for job RUNNING" step:
+
+```java
+private void awaitJobRunning(TestContainer container, String jobId) {
+ Awaitility.await()
+ .pollInterval(2, TimeUnit.SECONDS)
+ .atMost(1, TimeUnit.MINUTES)
+ .untilAsserted(
+ () -> Assertions.assertEquals("RUNNING",
container.getJobStatus(jobId)));
+}
+```
+
+### 3. Release Resources Promptly
+
+A leaked connection exhausts container resources and available host ports, and
can cause CI to hang. Every
+resource you open must be closed. Most E2E tests implement the `TestResource`
interface and override its
+`tearDown()` method, which runs once after all tests in the class. Close
resources there in reverse order of
+creation, with a null check on each (the test may have failed before the
resource was created), and stop any
+manually-started container explicitly.
+
+```java
+@AfterAll
+@Override
+public void tearDown() throws Exception {
+ if (producer != null) {
+ producer.close();
+ }
+ if (session != null) {
+ session.close();
+ }
+ if (connection != null) {
+ connection.close();
+ }
+ if (container != null) {
+ container.stop();
+ }
+}
+```
+
+For resources scoped to a single method, prefer try-with-resources over manual
close:
+
+```java
+private void executeDml(String sql) {
+ try (Connection conn = getJdbcConnection();
+ Statement stmt = conn.createStatement()) {
+ stmt.execute(sql);
+ } catch (SQLException e) {
+ throw new RuntimeException("Execute DML failed: " + sql, e);
+ }
+}
+```
+
+Cleanup checklist:
+
+- [ ] JDBC connections / statements closed
+- [ ] Message broker connections, sessions, producers, consumers closed
+- [ ] Custom clients (HTTP, gRPC) shut down
+- [ ] `ExecutorService` shut down with a timeout
+- [ ] Async `CompletableFuture` jobs cancelled
+- [ ] Temporary files / directories deleted
+- [ ] Manually-created Docker networks removed
+
+### 4. Submit Long-Running Jobs Asynchronously
+
+A streaming or CDC job runs until it is cancelled, so calling `executeJob`
inline blocks the test thread
+indefinitely and leaves no opportunity to inject data or assert intermediate
state. Submit such jobs with
+`CompletableFuture.supplyAsync`, wait until the job reaches `RUNNING`, run the
test steps, verify the result,
+then cancel the job.
+
+```java
+String jobId = "streaming-cdc-job";
+
+CompletableFuture<Void> job = CompletableFuture.supplyAsync(
+ () -> {
+ try {
+ container.executeJob("/streaming_job.conf", jobId);
+ } catch (Exception e) {
+ log.error("Job execution failed", e);
+ throw new CompletionException(e); // propagate, never swallow
+ }
+ return null;
+ });
+
+awaitJobRunning(container, jobId); // wait for RUNNING before proceeding
+
+insertCdcData(); // run test steps while the job is running
+
+Awaitility.await()
+ .atMost(60, TimeUnit.SECONDS)
+ .pollInterval(2, TimeUnit.SECONDS)
+ .untilAsserted(() -> verifySinkResults());
+
+job.cancel(true); // cancel the job
Review Comment:
```suggestion
container.cancelJob(jobId); // cancel the job
```
--
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]
