RFR: 7903965: Fix Launching Tests from Context Menu
Oleksii Sylichenko
duke at openjdk.org
Mon Mar 10 12:59:53 UTC 2025
On Mon, 10 Mar 2025 12:08:33 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
>> Currently, there is an issue with running TestNG/JUnit tests when using the context menu on the editor tab. TestNG/JUnit intercepts the test execution from JTReg.
>>
>> After fixing this issue, I decided to extend the fix to cover all potential use cases of the context menu.
>>
>> As part of this pull request, I propose enhancing the process of running tests via the context menu. The context menu can be used in the following cases:
>>
>> - Editor tab
>> - File in the project tree
>> - Directory in the project tree
>> - Any location within a file
>>
>> The current changes automatically determine what needs to be executed:
>>
>> - Directory
>> - File
>> - Method
>>
>> Additionally, the issue with intercepting test execution through Application, TestNG, and JUnit configurations when using the context menu on the editor tab has been fixed.
>>
>> # Summary
>>
>> - Ensure that run configuration priority is no longer lost to Application, TestNG, or JUnit setups when launching via the context menu.
>> - Fix run configuration created via the context menu: find and launch the proper test element, depending on the current PSI element.
>> - Improve the logic for comparing existing run configurations with newly generated ones.
>> - Add JUnit plugin in the dependencies.
>>
>> # Detailed changes
>>
>> ## Add JUnit plugin into dependencies
>> - build.gradle
>> - plugin.xml
>>
>> ## JTRegClassConfigurationProducer.java
>> - Refactor `setupConfigurationFromContext`:
>> - Move the check for whether a file can be run to `JTRegUtils#isRunnableByJTReg`.
>> - Allow this method to create configurations for directories as well.
>> - Implement `preventRunPriorityLoss` to prevent priority takeover by other run configuration types (Application, TestNG, JUnit) under certain conditions.
>> - Replace the use of the current PSI element for generating names and querying run configurations with the specific element intended for execution.
>> - Implement `preventRunPriorityLoss`.
>> - Add Javadoc for `nameForElement`.
>>
>> ## JTRegConfigurationProducer.java
>> - Fix logic in `isConfigurationFromContext` for checking if two configurations are identical:
>> - Replace the use of the current PSI element for queries with the exact element intended for execution.
>> - Compare string-typed configuration parameters using a "not-nullify" approach (`null == ""`).
>> - Implement `findExactRunElement` to locate the specific PSI element for execution among the current element’s parents.
>> - Add a JUnit chec...
>
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/configuration/producers/JTRegClassConfigurationProducer.java line 125:
>
>> 123: }
>> 124: }
>> 125: }
>
>> Additionally, the issue with intercepting test execution through Application, TestNG, and JUnit configurations when using the context menu on the editor tab has been fixed.
>
> I think this issue is addressed in this code here.
>
> What are the steps to reproduce this issue? When I try to create a run configuration through the context menu on the editor tab, I seem to get a JTreg configuration every time. Or I get the option to choose one or the other:
> 
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/configuration/producers/JTRegConfigurationProducer.java line 76:
>
>> 74: }
>> 75:
>> 76: final Location<?> location = JavaExecutionUtil.stepIntoSingleClass(contextLocation);
>
> Suggestion:
>
> Location<?> location = JavaExecutionUtil.stepIntoSingleClass(contextLocation);
finals have been already removed in the last commit (a1350eb9ed33e626ffeab2518f656b226e5a1b96)
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/configuration/producers/JTRegConfigurationProducer.java line 90:
>
>> 88: final String contextFilePath = null != contextVirtualFile ? contextVirtualFile.getPath() : null;
>> 89:
>> 90: final String contextDirPath = (element instanceof PsiDirectory d) ? d.getVirtualFile().getPath() : null;
>
> Suggestion:
>
> String contextQuery = getQuery(element);
>
> PsiFile contextFile = (element instanceof PsiFile psiFile) ? psiFile : element.getContainingFile();
> VirtualFile contextVirtualFile = null != contextFile ? contextFile.getVirtualFile() : null;
> String contextFilePath = null != contextVirtualFile ? contextVirtualFile.getPath() : null;
>
> String contextDirPath = (element instanceof PsiDirectory d) ? d.getVirtualFile().getPath() : null;
finals have been already removed in the last commit (a1350eb9ed33e626ffeab2518f656b226e5a1b96)
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/configuration/producers/JTRegConfigurationProducer.java line 131:
>
>> 129: if (null != identifyingElement) { // null for name of the non-test inner class
>> 130: if (null == retval) {
>> 131: // When found, check the left hierarchy up to the class for runnability
>
> Suggestion:
>
> // When found, check the rest of the hierarchy up to the class for runnability
done: b7ff142290026c6ee42e0713a8ca8dba1e54a7cd
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/configuration/producers/JTRegConfigurationProducer.java line 233:
>
>> 231: @Override
>> 232: public boolean shouldReplace(@NotNull ConfigurationFromContext self, @NotNull ConfigurationFromContext other) {
>> 233: final RunConfiguration otherCnf = other.getConfiguration();
>
> Suggestion:
>
> RunConfiguration otherCnf = other.getConfiguration();
finals have been already removed in the last commit (a1350eb9ed33e626ffeab2518f656b226e5a1b96)
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/util/JTRegUtils.java line 94:
>
>> 92: public static final Predicate<PsiElement> IS_DIR_ELEMENT = PsiDirectory.class::isInstance;
>> 93: public static final Predicate<PsiElement> IS_FILE_ELEMENT = PsiFile.class::isInstance;
>> 94: public static final Predicate<PsiElement> IS_FILE_OR_DIR_ELEMENT = IS_FILE_ELEMENT.or(IS_DIR_ELEMENT);
>
> Why are these all predicates? I'd much prefer a set of static methods. These predicates make the logic harder to follow.
ok, no problem, I will replace them with methods.
> plugins/idea/src/main/java/com/oracle/plugin/jtreg/util/JTRegUtils.java line 355:
>
>> 353: public static boolean isRunnableByJTReg(@NotNull PsiElement element) {
>> 354: final PsiFile runFile;
>> 355: final PsiDirectory runDir;
>
> Suggestion:
>
> PsiFile runFile;
> PsiDirectory runDir;
finals have been already removed in the last commit (a1350eb9ed33e626ffeab2518f656b226e5a1b96)
-------------
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987218141
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987189818
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987190040
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987212635
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987189232
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987199807
PR Review Comment: https://git.openjdk.org/jtreg/pull/252#discussion_r1987188779
More information about the jtreg-dev
mailing list