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]

Reply via email to