Skip to content

Fix sporadic failure of DebugIntegrationTest.testCancellation#910

Merged
pisv merged 1 commit into
eclipse-lsp4j:mainfrom
pisv:fix-cancellation-test
Nov 5, 2025
Merged

Fix sporadic failure of DebugIntegrationTest.testCancellation#910
pisv merged 1 commit into
eclipse-lsp4j:mainfrom
pisv:fix-cancellation-test

Conversation

@pisv

@pisv pisv commented Nov 2, 2025

Copy link
Copy Markdown
Contributor

This PR fixes sporadic failure of DebugIntegrationTest.testCancellation, which happened when merging #907:
https://github.com/eclipse-lsp4j/lsp4j/actions/runs/18980903518/job/54212884068

The test expected that CancellationException would be thrown when calling future.get after calling future.cancel, without checking that future.cancel actually returned true.

However, future.cancel can also return false if the future has already completed, which may happen concurrently with the future.cancel call. Attempting to cancel a future is inherently racy with the future completion.

In this specific case, the test should expect not only CancellationException (when future.cancel returns true), but also ExecutionException caused by ResponseErrorException with message "The request (id: 1, method: 'askClient') has been cancelled" (when future.cancel returns false).

See also #907 (comment) and later comments in the conversation.

@pisv

pisv commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

The same problem is also present in IntegrationTest.testCancellation (which seems to be a twin of DebugIntegrationTest.testCancellation). I'll update this PR accordingly.

Also fixes `IntegrationTest.testCancellation`, which is a twin of
`DebugIntegrationTest.testCancellation` and has the same problem.
@pisv pisv force-pushed the fix-cancellation-test branch from 66bed5d to b791ae1 Compare November 3, 2025 18:23
@pisv pisv merged commit 5d0dc40 into eclipse-lsp4j:main Nov 5, 2025
4 checks passed
@pisv pisv deleted the fix-cancellation-test branch November 5, 2025 14:24
@pisv pisv added this to the 1.0.0 milestone Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant