nevzheng commented on PR #13517:
URL: https://github.com/apache/gravitino/pull/13517#issuecomment-5876384804
@jerryshao PTAL — here's what changed since your initial review, and how
it's been tested.
## What changed:
- Replaced the single gatekeeper/master tag + boolean tag-expression scheme
with a typed, namespaced annotation API: CoreBackend.H2 / .MySQL / .PostgreSQL
/ .All. No raw @Tag("...") string literals in core/src/test anymore — a class
runs under exactly the lanes it's annotated for, nothing implicit.
- Removed the dead compare-legacy subcommand from
dev/ci/core_test_identity.py (unwired, unused by CI). manifest/reconcile are
unchanged and still the CI-enforced contract.
- Added ./gradlew :core:coreTestLaneOf -PclassName=<FQCN> — a discovery-only
local check that prints which lane(s) a class runs in (or an explicit warning
if it's Docker-tagged but carries no backend annotation), so contributors can
self-check before pushing instead of guessing.
- Deprecated :core:test in place (still works, now warns and points at the
four real lanes and coreTestLaneOf) rather than removing it, since other
tooling may still target it by name.
- Added design-docs/testing/core-db-split.md, an SPIP documenting the final
design, the alternatives we tried and rejected along the way (an
ExecutionCondition-based redesign that broke CI because skipped tests still
leave a trace in JUnit XML; a parameterized-annotation form rejected because
it'd break reconcile's exact-identity-equality requirement; a flat naming
scheme rejected after a real reproduced compile-error collision with existing
nested test classes).
## How it's been tested:
- Real ./gradlew :core:coreUnitTest / :core:coreH2Test runs against the
current code, confirmed via actual JUnit XML output (not just reasoning about
it).
- TestCoreDatabaseLaneAnnotations pins the annotation contract as a
regression test — exact tag expansion, stacking, inheritance, and that none of
the four annotations carry any meta-annotation outside {Documented, Inherited,
Retention, Target, Tag, Tags} (i.e., structurally guarantees no execution-time
hook can sneak back in).
- Two independent adversarial reviews (one on CI/build correctness, one on
contributor ergonomics) turned up a real bug before merge: a parameterized
test's default JUnit display name was leaking a backend token into its own
unit-lane XML, tripping manifest's foreign-marker check. Fixed, then
re-verified by running the real normalize_identity check across all 1968
coreUnitTest testcases — zero marker errors.
- MySQL/PostgreSQL Docker-backed lanes: [you mentioned you'd verify these
independently — fill in once you have]
--
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]