[
https://issues.apache.org/jira/browse/TINKERPOP-3010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17780548#comment-17780548
]
ASF GitHub Bot commented on TINKERPOP-3010:
-------------------------------------------
Cole-Greer commented on PR #2312:
URL: https://github.com/apache/tinkerpop/pull/2312#issuecomment-1783568217
The changes here LGTM, although I am not able to properly assess if the
changes are complete (there are no tests remaining in
TransactionMultiThreadedTest which do not belong). To the best of my knowledge,
moving all the tests which use TransactionExample is correct, however this is
not an area I know well.
VOTE +1
> Move TinkerGraph specific transaction testing
> ---------------------------------------------
>
> Key: TINKERPOP-3010
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3010
> Project: TinkerPop
> Issue Type: Bug
> Components: test-suite
> Affects Versions: 3.7.0
> Reporter: Ken Hu
> Priority: Critical
>
> As reported by multiple providers on Discord,
> From Boxuan:
> {quote}The newly added
> TransactionMultiThreadedTest::shouldHandleAddingSameVertexInDifferentTx in
> 3.7.0 release assumes the provider supports custom vertex ID, which was not
> the case in previous releases. Assuming this was not intentional, should we
> have a feature flag for this requirement?
> The newly added
> TransactionMultiThreadedTest::shouldHandleConcurrentChangeForProperty in
> 3.7.0 release assumes the behavior of two conflicting transactions is
> TransactionException. This is totally fine, but IMO this is TinkerGraph
> specific behavior and should not apply to providers in general. For example,
> another common approach is to let one transaction wait for the other.
> {quote}
> From Pieter:
> {quote}Hi, in upgrading Sqlg to 3.7.0 the TransactionMultiThreadedTest is
> executing now.
> Quite a few of the tests are hanging, but it seems to me an implementation
> issue with the test rather than the underlying transaction semantics.
> Multi threaded tests should not themselves have one thread waiting for
> another. The tests are creating artificial conditions where one thread waits
> for another.
> Take for example
> TransactionMultiThreadedTest.shouldHandleConcurrentVertexDelete
> shouldHandleAddingPropertyWhenOtherTxDeleteEdge
> shouldHandleAddingPropertyWhenOtherTxDeleteVertex
> shouldDeleteEdgeOnCommit
> shouldHandleConcurrentChangeForProperty
> shouldHandleAddingEdgeWhenOtherTxDeleteVertex
> shouldHandleConcurrentChangeForVertexProperty
> shouldHandleConcurrentDeleteEdge
> shouldHandleAddingSameVertexInDifferentTx
> shouldThrowExceptionWhenTryToAddVertexWithUsedId
> These are the ones I had to OptOut of.
> The other minor issue is on shouldThrowExceptionWhenTryToAddVertexWithUsedId
> My code throws an UnsupportedOperationException while the test expects an
> IllegalStateException
> I did not investigate which one is correct by I suspect its
> UnsupportedOperationException
> {quote}
> These tests exhibit TinkerTransactionGraph specific behavior and should be
> moved to TinkerTransactionGraphTest. Search through all tests in
> TransactionMultiThreadedTest for possible candidates.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
