nevzheng commented on PR #13517:
URL: https://github.com/apache/gravitino/pull/13517#issuecomment-5867613874
@jerryshao PTAL
**Changes:**
- `654cfe33a` — timeout 90→120min, renamed the `inputs.property` key to
`coreTestIncludesDockerTaggedTests` (distinct from the `extraProperties` flag),
dropped the hardcoded `()` in `BackendTestExtension#getDisplayName()`.
- `efee6a4c9` — replaced the per-backend `excludeTags()` scheme with
`@DatabaseTest`/`BackendLaneCondition` (see below).
**Explain (on the `includeDockerTaggedTests` ordering concern):**
Tested this directly rather than just reasoning about it — warm daemon, cold
daemon (`./gradlew --stop` first), and a `doLast` reflecting the actual
`testFramework` object off a real `:core:coreH2Test` run. All three show
`extraProperties[includeDockerTaggedTests]` and the resulting
`includeTags`/`excludeTags` exactly as intended at execution time. Root cause:
the read happens inside `param.doFirst {}`, i.e. execution phase, which runs
strictly after the *entire* configuration phase (including core's `register`
action) completes — so the configureEach-before-register ordering
([gradle/gradle#8906](https://github.com/gradle/gradle/issues/8906)) doesn't
apply here regardless. Could reproduce with different repro steps if you have
them, but couldn't get this to fail under CI's actual per-shard invocation
pattern (`dev/ci/test-shards.sh`).
**Minor (compare-legacy):**
Went further than wiring the Python script in.
`excludeTags(otherTwoBackends)` allowed two silent failure modes with zero
build-time check: a class tagged with two backends excluded from every lane, or
a class missing its tag running redundantly in all three. Replaced it with one
typed source of truth:
- `DatabaseTest` (`backends()`, defaults to all 3) + `BackendLaneCondition`
(`ExecutionCondition` reading the `gravitino.core.test.backend` property lanes
already set)
- `TestDatabaseTestClassification` — build-time guard, no DB/Docker, catches
empty `backends()` or leftover raw tags
- `core/build.gradle.kts`'s 3-way `excludeTags` table → one
`includeTags(coreDatabaseTestTag)`
Makes the bug class unrepresentable rather than just detectable
post-execution. Still happy to wire `core_test_identity.py` too as a
belt-and-suspenders CI check if you'd like it.
**Listing (before/after equivalence, `:core:coreH2Test`):**
| | Classes | Tests | Skipped | Executed | Failures |
|---|---|---|---|---|---|
| Before | 40 | 520 | 0 | 520 | 0 |
| After | 42 | 534 | 14 | 520 | 0 |
Same 520 executed, same outcomes. The 14 "new" tests were always excluded
from Gradle's own discovery (invisible); now they're a reported skip with a
reason string — e.g. `MySQLTest`/`PostgreSQLTest` correctly disabled under the
H2 lane, 0.004s, no container attempted.
Also added `TestBackendLaneCondition` (5/5, direct unit coverage of the
condition across all-backend/single-backend/multi-backend cases).
Nevin
--
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]
