-
Notifications
You must be signed in to change notification settings - Fork 73
test: added test case for jakarta/java/codeAction cancellation #1363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
test: added test case for jakarta/java/codeAction cancellation #1363
Conversation
…tion requests gracefully
|
Please start your commit with test: |
|
I can merge your test if you wish but I am not sure that it reproduces the case with jakarta codeaction which misses, right? |
|
@angelozerr I'm still working into this PR. Will change the PR status and update you once it is ready for review. Thanks! |
| } | ||
| ]""" | ||
| , | ||
| false, // simulateCancellation = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As simulateCancellation is a specific case, please remove this change and define
List<IntentionAction> assertCodeActions(@NotNull String fileName,
@NotNull IntentionActionKind kind,
@NotNull String editorContentText,
@NotNull List<CodeAction> codeActions,
@NotNull String... expectedActions)
which call the method with simulateCancellation=false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed the suggestion. Thank you @angelozerr
| /** | ||
| * Test case for InvalidWebFilter quick fix | ||
| */ | ||
| public class CodeActionCancellationTest extends LSPCodeActionFixtureTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To show in action the cancel support easily, I suggest that you remove this classe and you do test in the same WebFilterQuickFixTest.
This class could define a private method like
assertCodeActionss(boolen simulateCancellation , @NotNull String... expectedActions) (which will call
assertCodeActions(TEST_FILE_NAME,
IntentionActionKind.QUICK_FIX_ONLY,
// language=JAVA
"""
package io.openliberty.sample.jakarta.servlet;
...
and after that you could define 2 tests methods:
public void testWebFilterQuickFix() {
assertCodeActions(false, "Add the `servletNames` attribute to @WebFilter",
"Add the `urlPatterns` attribute to @WebFilter",
"Add the `value` attribute to @WebFilter");
}
public void testWebFilterQuickFixWithCancellation() {
assertCodeActions(true);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angelozerr I’ve made the modification as suggested, with a small change — since expectedActions is included in the request, I had to add that argument for both tests. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do my suggestion to fix tests
|
@dessina-devasia I dont understand your last commit? Hzve you understood my suggestion? You should remove your cancellation test class and write 2 tests in WebFilterQuickFixTest |
|
@angelozerr Regarding the last commit, I just formatted the code and removed the unnecessary comments I had added. Regarding the suggestion, I understood it and have made the changes. I’m currently testing them and confirming that the results are as expected. |
| "Add the `servletNames` attribute to @WebFilter", | ||
| "Add the `urlPatterns` attribute to @WebFilter", | ||
| "Add the `value` attribute to @WebFilter"); | ||
| private List<IntentionAction> assertCodeActions(boolean simulateCancellation) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I have suggested, please add @NotNull String... expectedActions as second parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI build fail https://github.com/redhat-developer/lsp4ij/actions/runs/19169198726/job/54801075380?pr=1363#step:5:297 because you need to do my suggestion with expectedActions.
|
Hi @angelozerr , I cannot speak to the details for this PR, but I just want to make sure we are all on the same page from a high level. The goal of this new test case is to reliably reproduce the intermittent quick fix missing bug (Issue in Liberty Tools repo: OpenLiberty/liberty-tools-intellij#748, Issue in LSP4IJ repo: #294). This is to follow up on your request here: #294 (comment). As you are aware, Dessina worked with you to create a positive test case for the Jakarta quick fix scenario: #862. The idea here is to take the positive scenario and recreate the bug scenario with this new test. We would expect this new test to fail, resulting in CI build failures. This PR does not need to be merged right away. The intention is to provide a test that can reliably reproduce the bug and allow for moving forward on investigating the root cause of the bug. |
|
Hi @TrevCraw I understand your goal but the test fails because it expects a result although quickfix is cancelled so the test must be fixed. This test is interesting but I fear that it doesnt cover your usecase. |
@angelozerr , I think this is what I would expect. A cancellation request is causing the quick fixes to disappear. Maybe the bug is the fact that a cancellation request is being sent when it should not be? In that case, we would need to reproduce triggering the unwanted cancellation request instead of directly creating one in the test. Is that what you would suggest?
I agree that might be true. Could you explain how we can recreate our use case? |
Exactly and it is one reason why the test is failling. My suggestion was to do:
I don't know since I don't understand how you can have this problem and since according @dessina-devasia it is working with MicroProfile LS? Without debugging your Jakarta LS it is hard to help you. |
That is not my understanding. I believe this issue is reproducible with LSP4MP as well. Maybe @mrglavas, you could chime in here? I'm fairly certain we reproduced this problem with MicroProfile quick fixes as well. |
|
My questions are:
|
|
@angelozerr Let me try to reproduce the issue with LSP4MP using Liberty Tools. |
|
@angelozerr I'm able to reproduce this issue with LSP4MP using Liberty Tools.
|
|
@angelozerr Reproduced issue with LSP4Jakarta using Liberty Tools.
|
|
Could you please test with IJ Quarkus? |
|
Your sample returns codeaction as source kind it is not a quickfix. Please try with a quickfix. |
@TrevCraw We have reproduced this with Liberty Tools and LSP4MP before though not sure we collected full trace. |
|
@dessina-devasia @TrevCraw to be honnest with you I am lost and I don't understand what it doesn't work with LSP4MP in Liberty Tools. Take a commons sample:
* If you click on the `Insert @Liveness` it should insert it:
Is it working with your Liberty Tools? If no is it working with IJ Quarkus? |
|
@angelozerr We’re encountering this issue intermittently - it doesn’t occur consistently. |
|
With the health usecase? |
|
@angelozerr @TrevCraw @mrglavas Please find the LTI automation test failure due to intermittent quick fix missing case. Here I'm attaching the log. LTI automation test failure logGradleSingleModJakartaLSTest > testJakartaQuickFixInJavaPart() STANDARD_ERROR log4j:WARN No appenders could be found for logger (com.automation.remarks.video.recorder.monte.MonteRecorder). log4j:WARN Please initialize the log4j system properly. log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info. java.lang.RuntimeException: Hover on text: 'getProperties' did not trigger a pop-up window to open at io.openliberty.tools.intellij.it.UIBotTestUtils.hoverForQuickFixInAppFile(UIBotTestUtils.java:883) at io.openliberty.tools.intellij.it.SingleModJakartaLSTestCommon.testJakartaQuickFixInJavaPart(SingleModJakartaLSTestCommon.java:176) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727) at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131) at com.intellij.testFramework.junit5.impl.TestLoggerInterceptor.intercept$lambda$0(TestLoggerInterceptor.kt:12) at com.intellij.testFramework.TestLoggerKt.recordErrorsLoggedInTheCurrentThreadAndReportThemAsFailures(testLogger.kt:72) at com.intellij.testFramework.junit5.impl.TestLoggerInterceptor.intercept(TestLoggerInterceptor.kt:11) at com.intellij.testFramework.junit5.impl.AbstractInvocationInterceptor.interceptTestMethod(AbstractInvocationInterceptor.kt:29) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106) at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92) at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86) at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:119) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:94) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:89) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:62) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at jdk.proxy1/jdk.proxy1.$Proxy2.stop(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) Caused by: com.intellij.remoterobot.utils.WaitForConditionTimeoutException: Exceeded timeout (PT20S) for condition function (Failed to find 'Container' by '//div[@Class='JPanel']//div[@Class='HyperlinkLabel' and @mytext.key='daemon.tooltip.more.actions.link.label']' in 20s) at com.intellij.remoterobot.utils.RepeatUtilsKt.waitFor(RepeatUtils.kt:51) at com.intellij.remoterobot.utils.RepeatUtilsKt.waitFor$default(RepeatUtils.kt:37) at com.intellij.remoterobot.SearchContext$find$1.invoke(SearchContext.kt:53) at com.intellij.remoterobot.SearchContext$find$1.invoke(SearchContext.kt:51) at com.intellij.remoterobot.stepsProcessing.StepWorkerKt.step(StepWorker.kt:23) at com.intellij.remoterobot.SearchContext$DefaultImpls.find(SearchContext.kt:51) at com.intellij.remoterobot.fixtures.ContainerFixture.find(ContainerFixture.kt:12) at io.openliberty.tools.intellij.it.fixtures.ProjectFrameFixture.getQuickFixMoreActionsLink(ProjectFrameFixture.java:294) at io.openliberty.tools.intellij.it.UIBotTestUtils.hoverForQuickFixInAppFile(UIBotTestUtils.java:861) ... 89 more |
|
@dessina-devasia before spending time to investigate your issue I would like to know if you have the same problem with MicroProfile and IJ Quarkus because me I have never seen this issue in IJ Quarkus |




Adds
JakartaCodeActionCancellationTestunderfeatures/codeActionto emulate the language server cancelling ajakarta/java/codeActionrequest (eg: LSP error -32800).This test confirms that LSP4IJ correctly handles cancellation without producing quick fixes or propagating exceptions.