slachiewicz opened a new pull request, #883: URL: https://github.com/apache/incubator-xtable/pull/883
## What is the purpose of the pull request `ITConversionController` is the single longest-running thing in CI, and because of how failsafe is configured it cannot be shortened by any amount of extra hardware. The root pom sets `reuseForks=false` alongside `forkCount=6`. That means a test class occupies exactly one fork and its tests run serially inside it — `forkCount` parallelises *across* classes, never within one. `ITConversionController` held 13 test methods expanding to 44 invocations, and in a recent run it reported: ``` Tests run: 44, ... Time elapsed: 1003 s -- in org.apache.xtable.ITConversionController ``` For context, in that same run `XTable Project Core` took 18:05 of a 27:22 total, and the next-longest test classes are well below the 1003s mark (`ITDeltaConversionSource` 684s, `ITHudiConversionSource` 635s). So this one class sets the floor for the whole build: more forks, a bigger runner, or build caching cannot move it. ## Brief change log - Add `ConversionControllerTestBase`, an abstract class holding the shared Spark fixture (`@BeforeAll` / `@AfterAll`), the `checkDatasetEquivalence` overloads, the UUID comparison helper, the partition argument builders and `getTableSyncConfig` - Split the tests across five classes, grouped by what they exercise: | Class | Methods | Invocations | | --- | --- | --- | | `ITConversionController` | `testVariousOperations` | 16 | | `ITConversionControllerIncrementalSync` | time travel, single-format, out-of-sync, no-op, retention | 8 | | `ITConversionControllerSchemaEvolution` | UUID columns, Delta column mapping, corrupted-snapshot recovery | 6 | | `ITConversionControllerConcurrentWrites` | concurrent inserts, table-service/compaction | 8 | | `ITConversionControllerPartitioning` | partition transforms, other Iceberg partition types | 6 | Test bodies and assertions are unchanged — this is a move, not a rewrite. 13 methods and 44 invocations before and after. The base class is named `ConversionControllerTestBase` on purpose: it matches neither failsafe's `**/IT*.java` includes nor surefire's `**/*Test.java` patterns, so it is not collected as a test class in its own right. Each class gets its own JVM and therefore its own `SparkSession`. That costs a little startup per class, which is the trade being made for letting the existing forks run the groups concurrently. ## Verify this pull request This pull request is already covered by existing tests — it is a reorganisation of them. Verified locally on Temurin 11 with the project's wrapper: - `./mvnw -pl xtable-core verify -Dit.test='ITConversionController*'` reports **`Tests run: 44`**, the same count as the class before the split - The failsafe output now shows all five classes starting concurrently, where previously there was a single `Running org.apache.xtable.ITConversionController` line with everything serialised behind it - `spotless:check` and `apache-rat:check` pass ### Expected effect, and its limit I want to be precise rather than optimistic here. `testVariousOperations` is roughly 64% of the original class's test time, so after the split the largest piece lands around 640s. That is just below `ITDeltaConversionSource` at 684s, which then becomes the binding constraint for `xtable-core`. So the expected shape is `xtable-core` roughly 18:05 → ~11:30, and the total build roughly 27:22 → ~21:00. Splitting `testVariousOperations` any further would *not* help until `ITDeltaConversionSource` and `ITHudiConversionSource` are split too — which is why this PR stops where it does. Those are model-based numbers from per-class timings, not a measurement; the CI run on this PR is the real check, and I am happy to post the observed `XTable Project Core` time here once it completes. -- 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]
