[paul-lartigau]

https://issues.apache.org/jira/browse/TINKERPOP-3304

mergeV()/mergeE() now report a keyed property (KeyedProperty / KeyedVertexProperty) as the old value of a property that did not exist yet, as property() does since 3.4.0 (TINKERPOP-1831), instead of null. The stale Event Javadoc is aligned.

Tests: StubMutationListener now records the old values and removed properties it receives, and EventStrategyProcessTest asserts on them:

• existing mergeV/mergeE tests (shouldTriggerAddVertexPropertyChangedViaMergeV, shouldTriggerUpdateEdgePropertyAddedViaMergeE, …) now check the old value: a keyed property for a new key, the real previous property otherwise;
• new tests in the same pattern: shouldTriggerAddVertexPropertyAddedViaMergeV, shouldTriggerUpdateEdgePropertyChangedViaMergeE, shouldTriggerEdgePropertyAdded;
• the property() tests now assert the KeyedProperty / KeyedVertexProperty type;
• the property removal tests assert the removed property's key and value.

shouldTriggerAddVertexPropertyAddedViaMergeV and shouldTriggerUpdateEdgePropertyAddedViaMergeE fail without the fix.

Validated: targeted run of EventStrategyProcessTest through TinkerGraphProcessStandardTest (gremlin-core, gremlin-test, tinkergraph-gremlin) on JDK 17, not a full reactor build. 3.8-dev and master have the same code, the change merges forward.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.62500% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.02%. Comparing base (4722890) to head (12ec62c).
⚠️ Report is 183 commits behind head on 3.7-dev.

Files with missing lines	Patch %	Lines
.../strategy/decoration/EventStrategyProcessTest.java	90.16%	9 Missing and 3 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             3.7-dev    #3759      +/-   ##
=============================================
+ Coverage      75.49%   76.02%   +0.52%     
- Complexity     13161    13356     +195     
=============================================
  Files           1092     1095       +3     
  Lines          67208    67725     +517     
  Branches        7391     7465      +74     
=============================================
+ Hits           50742    51491     +749     
+ Misses         13837    13587     -250     
- Partials        2629     2647      +18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[spmallette]

For committers - Created PR root bead: tp-632b

🤖 Agent assisted comment.

[spmallette]

thanks for submitting this - looking at the tests holistically, i can't help wondering if rather than adding new mergeV/E tests we'd do better to fix the existing ones and perhaps add more in the established pattern. If StubMutationListener would have helped to perform the right assertions from the outset I think we would have caught this problem early on because we already have shouldTriggerAddVertexPropertyChangedViaMergeV and shouldTriggerUpdateEdgePropertyAddedViaMergeE. On top of that, nothing is really asserting that property() is even returning KeyedProperty and KeyedVertexProperty types (for that matter I'm not sure I even see tests validating cases for when there was a key removed.

would you be up for hardening the test cases a bit more with respect to these concerns? i think it would be nice to close these gaps.

[paul-lartigau]

Thanks, good point. I dropped the two standalone tests and hardened the existing ones instead: StubMutationListener now records the old values and removed properties, so the existing mergeV/mergeE tests catch the bug (they fail without the fix). I also added a few tests in the same pattern, assertions that property() hands listeners KeyedProperty/KeyedVertexProperty, and key/value checks on the removal events. Squashed into a single commit; the description is updated.