This is an automated email from the ASF dual-hosted git repository. JackieTien97 pushed a commit to branch ty/it-authoring-guidelines in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit e0e46272b0679266544c968be60feb136953cd06 Author: JackieTien97 <[email protected]> AuthorDate: Fri Oct 30 11:18:02 2026 +0800 Document IT discovery and reliability guidelines --- integration-test/AGENTS.md | 81 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/integration-test/AGENTS.md b/integration-test/AGENTS.md index 25bb23fe836..ed67a6b613f 100644 --- a/integration-test/AGENTS.md +++ b/integration-test/AGENTS.md @@ -23,6 +23,27 @@ These instructions apply to `integration-test/` and supplement the repository-level instructions. +## Test discovery and categories + +- Name concrete IT classes to match Failsafe's default includes: `IT*`, `*IT`, or `*ITCase`. + Prefer the existing `IoTDB...IT` convention; put variant numbers before the suffix, such as + `IoTDBActiveRegionScan2IT`, not `IoTDBActiveRegionScanIT2`. An `IoTDB` prefix or a `Test` suffix + alone does not make a class discoverable. +- Every test method must have an effective JUnit `@Category`, declared on the method, its class, + or a superclass. All test profiles filter by category; `@Test` and `@RunWith` alone are not + sufficient. In a class using method-level categories without a class/superclass category, + annotate every new `@Test` method, even when neighboring methods already have categories. +- Prefer a class-level category when all cases share the same requirements. Preserve method-level + categories when cases need different profiles; adding a broad class category can enable cases + in unintended environments. Inherited categories are valid and need not be copied to subclasses. +- Select categories by the actual data model and topology, using the profile mapping below and + `pom.xml`. Shared packages such as `session/it` contain both tree and table tests; the package + name alone does not determine the category. Check the corresponding CI workflow for specialized + categories such as `DailyIT` and multi-cluster tests. +- Follow the neighboring tests' runner and environment lifecycle conventions. Ordinary ITs use + `@RunWith(IoTDBTestRunner.class)`; retain inherited or specialized runners for parameterized + fixtures such as `AbstractSchemaIT` instead of replacing them. + ## Credentials and deployment defaults - Use `EnvFactory.getEnv()` connection helpers or client builders with their default credentials @@ -53,6 +74,10 @@ These instructions apply to `integration-test/` and supplement the repository-le - Use bounded condition waits, such as Awaitility or environment status helpers, for startup, discovery, leader election, and recovery. Prefer observable conditions over fixed sleeps. +- When no read-only condition exposes the state being awaited, use a finite wait derived from the + running server's effective configuration and the required propagation rounds, with a documented + safety margin. Reuse a feature-specific wait helper; do not guess a fixed duration or assume + the test JVM's configuration singleton reflects a separately launched node's configuration. - Wait for each relevant layer separately. A node status reported by ConfigNode does not prove that an existing client has refreshed its endpoint snapshot or recovered its connection. - For locally stopped nodes, use `EnvUtils.getNodeStatusAfterLocalStop()` where applicable; @@ -76,13 +101,23 @@ These instructions apply to `integration-test/` and supplement the repository-le cover the scenario. Use a separate cluster or restart only when required by the tested lifecycle, topology, configuration, or isolation. - Keep tests that share a cluster independent of execution order: isolate their data and restore - changed state or configuration using the repository's test helpers. -- Follow the neighboring tests' runner, category, and environment lifecycle conventions. Select - the category matching the data model and topology required by the scenario. + changed state or configuration using the repository's test helpers. Wait for asynchronous + cleanup effects, such as quota accounting after schema deletion, before the next method's setup; + successful cleanup SQL alone does not prove that dependent cluster state has converged. - Close sessions, pools, and result sets; shut down test-owned executors and clean the cluster even when assertions fail. Use try-with-resources and `finally` as appropriate. - Assert the behavior under test and its recovery result. Only tolerate expected cleanup errors in narrowly scoped cleanup code; do not swallow failures from the operation being verified. +- When enabling an old test or changing its expected result, verify the current intended + client/server contract and preserve the original regression input and execution path. For an + expected rejection, assert the specific exception and status/message so that an unrelated + transport or setup failure cannot satisfy the test. +- Do not weaken assertions, remove cluster categories, or add `@Ignore` merely to make a failing + test pass CI. If a known product defect must be quarantined, retain the reproducer and document + the defect, affected topology, and tracking reference; report skipped cases separately. +- Account for platform semantics in process-exit assertions: POSIX exposes only the low 8 bits + of an exit status. When checking a portable process status derived from a `TSStatusCode`, + compare the low 8 bits of both values (`& 0xFF`); keep full status-code checks for RPC responses. ### Result ordering across DataNodes @@ -111,16 +146,36 @@ always enabling `-P with-integration-tests` when building or testing this module Format Java changes with `mvn spotless:apply -pl integration-test -P with-integration-tests`. +### Selection check + +After adding or renaming tests, changing categories or inheritance, or editing CI test selection, +run this source-level check from the repository root before the Maven build: + +```bash +python3 .github/scripts/check-it-selection.py +``` + +- The check detects non-matching class names, missing effective categories, and missing or + duplicated shard candidates. Also inspect the intended profile's `groups`/`excludedGroups`, + POM excludes, and `@Ignore`; the check does not prove that a particular CI job selects a test. + Running a test in an IDE or explicitly with `-Dit.test` does not prove default CI discovery. +- When editing CI sharding, reuse `.github/scripts/build-it-shard-list.sh`: shard all candidate + classes by fully qualified name and let Failsafe apply category filters. Do not grep source + files for category names; that misses inherited categories. Preserve POM excludes by using + `failsafe.includesFile`, and reject empty lists because Failsafe falls back to default includes. + Verify that shards cover each candidate exactly once and that their selected-test union matches + the unsharded run for the same profile. + ### Targeted test runs Run only the affected IT classes or methods for a focused change. Select the matching test profile: -| Data model | Default topology | Test profile | -| --- | --- | --- | -| Tree | 1 ConfigNode + 1 DataNode | `SimpleIT` (default) | -| Tree | 1 ConfigNode + 3 DataNodes | `ClusterIT` | -| Table | 1 ConfigNode + 1 DataNode | `TableSimpleIT` | -| Table | 1 ConfigNode + 3 DataNodes | `TableClusterIT` | +| Data model | Default topology | JUnit category | Test profile | +| --- | --- | --- | --- | +| Tree | 1 ConfigNode + 1 DataNode | `LocalStandaloneIT` | `SimpleIT` (default) | +| Tree | 1 ConfigNode + 3 DataNodes | `ClusterIT` | `ClusterIT` | +| Table | 1 ConfigNode + 1 DataNode | `TableLocalStandaloneIT` | `TableSimpleIT` | +| Table | 1 ConfigNode + 3 DataNodes | `TableClusterIT` | `TableClusterIT` | For example, run one tree-model cluster test method: @@ -134,8 +189,12 @@ mvn clean verify -DskipUTs \ Use `-Dit.test=ClassName` to run a whole class. Replace `-PClusterIT` with the profile needed by the test; it may be omitted for the default `SimpleIT` profile. -- Check the Failsafe report to confirm that the selected test actually ran; the no-tests flags - allow upstream reactor modules to have no matching tests. +- Check `integration-test/target/failsafe-reports` to confirm that the intended classes and + methods actually ran, including expected parameterized variants, and inspect skipped counts. + A successful Maven exit is insufficient: the no-tests flags allow upstream reactor modules + to have no matching tests and can also hide an incorrectly selected IT. +- For changes involving distributed state in tests enabled for both single- and multi-DataNode + profiles, verify the affected cases in both profiles. A 1C1D pass does not establish 1C3D behavior. - Report the tested revision, edition, profile, and result accurately. Passing an upstream build does not establish that the corresponding downstream edition was tested.
