ignitetcbot commented on PR #13136: URL: https://github.com/apache/ignite/pull/13136#issuecomment-4462054763
Thanks for working on this flaky test. I have two small suggestions: 1. Please reconsider making `TxDeadlockDetection.deadLockTimeout` `static final` while `TxDeadlockDetectionNoHangsTest` relies on `@WithSystemProperty`. In the suite, earlier tests can initialize `TxDeadlockDetection` before the class-level property is applied, so this test may still use the default timeout. A safer option is to read `IGNITE_TX_DEADLOCK_DETECTION_TIMEOUT` when creating the timeout object, or keep an explicit test hook. See [`TxDeadlockDetection`](https://github.com/apache/ignite/blob/c4d175a2a512c274d7ffbc9e6dfd8ec423971fbb/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetection.java#L56), [`TxDeadlockDetectionNoHangsTest`](https://github.com/apache/ignite/blob/c4d175a2a512c274d7ffbc9e6dfd8ec423971fbb/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockDetectionNoHangsTest.java#L50), and suite order in [`TxDeadlockDetectionTestSuite`](https://github.com/apache/ignite/blob/c4d 175a2a512c274d7ffbc9e6dfd8ec423971fbb/modules/core/src/test/java/org/apache/ignite/testsuites/TxDeadlockDetectionTestSuite.java#L39-L46). 2. In `TxDeadlockCauseTest`, the new `catch` stores only exceptions that already have `TransactionDeadlockException`. If both transaction threads fail for another reason, the test will lose the original failure and report only `assertNotNull`. Could we keep the first non-deadlock exception as a diagnostic fallback, but still prefer the deadlock exception when it appears? See [`checkCauseObject`](https://github.com/apache/ignite/blob/c4d175a2a512c274d7ffbc9e6dfd8ec423971fbb/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxDeadlockCauseTest.java#L206-L209). -- 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]
