[
https://issues.apache.org/jira/browse/TINKERPOP-3294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127214#comment-18127214
]
ASF GitHub Bot commented on TINKERPOP-3294:
-------------------------------------------
codecov-commenter commented on PR #3715:
URL: https://github.com/apache/tinkerpop/pull/3715#issuecomment-6174621531
##
[Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `78.57143%` with `6 lines` in your changes missing
coverage. Please review.
:warning: Please [upload](https://docs.codecov.com/docs/codecov-uploader)
report for BASE (`3.8-dev@c7538d1`). [Learn
more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#section-missing-base-commit)
about missing BASE report.
| [Files with missing
lines](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...ess/traversal/step/sideEffect/AddPropertyStep.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?src=pr&el=tree&filepath=gremlin-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstep%2FsideEffect%2FAddPropertyStep.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdGVwL3NpZGVFZmZlY3QvQWRkUHJvcGVydHlTdGVwLmphdmE=)
| 62.50% | [0 Missing and 3 partials :warning:
](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[.../strategy/decoration/EventStrategyProcessTest.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?src=pr&el=tree&filepath=gremlin-test%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstrategy%2Fdecoration%2FEventStrategyProcessTest.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi10ZXN0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdHJhdGVneS9kZWNvcmF0aW9uL0V2ZW50U3RyYXRlZ3lQcm9jZXNzVGVzdC5qYXZh)
| 85.00% | [1 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.8-dev #3715 +/- ##
==========================================
Coverage ? 76.73%
Complexity ? 15063
==========================================
Files ? 1134
Lines ? 66490
Branches ? 8131
==========================================
Hits ? 51020
Misses ? 12725
Partials ? 2745
```
</details>
[:umbrella: View full report in Codecov by
Harness](https://app.codecov.io/gh/apache/tinkerpop/pull/3715?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> AddPropertyStep does not capture the old property for EventStrategy when
> cardinality is not specified
> -----------------------------------------------------------------------------------------------------
>
> Key: TINKERPOP-3294
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3294
> Project: TinkerPop
> Issue Type: Bug
> Components: process
> Affects Versions: 3.7.7, 3.8.2
> Reporter: Paul
> Priority: Minor
> Fix For: 3.7.8, 3.8.3
>
>
> When EventStrategy is configured, AddPropertyStep calls
> captureRemovedProperty() to find the property being replaced, so it can be
> passed to vertexPropertyChanged() as the old value.
> captureRemovedProperty() checks the {{cardinality}} field of the step. That
> field is only set when the cardinality is given explicitly in the traversal,
> e.g. {{{}property(single, "name", "x"){}}}. With {{property("name", "x")}} it
> is null, so neither the {{single}} nor the {{set}} branch is taken and the
> old value is always reported as empty.
> The step already falls back to
> {{graph.features().vertex().getCardinality(key)}} when it writes the property
> (see TINKERPOP-2159), but that fallback is not used by
> captureRemovedProperty().
> To reproduce, with TinkerGraph (default cardinality {{{}single{}}}):
> {code:java}
> MutationListener listener = ...; // records oldValue in vertexPropertyChanged
> GraphTraversalSource g = traversal().withEmbedded(graph)
> .withStrategies(EventStrategy.build().addListener(listener).create());
> g.addV().property("name", "a").iterate();
> g.V().property("name", "b").iterate();
> // oldValue received by the listener is vp[empty], expected vp[name->a]
> {code}
> With {{property(single, "name", "b")}} the old value is reported correctly.
> The fix is to compute the effective cardinality once and pass it to
> captureRemovedProperty(). On 3.8-dev the fallback is computed after the call
> to captureRemovedProperty(), so it also needs to be moved up. The code
> differs between the two branches, so I plan to open one PR for 3.7-dev and
> one for 3.8-dev.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
