You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "petrovic-d (via GitHub)" <gi...@apache.org> on 2024/11/22 15:04:24 UTC
[PR] Action for running tests in parallel [netbeans]
petrovic-d opened a new pull request, #7979:
URL: https://github.com/apache/netbeans/pull/7979
New action for running tests in parallel. It can be invoked via command: `nbls.run.test.parallel`
Command support specifying modules for which tests will be ran in parallel. It's up to client to pass the module list, so by default it will run without passed modules.
To test it with module list, it needs to be modified like this:
in `java/java.lsp.server/vscode/src/extension.ts` file
modify line `835` to:
`testAdapter?.run(new vscode.TestRunRequest(), new vscode.CancellationTokenSource().token, true, ["module1", "module2"]);`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869195050
##########
java/maven/src/org/netbeans/modules/maven/ActionProviderImpl.java:
##########
@@ -132,6 +132,7 @@ public class ActionProviderImpl implements ActionProvider {
"javadoc", //NOI18N
COMMAND_TEST,
COMMAND_TEST_SINGLE,
+ COMMAND_TEST_PARALLEL,
Review Comment:
Just use current. They're automatically bumped between each NetBeans release.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2551995391
@petrovic-d assuming rebase & squash
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1872986747
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -154,7 +155,14 @@ private String statusToState(Status status) {
private static ModuleInfo getModuleInfo(TestSession session) {
Project project = session.getProject();
String moduleName = project != null ? ProjectUtils.getInformation(project).getDisplayName() : null;
- String modulePath = project != null ? project.getProjectDirectory().getPath() : null;
+ String modulePath = getModuleTestPath(project);
return new ModuleInfo(moduleName, modulePath);
}
+
+ private static String getModuleTestPath(Project project) {
+ if (project == null) {
+ return null;
+ }
+ return Paths.get(project.getProjectDirectory().getPath(), "src", "test", "java").toString();
Review Comment:
I agree that hardcoding `src/test/java` is not ideal. I wonder how this is show is this visible in the UI? I tried to find out, but so far, I wasn't successful.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1870947902
##########
java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java:
##########
@@ -227,7 +218,7 @@ private boolean isSurefireRunningInParallel() {
}
private static final String SECONDS_REGEX = "s(?:ec(?:ond)?(?:s|\\(s\\))?)?"; //NOI18N
- private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " - in (.*)";
+ private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " -+ in (.*)";
Review Comment:
Yes. `--` appear in the output.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "matyasberry (via GitHub)" <gi...@apache.org>.
matyasberry commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2709798775
Hi, any chance of reusing these changes to also support what https://github.com/apache/netbeans/pull/3995 was intended to do?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869635687
##########
ide/projectapi/src/org/netbeans/spi/project/ContainedProjectFilter.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.spi.project;
+
+import java.util.Collections;
+import java.util.List;
+import org.netbeans.api.project.Project;
+
+/**
+ *
Review Comment:
Well, we can introduce `filterMode`, which would be filled depending on the list size?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2519804617
@sdedic sorry, I'd missed that the addition was incompatible. Am not involved with releases at the moment, and only commented after a glance at this to try and save @ebarboni some headaches post NB24.
_If_ this has to go in as is, then either wait for the API sigs to merge in a week or so and rebase, manually patch the 3 required lines into the sig file (which should pass then?), or merge and be prepared to help Eric with the API signature integration post release.
OTOH, either the test is relevant or not? Add the methods as default on the interface, as you did in the last change to that file to not require changes to pass tests? Or lets just stop having sigtests for friend APIs as they're irrelevant - the build will pick up internal breakage, and we don't support external usage.
Our process for handling sig tests for releases could do with a review anyway. I have some ideas how to improve that, but we have the one we have for now.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1890306868
##########
ide/projectapi/src/org/netbeans/spi/project/NestedClass.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.spi.project;
+
+import java.util.Objects;
+import org.openide.filesystems.FileObject;
+
+/**
+ * Structure representing an identification of a nested class in a file.
+ *
+ * <p>
+ * <code>NestedClass</code> can be used to represent nested classes within parent class
+ * Example:
+ * If we have following structure: ParentClass (parent-of) ChildClass1 (parent-of) ChildClass2,
+ * for ChildClass1 className field would contain "ChildClass1" and topLevelClassName would contain "ParentClass",
+ * for ChildClass2 className field would contain "ChildClass1.ChildClass2" and topLevelClassName would contain "ParentClass"
+ * </p>
+ *
+ * @author Dusan Petrovic
+ *
+ * @since 1.99
+ */
+public final class NestedClass {
Review Comment:
Not really a request to change anything, just an observation: this whole angle around `NestedClass` seems mostly orthogonal to the run tests in parallel feature, and could probably have been a separate PR.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1889758592
##########
ide/projectapi/src/org/netbeans/spi/project/NestedClass.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.spi.project;
+
+import java.util.Objects;
+import org.openide.filesystems.FileObject;
+
+/**
+ * Structure representing an identification of a nested class in a file.
+ *
+ * <p>
+ * <code>NestedClass</code> can be used to represent nested classes within parent class
+ * Example:
+ * If we have following structure: ParentClass (parent-of) ChildClass1 (parent-of) ChildClass2,
+ * for ChildClass1 className field would contain "ChildClass1", and
+ * for ChildClass2 className field would contain "ChildClass1$ChildClass2"
+ * </p>
+ *
+ * @author Dusan Petrovic
+ *
+ * @since 1.99
+ */
+public final class NestedClass {
+
+ private FileObject file;
Review Comment:
Make this `final` to be thread-safe.
##########
ide/projectapi/src/org/netbeans/spi/project/SingleMethod.java:
##########
@@ -53,6 +54,37 @@ public SingleMethod(FileObject file, String methodName) {
this.file = file;
this.methodName = methodName;
}
+
+ /**
+ * Creates a new instance holding the specified identification
+ * of a method/function in nested class in a file.
+ *
+ * @param file file to be kept in the object
+ * @param methodName name of a method inside the file
+ * @param nestedClass nested class containing the method
+ *
+ * @exception java.lang.IllegalArgumentException
+ * if the nested class name is {@code null}
+ * @since 1.99
+ */
+ public SingleMethod(FileObject file, String methodName, NestedClass nestedClass) {
Review Comment:
I'd take the `file` from `NestedClass`.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/WorkspaceServiceImpl.java:
##########
@@ -401,11 +403,15 @@ public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
LOG.log(Level.INFO, "Project {2}: {0} test roots opened in {1}ms", new Object[] { testRoots.size(), (System.currentTimeMillis() - t), file});
BiFunction<FileObject, Collection<TestMethodController.TestMethod>, Collection<TestSuiteInfo>> f = (fo, methods) -> {
String url = Utils.toUri(fo);
+ Project owner = FileOwnerQuery.getOwner(fo);
+ String moduleName = owner != null ? ProjectUtils.getInformation(owner).getDisplayName(): null;
Review Comment:
Careful here. Check with customized gradle and maven project (display) names (different from artfiact names) that it works well.
##########
ide/projectapi/src/org/netbeans/spi/project/SingleMethod.java:
##########
@@ -94,14 +126,15 @@ public boolean equals(Object obj) {
return false;
}
SingleMethod other = (SingleMethod) obj;
- return other.file.equals(file) && other.methodName.equals(methodName);
+ return other.file.equals(file) && other.methodName.equals(methodName) && other.nestedClass.equals(nestedClass);
}
@Override
public int hashCode() {
int hash = 7;
hash = 29 * hash + this.file.hashCode();
hash = 29 * hash + this.methodName.hashCode();
+ hash = 29 * hash + this.nestedClass.hashCode();
Review Comment:
:warning: Objects.hashCode() to avoid NPE on nestedClass; similar for equals. Unlike the other fields, nestedclass is optional.
##########
ide/projectapi/src/org/netbeans/spi/project/SingleMethod.java:
##########
@@ -31,6 +31,7 @@ public final class SingleMethod {
private FileObject file;
private String methodName;
+ private NestedClass nestedClass;
Review Comment:
maybe final, as well as the other fields.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java:
##########
@@ -632,11 +656,27 @@ static Lookup createTargetLookup(Project prj, SingleMethod singleMethod, FileObj
Lookup methodLookup = Lookups.singleton(singleMethod);
arr.add(methodLookup);
}
+ if (nestedClass != null) {
+ Lookup nestedClassLookup = Lookups.singleton(nestedClass);
+ arr.add(nestedClassLookup);
+ }
+ if (projectFilter != null) {
+ Lookup projectLookup = Lookups.singleton(projectFilter);
+ arr.add(projectLookup);
+ }
if (toRun != null) {
arr.add(toRun.getLookup());
}
return new ProxyLookup(arr.toArray(new Lookup[0]));
}
+
+ static ContainedProjectFilter getProjectFilter(Project prj, Map<String, Object> launchArguments) {
+ List<String> projectsArg = argsToStringList(launchArguments.get("projects"));
+ List<Project> projects = ProjectUtils.getContainedProjects(prj, false).stream()
Review Comment:
Note: this does not allow to specify projects nested more levels in the project's hierarchy. Not a bug, and can be enhanced later in a compatible way.
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -63,53 +87,122 @@ export class NbTestAdapter {
for (let item of include) {
if (item.uri) {
this.set(item, 'enqueued');
+ item.parent?.children.forEach(child => {
+ if (child.id?.includes(item.id)) {
+ this.set(child, 'enqueued');
+ }
+ })
const idx = item.id.indexOf(':');
+ const nestedClassIdx = item.id.indexOf('$');
+ let nestedClass: string | undefined;
+ if (nestedClassIdx > 0) {
+ nestedClass = idx < 0
+ ? item.id.slice(nestedClassIdx + 1)
+ : item.id.substring(nestedClassIdx + 1, idx);
+ }
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1));
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1), nestedClass);
}
}
}
} else {
this.testController.items.forEach(item => this.set(item, 'enqueued'));
for (let workspaceFolder of workspace.workspaceFolders || []) {
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ if (testInParallel) {
+ await commands.executeCommand(COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString(), undefined, undefined, undefined, true, projects);
+ } else {
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ }
}
}
}
if (this.started) {
this.itemsToRun.forEach(item => this.set(item, 'skipped'));
+ }
+ // TBD - message
Review Comment:
TBD - for the future ?
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -63,53 +87,122 @@ export class NbTestAdapter {
for (let item of include) {
if (item.uri) {
this.set(item, 'enqueued');
+ item.parent?.children.forEach(child => {
+ if (child.id?.includes(item.id)) {
+ this.set(child, 'enqueued');
+ }
+ })
const idx = item.id.indexOf(':');
+ const nestedClassIdx = item.id.indexOf('$');
+ let nestedClass: string | undefined;
+ if (nestedClassIdx > 0) {
+ nestedClass = idx < 0
+ ? item.id.slice(nestedClassIdx + 1)
+ : item.id.substring(nestedClassIdx + 1, idx);
+ }
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1));
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1), nestedClass);
}
}
}
} else {
this.testController.items.forEach(item => this.set(item, 'enqueued'));
for (let workspaceFolder of workspace.workspaceFolders || []) {
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ if (testInParallel) {
+ await commands.executeCommand(COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString(), undefined, undefined, undefined, true, projects);
+ } else {
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ }
}
}
}
if (this.started) {
this.itemsToRun.forEach(item => this.set(item, 'skipped'));
+ }
+ // TBD - message
+ else {
+ this.itemsToRun.forEach(item => this.set(item, 'failed', new TestMessage('Build failure'), false, true));
}
this.itemsToRun = undefined;
this.currentRun.end();
this.currentRun = undefined;
}
}
- set(item: TestItem, state: 'enqueued' | 'started' | 'passed' | 'failed' | 'skipped' | 'errored', message?: TestMessage | readonly TestMessage[], noPassDown? : boolean): void {
+ set(item: TestItem, state: SuiteState, message?: TestMessage | readonly TestMessage[], noPassDown? : boolean, dispatchBuildFailEvent?: boolean): void {
if (this.currentRun) {
switch (state) {
case 'enqueued':
+ this.dispatchTestEvent(state, item);
this.itemsToRun?.add(item);
this.currentRun.enqueued(item);
break;
+ case 'skipped':
+ this.dispatchTestEvent(state, item);
case 'started':
case 'passed':
- case 'skipped':
this.itemsToRun?.delete(item);
this.currentRun[state](item);
break;
case 'failed':
case 'errored':
+ if (dispatchBuildFailEvent) {
+ this.dispatchTestEvent(state, item);
+ }
this.itemsToRun?.delete(item);
this.currentRun[state](item, message || new TestMessage(""));
break;
}
+ this.suiteStates.set(item, state);
if (!noPassDown) {
- item.children.forEach(child => this.set(child, state, message, noPassDown));
+ item.children.forEach(child => this.set(child, state, message, noPassDown, dispatchBuildFailEvent));
}
}
}
+
+ dispatchTestEvent(state: SuiteState, testItem: TestItem): void {
+ if (testItem.parent && testItem.children.size > 0) {
+ this.dispatchEvent({
+ name: testItem.id,
+ moduleName: testItem.parent?.id,
Review Comment:
nitpick: unnecessary parent?. as parent was checked to be non-falsy above.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1866032258
##########
java/maven/src/org/netbeans/modules/maven/execute/AbstractOutputHandler.java:
##########
@@ -226,12 +232,16 @@ protected final void processEnd(String id, OutputWriter writer) {
writer.println(visitor.getLine());
}
}
- Set set = processors.get(id);
- if (set != null) {
- //TODO a bulletproof way would be to keep a list of currently started
- // sections and compare to the list of getRegisteredOutputSequences fo each of the
- // processors in set..
- currentProcessors.removeAll(set);
+ AtomicInteger count = id2count.getOrDefault(id, new AtomicInteger(1));
+ if (count.decrementAndGet() == 0) {
Review Comment:
Not sure, if the proposed solution works - `id2count.remove()` should be called only when `count` drops to zero.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869690511
##########
java/maven/nbproject/org-netbeans-modules-maven.sig:
##########
Review Comment:
Please remove the changes to this file. Sigfiles are generated after each release. Changes during delivery phase complicate that.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869195050
##########
java/maven/src/org/netbeans/modules/maven/ActionProviderImpl.java:
##########
@@ -132,6 +132,7 @@ public class ActionProviderImpl implements ActionProvider {
"javadoc", //NOI18N
COMMAND_TEST,
COMMAND_TEST_SINGLE,
+ COMMAND_TEST_PARALLEL,
Review Comment:
Just use current. They're automatically bumped between each NetBeans release.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1870947902
##########
java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java:
##########
@@ -227,7 +218,7 @@ private boolean isSurefireRunningInParallel() {
}
private static final String SECONDS_REGEX = "s(?:ec(?:ond)?(?:s|\\(s\\))?)?"; //NOI18N
- private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " - in (.*)";
+ private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " -+ in (.*)";
Review Comment:
Yes. `--` appears in the output.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2520198781
@mbien I tried with `mvnd` and it ran all tests successfully with this action
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869443748
##########
java/maven/src/org/netbeans/modules/maven/api/execute/RunConfig.java:
##########
@@ -79,6 +79,28 @@ public interface RunConfig {
String getActionName();
+ /**
+ * Options/switches passed to maven.
+ * @return a read-only copy of the current maven options
+ * @since 2.166
+ */
+ @NonNull Map<? extends String,? extends String> getOptions();
Review Comment:
I already added entry for `options`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2728727816
> Hi, any chance of reusing these changes to also support what #3995 was intended to do?
@matyasberry Hi, this PR should also add support for nested classes -> see [this](https://github.com/petrovic-d/netbeans/blob/40bae1af2279a038afbc2ce873ac0ffb7d5249d4/ide/projectapi/src/org/netbeans/spi/project/SingleMethod.java#L35)
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1865368830
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -198,19 +213,41 @@ export class NbTestAdapter {
} else {
this.set(currentSuite, suite.state, undefined, true);
}
+ this.set(currentModule, this.calculateStateFor(currentModule), undefined, true);
}
}
break;
}
}
+ calculateStateFor(testItem: TestItem): 'passed' | 'failed' | 'skipped' | 'errored' {
+ let passed: number = 0;
+ testItem.children.forEach(item => {
+ const state = this.suiteStates.get(item);
+ if (state === 'enqueued' || state === 'failed') return state;
+ if (state === 'passed') passed++;
+ })
+ if (passed > 0) return 'passed';
Review Comment:
This is used to calculate end state for the module node - https://github.com/apache/netbeans/blob/a3b539b8d2cd416f5ca54c800d6df2ca3bb590df/java/java.lsp.server/vscode/src/testAdapter.ts#L216, thats why there is no `started` state
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869054872
##########
ide/projectapi/src/org/netbeans/spi/project/ContainedProjectFilter.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.spi.project;
+
+import java.util.Collections;
+import java.util.List;
+import org.netbeans.api.project.Project;
+
+/**
+ *
Review Comment:
Pls. include some class overview and a description of an example usage (i.e. action acting on several projects may use this to filter). Should be noted that usage is optional and decided by individual actions and must be documented with the action.
I see that in the implementation an empty ContentProjectFilter is treated as no filter; hypotetically (since this is an API) - is there a way how to express that an action working with this selector should operate on the project itself, not any of its children ?
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -131,9 +132,15 @@ private void processTestProgress(TestProgressEvent evt) {
}
}
+ @NbBundle.Messages({
+ "ERR_Session_Null_Message=TestSession is null.",
+ })
private void processTestOutput(TestOutputEvent evt) {
TestSession session = sessions.get(getSessionKey(evt.getDescriptor()));
assert session != null;
+ if (session == null) {
+ throw new IllegalArgumentException(Bundle.ERR_Session_Null_Message());
Review Comment:
Nitpick: IMHO no need to localize exceptons :) they'll probably result in an general error; and users rarely read the logfile.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -154,7 +155,14 @@ private String statusToState(Status status) {
private static ModuleInfo getModuleInfo(TestSession session) {
Project project = session.getProject();
String moduleName = project != null ? ProjectUtils.getInformation(project).getDisplayName() : null;
- String modulePath = project != null ? project.getProjectDirectory().getPath() : null;
+ String modulePath = getModuleTestPath(project);
return new ModuleInfo(moduleName, modulePath);
}
+
+ private static String getModuleTestPath(Project project) {
+ if (project == null) {
+ return null;
+ }
+ return Paths.get(project.getProjectDirectory().getPath(), "src", "test", "java").toString();
Review Comment:
Not sure about this -- @dbalek: shouldn't we rather use `Sources` API to get java/test source group and its folder from the project ?
##########
ide/projectapi/src/org/netbeans/spi/project/ContainedProjectFilter.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.spi.project;
+
+import java.util.Collections;
+import java.util.List;
+import org.netbeans.api.project.Project;
+
+/**
+ *
+ * @author Dusan Petrovic
+ *
+ * @since 1.90
+ */
+public final class ContainedProjectFilter {
Review Comment:
IMHO this should be in the `api` package: it's client facing, part of the "request', not implementation (SPI) - provided or produced by implementations.
It's bad enough (an old sin) that clients access `spi.ActionProvider` directly ;) and not through an utility
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/ModuleInfo.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.lsp.server.progress;
+
+/**
+ *
+ * @author dusanpetrovic
+ */
+public class ModuleInfo {
Review Comment:
final ?
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -241,11 +259,38 @@ private void caseFinish(TestFinishEvent evt, JvmTestOperationDescriptor op) {
}
}
- runningTests.remove(getTestOpKey(op));
}
}
+ private static String GRADLE_TEST_RUN = "Gradle Test Run :";
Review Comment:
nitpick: make final :), `// NOI18N`
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java:
##########
@@ -76,6 +76,7 @@
import org.netbeans.modules.nativeimage.api.debug.StartDebugParameters;
import org.netbeans.spi.project.ActionProgress;
import org.netbeans.spi.project.ActionProvider;
+import org.netbeans.spi.project.ContainedProjectFilter;
Review Comment:
bump dependency version for project.api
##########
ide/projectapi/apichanges.xml:
##########
@@ -83,6 +83,18 @@ is the proper place.
<!-- ACTUAL CHANGES BEGIN HERE: -->
<changes>
+ <change id="contained-project-context">
Review Comment:
Pls mention ActionProvider's change as well.
##########
java/maven/src/org/netbeans/modules/maven/api/execute/RunConfig.java:
##########
@@ -79,6 +79,28 @@ public interface RunConfig {
String getActionName();
+ /**
+ * Options/switches passed to maven.
+ * @return a read-only copy of the current maven options
+ * @since 2.166
+ */
+ @NonNull Map<? extends String,? extends String> getOptions();
Review Comment:
Friend API change; probably add `apichanges.xml` entry for this + bump version of the module.
##########
java/maven/src/org/netbeans/modules/maven/ActionProviderImpl.java:
##########
@@ -132,6 +132,7 @@ public class ActionProviderImpl implements ActionProvider {
"javadoc", //NOI18N
COMMAND_TEST,
COMMAND_TEST_SINGLE,
+ COMMAND_TEST_PARALLEL,
Review Comment:
bump dependency version on project.api
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -164,7 +171,7 @@ private void sessionStart(TestStartEvent evt) {
}
}
- private void sessionFinish(TestFinishEvent evt) {
+ private synchronized void sessionFinish(TestFinishEvent evt) {
Review Comment:
I briefly checked the implementation of CoreManager.sessionFInished() called below, and it seems that the impl uses Mutex.EVENT.writeAccess that blocks until the action is executed in EDT thread. This `synchronized` lock will be held for the whole time, incl. waiting on EDT; may lead to deadlock eventually. Same for other CoreManager etc calls that may fire events to unknown listeners or synchronize with other threads.
##########
ide/projectapi/src/org/netbeans/spi/project/ActionProvider.java:
##########
@@ -83,6 +83,11 @@ public interface ActionProvider {
*/
String COMMAND_TEST_SINGLE = "test.single"; // NOI18N
+ /**
+ * Standard command for running tests in parallel on given projects sub-modules
+ */
+ String COMMAND_TEST_PARALLEL = "test.parallel"; // NOI18N
Review Comment:
`@since` missing
##########
java/maven/src/org/netbeans/modules/maven/execute/MavenCommandLineExecutor.java:
##########
@@ -446,21 +451,24 @@ private static List<String> createMavenExecutionCommand(RunConfig config, Constr
}
}
+ //#164234
+ //if maven.bat file is in space containing path, we need to quote with simple quotes.
+ String quote = "\"";
+
for (Map.Entry<? extends String, ? extends String> entry : config.getOptions().entrySet()) {
String key = entry.getKey();
String value = quote2apos(entry.getValue());
- if (MavenCommandLineOptions.optionRequiresValue(key) && (value.isBlank() || value.equals("${" + key + "}"))) { //NOI18N
+ if (MavenCommandLineOptions.optionRequiresValue(key) && (value.isEmpty() || value.equals("${" + key + "}"))) { //NOI18N
+ DialogDisplayer.getDefault().notify(new NotifyDescriptor.Message(Bundle.MSG_MissingValue(key), NotifyDescriptor.WARNING_MESSAGE));
Review Comment:
`notify` will suspend the caller (launching process) until the user confirms / closes the message. Consider using `notifyLater` if that's not the intent.
##########
java/gradle.java/src/org/netbeans/modules/gradle/java/JavaActionProvider.java:
##########
@@ -72,6 +72,7 @@ public class JavaActionProvider extends DefaultGradleActionsProvider {
COMMAND_RUN,
COMMAND_DEBUG,
COMMAND_TEST,
+ COMMAND_TEST_PARALLEL,
Review Comment:
check the project's dependency on `project.api`, make sure it requests 1.90 where the symbol is added. Increase this module's spec version (feature added).
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/WorkspaceServiceImpl.java:
##########
@@ -805,6 +808,13 @@ public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
throw new UnsupportedOperationException("Command not supported: " + params.getCommand());
}
+ private String getModuleTestPath(Project project) {
+ if (project == null) {
+ return null;
+ }
+ return Paths.get(project.getProjectDirectory().getPath(), "src", "test", "java").toString();
Review Comment:
similar for the usage of Source API - whatever decision is taken.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -144,4 +149,20 @@ private String statusToState(Status status) {
throw new IllegalStateException("Unexpected testsuite status: " + status);
}
}
+
+ private static Pattern PATTERN = Pattern.compile("Gradle Test Run :(.*):test");
Review Comment:
make final, // NOI18N
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1869690511
##########
java/maven/nbproject/org-netbeans-modules-maven.sig:
##########
Review Comment:
Please remove the changes to this file. Sigfiles are generated after each release. Changes during delivery phase complicate that. See pending PR at #7896
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1889738561
##########
java/java.source.base/src/org/netbeans/api/java/source/SourceUtils.java:
##########
@@ -1477,10 +1478,11 @@ public static void forceSource(CompilationController cc, FileObject file) {
*
* @param info the ClasspathInfo used to resolve
* @param relativePath input source file path relative to the corresponding source root
+ * @param nestedClass nested class which name is searched
* @return class name for the corresponding input source file
* @since 2.73
*/
- public static String classNameFor(ClasspathInfo info, String relativePath) {
+ public static String classNameFor(ClasspathInfo info, String relativePath, NestedClass nestedClass) {
Review Comment:
Sorry I missed this one. Paperwork for this API addition is missing - `apichanges.xml` entry + update version for dependency on `java.source.base' in maven + gradle modules which depend on this new API
##########
java/maven/src/org/netbeans/modules/maven/api/execute/RunConfig.java:
##########
@@ -79,6 +79,28 @@ public interface RunConfig {
String getActionName();
+ /**
+ * Options/switches passed to maven.
+ * @return a read-only copy of the current maven options
+ * @since 2.166
Review Comment:
The current master's version of java/maven is 2.166 as well, this should be increased, 2.166 is over 2 months old already.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -144,4 +150,27 @@ private String statusToState(Status status) {
throw new IllegalStateException("Unexpected testsuite status: " + status);
}
}
+
+ private static final Pattern PATTERN = Pattern.compile("Gradle Test Run :(.*):test"); // NOI18N
Review Comment:
Seems unused.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1870199788
##########
java/maven/src/org/netbeans/modules/maven/api/execute/RunConfig.java:
##########
@@ -79,6 +79,28 @@ public interface RunConfig {
String getActionName();
+ /**
+ * Options/switches passed to maven.
+ * @return a read-only copy of the current maven options
+ * @since 2.166
+ */
+ @NonNull Map<? extends String,? extends String> getOptions();
Review Comment:
Sorry, I missed it in the diff.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1870778076
##########
java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java:
##########
@@ -227,7 +218,7 @@ private boolean isSurefireRunningInParallel() {
}
private static final String SECONDS_REGEX = "s(?:ec(?:ond)?(?:s|\\(s\\))?)?"; //NOI18N
- private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " - in (.*)";
+ private static final String TESTSUITE_STATS_REGEX = "Tests run: +([0-9]+), +Failures: +([0-9]+), +Errors: +([0-9]+), +Skipped: +([0-9]+), +Time elapsed: +(.+)" + SECONDS_REGEX + " -+ in (.*)";
Review Comment:
was the `+` intended?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2519239062
@neilcsmith-net please advise how to fix build failure https://github.com/apache/netbeans/actions/runs/12162766052/job/33936238338?pr=7979 related to signature check. There's a (formally) incompatible change on friend-only API.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2539169487
@petrovic-d follow #8034
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1865843917
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -241,11 +252,38 @@ private void caseFinish(TestFinishEvent evt, JvmTestOperationDescriptor op) {
}
}
- runningTests.remove(getTestOpKey(op));
}
}
+ private static String GRADLE_TEST_RUN = "Gradle Test Run :";
+ private static String TEST = ":test";
+
+ private Project getProject(String key) {
+ if (key != null && key.startsWith(GRADLE_TEST_RUN)) {
+ key = key.substring(GRADLE_TEST_RUN.length());
+ if (key.endsWith(TEST)) {
+ key = key.substring(0, key.length() - TEST.length()).trim();
+ if (!key.isEmpty()) {
+ for (Project containedPrj : ProjectUtils.getContainedProjects(project, false)) {
+ if (key.equals(containedPrj.getProjectDirectory().getName())) {
+ return containedPrj;
+ }
+ }
+ }
+ }
+ }
+ return project;
+ }
+
+ private static String getSessionKey(OperationDescriptor op) {
+ String id = "";
+ for (OperationDescriptor descriptor = op; descriptor != null; descriptor = descriptor.getParent()) {
+ id = descriptor.getName();
Review Comment:
IIRC- yes
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1866032258
##########
java/maven/src/org/netbeans/modules/maven/execute/AbstractOutputHandler.java:
##########
@@ -226,12 +232,16 @@ protected final void processEnd(String id, OutputWriter writer) {
writer.println(visitor.getLine());
}
}
- Set set = processors.get(id);
- if (set != null) {
- //TODO a bulletproof way would be to keep a list of currently started
- // sections and compare to the list of getRegisteredOutputSequences fo each of the
- // processors in set..
- currentProcessors.removeAll(set);
+ AtomicInteger count = id2count.getOrDefault(id, new AtomicInteger(1));
+ if (count.decrementAndGet() == 0) {
Review Comment:
Not sure, whether the proposed solution works - `id2count.remove()` should be called only when `count` drops to zero.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1889958127
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -63,53 +87,122 @@ export class NbTestAdapter {
for (let item of include) {
if (item.uri) {
this.set(item, 'enqueued');
+ item.parent?.children.forEach(child => {
+ if (child.id?.includes(item.id)) {
+ this.set(child, 'enqueued');
+ }
+ })
const idx = item.id.indexOf(':');
+ const nestedClassIdx = item.id.indexOf('$');
+ let nestedClass: string | undefined;
+ if (nestedClassIdx > 0) {
+ nestedClass = idx < 0
+ ? item.id.slice(nestedClassIdx + 1)
+ : item.id.substring(nestedClassIdx + 1, idx);
+ }
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1));
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.single' : COMMAND_PREFIX + '.run.single', item.uri.toString(), idx < 0 ? undefined : item.id.slice(idx + 1), nestedClass);
}
}
}
} else {
this.testController.items.forEach(item => this.set(item, 'enqueued'));
for (let workspaceFolder of workspace.workspaceFolders || []) {
if (!cancellation.isCancellationRequested) {
- await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ if (testInParallel) {
+ await commands.executeCommand(COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString(), undefined, undefined, undefined, true, projects);
+ } else {
+ await commands.executeCommand(request.profile?.kind === TestRunProfileKind.Debug ? COMMAND_PREFIX + '.debug.test': COMMAND_PREFIX + '.run.test', workspaceFolder.uri.toString());
+ }
}
}
}
if (this.started) {
this.itemsToRun.forEach(item => this.set(item, 'skipped'));
+ }
+ // TBD - message
Review Comment:
Yes. This case will happen when the build fails - tests are not even started. We need to decide what will be the message, or to somehow parse output from the console
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic merged PR #7979:
URL: https://github.com/apache/netbeans/pull/7979
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1872998151
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -154,7 +155,14 @@ private String statusToState(Status status) {
private static ModuleInfo getModuleInfo(TestSession session) {
Project project = session.getProject();
String moduleName = project != null ? ProjectUtils.getInformation(project).getDisplayName() : null;
- String modulePath = project != null ? project.getProjectDirectory().getPath() : null;
+ String modulePath = getModuleTestPath(project);
return new ModuleInfo(moduleName, modulePath);
}
+
+ private static String getModuleTestPath(Project project) {
+ if (project == null) {
+ return null;
+ }
+ return Paths.get(project.getProjectDirectory().getPath(), "src", "test", "java").toString();
Review Comment:
@lahodaj This is not visible in the UI. The purpose of it is to run tests for a single module (at least that's the idea). This way we know the path to the module tests on the client side. It's a bit coupled to this [PR](https://github.com/apache/netbeans/pull/8021) which fixes single test runs
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2539269944
sig pr merged
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#issuecomment-2539161797
@ebarboni When can we expect sigs to be generated for the release, since this PR should (probably) introduce changes to .sig?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "petrovic-d (via GitHub)" <gi...@apache.org>.
petrovic-d commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1889895537
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/WorkspaceServiceImpl.java:
##########
@@ -401,11 +403,15 @@ public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
LOG.log(Level.INFO, "Project {2}: {0} test roots opened in {1}ms", new Object[] { testRoots.size(), (System.currentTimeMillis() - t), file});
BiFunction<FileObject, Collection<TestMethodController.TestMethod>, Collection<TestSuiteInfo>> f = (fo, methods) -> {
String url = Utils.toUri(fo);
+ Project owner = FileOwnerQuery.getOwner(fo);
+ String moduleName = owner != null ? ProjectUtils.getInformation(owner).getDisplayName(): null;
Review Comment:
Yes. For the maven we will have `parentName-artifactId`, and for the gradle it's `parentName:moduleName`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1863420851
##########
extide/gradle/src/org/netbeans/modules/gradle/ActionProviderImpl.java:
##########
@@ -215,6 +214,9 @@ static String taskName(Project project, String action, Lookup lkp) {
case ActionProvider.COMMAND_TEST:
title = TXT_Test(prjLabel);
break;
+ case ActionProvider.COMMAND_TEST_PARALLEL:
+ title = TXT_Test(prjLabel);
Review Comment:
Different label from COMMAND_TEST, please
##########
java/java.lsp.server/nbcode/integration/src/org/netbeans/modules/nbcode/integration/maven-actions-override.xml:
##########
@@ -55,7 +55,23 @@
<test>${packageClassName}</test>
</properties>
</action>
-
+ <action>
Review Comment:
Is there a reason the action mapping is defined in the NBLS `integration` module, while the action's implementation iself is in `java/maven` standard distribution ?
##########
java/maven/src/org/netbeans/modules/maven/execute/MavenCommandLineExecutor.java:
##########
@@ -445,6 +445,18 @@ private static List<String> createMavenExecutionCommand(RunConfig config, Constr
LOGGER.log(Level.FINE, "Could not canonicalize " + basedir, x);
}
}
+
+ for (Map.Entry<? extends String, ? extends String> entry : config.getOptions().entrySet()) {
+ String key = entry.getKey();
+ String value = quote2apos(entry.getValue());
+ if (MavenCommandLineOptions.optionRequiresValue(key) && (value.isBlank() || value.equals("${" + key + "}"))) { //NOI18N
Review Comment:
why the check for `${optionName}` ?
Consider to print a warning visibly to the user: if trying to pass no value for an option that is known to have one, it's a bug somewhere in the code.
Maybe I am picky, but `isBlank()` is true also for " " (two spaces); I wouldn't equal that to a no-value. Consider value quoting (see proprerties below).
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java:
##########
@@ -374,6 +377,9 @@ private static ExplicitProcessParameters createExplicitProcessParameters(Map<Str
if (!env.isEmpty()) {
bld.environmentVariables(env);
}
+ if (!projects.isEmpty()) {
Review Comment:
So, the goal here is to pass list of projects the **project action** should apply to, rather than configure the launched application; the rest of ExplicitProcessParameters is used to setup environment for the executed VM.
We do not have much support for "umbrella" actions in container projects at this moment; so it's maybe better to introduce such explicit support at `projects.api` level, given that the concept of `ProjectContainerProvider` is here.
A simple bean like
```
public final class ContainedProjectFilter {
private List<Project> projectsToProcess;
}
```
would be sufficient, and usable (possibly!) for other actions. I thought about an `Predicate<Project>`, but the caller can inspect the project structure in advance and generate explicit information which is easier to process during build.
The bean should be final with a nonpublic constructor + static factory, so that it can be easily extended in the future (e.g. with a Builder).
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -131,17 +132,19 @@ private void processTestProgress(TestProgressEvent evt) {
}
private void processTestOutput(TestOutputEvent evt) {
+ TestSession session = sessions.get(getSessionKey(evt.getDescriptor()));
+ assert session != null;
Review Comment:
Maybe fail with IAE even in production mode if `session == null`; otherwise invent a way how the output will be displayed and/or reported even in that case.
The original code would IMHO fail if sessionStart wouldn't be run first.
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -241,11 +252,38 @@ private void caseFinish(TestFinishEvent evt, JvmTestOperationDescriptor op) {
}
}
- runningTests.remove(getTestOpKey(op));
}
}
+ private static String GRADLE_TEST_RUN = "Gradle Test Run :";
+ private static String TEST = ":test";
+
+ private Project getProject(String key) {
+ if (key != null && key.startsWith(GRADLE_TEST_RUN)) {
+ key = key.substring(GRADLE_TEST_RUN.length());
+ if (key.endsWith(TEST)) {
+ key = key.substring(0, key.length() - TEST.length()).trim();
+ if (!key.isEmpty()) {
+ for (Project containedPrj : ProjectUtils.getContainedProjects(project, false)) {
+ if (key.equals(containedPrj.getProjectDirectory().getName())) {
+ return containedPrj;
+ }
+ }
+ }
+ }
+ }
+ return project;
+ }
+
+ private static String getSessionKey(OperationDescriptor op) {
+ String id = "";
+ for (OperationDescriptor descriptor = op; descriptor != null; descriptor = descriptor.getParent()) {
+ id = descriptor.getName();
Review Comment:
Just checking: Is session key the topmost (root) operation's name ?
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -151,8 +154,9 @@ private void processTestOutput(TestOutputEvent evt) {
private void sessionStart(TestStartEvent evt) {
- session = new TestSession(evt.getDisplayName(), project, TestSession.SessionType.TEST);
- runningTests.clear();
+ String key = getSessionKey(evt.getDescriptor());
Review Comment:
Please check that the event delivery dispatched into `*Start / *Stop` methods is serialized, there are now 2 Maps (sessions + runningTests 1.st level) that work together and their mutations are not atomic. Otherwise these two may need synchronizing.
##########
ide/extexecution.base/src/org/netbeans/api/extexecution/base/ExplicitProcessParameters.java:
##########
@@ -146,6 +148,16 @@ public boolean isEmpty() {
public List<String> getArguments() {
return arguments;
}
+
+ /**
+ * Returns the projects to be passed. Returns {@code null} if the object does not
+ * want to alter the project list.
+ * @return projects to be passed or {@code null} if the project list should not be altered.
+ */
+ public List<String> getProjects() {
Review Comment:
Sorry, this is not acceptable. `extexecution.base` is project-agnostic and should remain that way. This whole support API arguments for a final launcher, VM or executable that is to be run "somehow", maybe through a project action or a build system, but also independently.
This PR is not trying to pass projects to the user application, its VM or a potential wrapping launcher, but rather to the build system; it does not affect the user's VM/launch setup.
##########
java/gradle.test/src/org/netbeans/modules/gradle/test/GradleTestProgressListener.java:
##########
@@ -241,11 +252,38 @@ private void caseFinish(TestFinishEvent evt, JvmTestOperationDescriptor op) {
}
}
- runningTests.remove(getTestOpKey(op));
}
}
+ private static String GRADLE_TEST_RUN = "Gradle Test Run :";
+ private static String TEST = ":test";
+
+ private Project getProject(String key) {
+ if (key != null && key.startsWith(GRADLE_TEST_RUN)) {
+ key = key.substring(GRADLE_TEST_RUN.length());
+ if (key.endsWith(TEST)) {
+ key = key.substring(0, key.length() - TEST.length()).trim();
+ if (!key.isEmpty()) {
+ for (Project containedPrj : ProjectUtils.getContainedProjects(project, false)) {
Review Comment:
is it possible that toplevel `test` executes tests on 3rd level projects as well ? In that case, you might need contained projects recursively.
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java:
##########
@@ -144,4 +148,13 @@ private String statusToState(Status status) {
throw new IllegalStateException("Unexpected testsuite status: " + status);
}
}
+
+ private static Pattern PATTERN = Pattern.compile("Gradle Test Run :(.*):test");
+
+ private static ModuleInfo getModuleInfo(TestSession session) {
+ Project project = session.getProject();
+ String moduleName = project != null ? ProjectUtils.getInformation(project).getDisplayName() : null;
+ String modulePath = project != null ? project.getProjectDirectory().getPath() : null;
Review Comment:
Not sure about this; subproject information is usually relative to the root project. Is it desirable to have full FS path here ?
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java:
##########
@@ -568,7 +574,8 @@ public void finished(boolean success) {
actions = debug ? mainSource ? new String[] {ActionProvider.COMMAND_DEBUG_SINGLE}
: new String[] {ActionProvider.COMMAND_DEBUG_TEST_SINGLE, ActionProvider.COMMAND_DEBUG_SINGLE}
: mainSource ? new String[] {ActionProvider.COMMAND_RUN_SINGLE}
- : new String[] {ActionProvider.COMMAND_TEST_SINGLE, ActionProvider.COMMAND_RUN_SINGLE};
+ : testInParallel ? new String[] {ActionProvider.COMMAND_TEST_PARALLEL, ActionProvider.COMMAND_TEST}
Review Comment:
Why the difference if `testInParallel` ?
COMMAND_TEST_**SINGLE** vs. COMMAND_TEST_PARALLEL (a clone of COMMAND_TEST not *single* actually) and COMMAND_**TEST** vs COMMAND_**RUN_SINGLE** ?
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -265,14 +302,42 @@ export class NbTestAdapter {
});
} else {
currentSuite.children.replace(children);
+ currentModule.children.replace(suiteChildren);
}
}
+ getModuleName(suite: TestSuite): string {
+ return suite.moduleName?.replace(":", "-") || "";
+ }
+
+ getModulePath(suite: TestSuite): Uri {
+ const basePath = Uri.parse(suite.modulePath || "");
+ return Uri.joinPath(basePath, "src", "test", "java");
Review Comment:
I woudn't hardcode any path inside a project, better have NBLS to report the path to the LS client.
##########
java/java.lsp.server/vscode/src/testAdapter.ts:
##########
@@ -198,19 +213,41 @@ export class NbTestAdapter {
} else {
this.set(currentSuite, suite.state, undefined, true);
}
+ this.set(currentModule, this.calculateStateFor(currentModule), undefined, true);
}
}
break;
}
}
+ calculateStateFor(testItem: TestItem): 'passed' | 'failed' | 'skipped' | 'errored' {
+ let passed: number = 0;
+ testItem.children.forEach(item => {
+ const state = this.suiteStates.get(item);
+ if (state === 'enqueued' || state === 'failed') return state;
+ if (state === 'passed') passed++;
+ })
+ if (passed > 0) return 'passed';
Review Comment:
what about `started` state ? Shouldn't the module indicate somehow it is still running (although still passing, or already failed) ?
##########
java/maven/src/org/netbeans/modules/maven/api/execute/RunConfig.java:
##########
@@ -79,6 +79,17 @@ public interface RunConfig {
String getActionName();
+ /**
+ * Options/switches passed to maven.
+ * @return a read-only copy of the current maven options
+ * @since 2.166
+ */
+ @NonNull Map<? extends String,? extends String> getOptions();
+
+ void setOptions(@NonNull String key, @NullAllowed String value);
Review Comment:
`setOptio`n`` or `addOption` :) no multple key-values here.
Pls. document all added methods.
##########
java/maven/src/org/netbeans/modules/maven/ActionProviderImpl.java:
##########
@@ -457,6 +459,8 @@ private static void setupTaskName(String action, RunConfig config, Lookup lkp) {
title = TXT_Profile(prjLabel);
} else if (ActionProvider.COMMAND_TEST.equals(action)) {
title = TXT_Test(prjLabel);
+ } else if (ActionProvider.COMMAND_TEST_PARALLEL.equals(action)) {
+ title = TXT_Test(prjLabel);
Review Comment:
Different title from COMMAND_TEST
##########
java/maven/src/org/netbeans/modules/maven/execute/AbstractOutputHandler.java:
##########
@@ -226,12 +232,16 @@ protected final void processEnd(String id, OutputWriter writer) {
writer.println(visitor.getLine());
}
}
- Set set = processors.get(id);
- if (set != null) {
- //TODO a bulletproof way would be to keep a list of currently started
- // sections and compare to the list of getRegisteredOutputSequences fo each of the
- // processors in set..
- currentProcessors.removeAll(set);
+ AtomicInteger count = id2count.getOrDefault(id, new AtomicInteger(1));
+ if (count.decrementAndGet() == 0) {
Review Comment:
nitpick: add+remove AtomicInteger to the map on missing `id`; consider using just `count = id2count.remove();` then `count == null || count.decrementAndGet())` as test.
##########
java/maven/src/org/netbeans/modules/maven/execute/MavenCommandLineOptions.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.maven.execute;
+
+import java.util.List;
+
+/**
+ *
+ * @author Dusan Petrovic
+ */
+public class MavenCommandLineOptions {
+
+ private static final List<String> OPTIONS_WITH_VALUES = List.of(
+ "file",
Review Comment:
Also
- builder
- color
- define
- global-toolchains
Pls.order alphabetically in source, it is easier to find an option when reading :) I'd use `Set.of`, faster `contains` lookups.
##########
java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java:
##########
@@ -632,25 +624,21 @@ static Trouble constructTrouble(@NonNull String type, @NullAllowed String messag
}
private void generateTest() {
- String suffix = reportNameSuffix;
- if (suffix == null) {
- suffix = "";
- } else {
- //#204480
- suffix = "-" + suffix;
- }
- File report = new File(outputDir, "TEST-" + runningTestClass + suffix + ".xml");
- if (!report.isFile() || report.lastModified() < startTimeStamp) { //#219097 ignore results from previous invokation.
- if(surefireRunningInParallel) { // try waiting a bit to give time for the result file to be created
- try {
- Thread.sleep(500);
- } catch (InterruptedException ex) {
- Exceptions.printStackTrace(ex);
- }
- }
- if (!report.isFile() || report.lastModified() < startTimeStamp) { // and now try again
- return;
+ String suffix = reportNameSuffix == null ? "" : "-" + reportNameSuffix;
+ File outputDir = locateOutputDir(suffix);
+ if (outputDir == null && surefireRunningInParallel) {
+ // try waiting a bit to give time for the result file to be created
Review Comment:
Maybe there could be a warning message to the user if the output is not created even after the wait.
##########
java/maven/src/org/netbeans/modules/maven/execute/cmd/ExecMojo.java:
##########
@@ -155,4 +169,24 @@ public String getImplementationClass() {
void setImplementationClass(String implementationClass) {
this.implementationClass = implementationClass;
}
+
+ public @CheckForNull Project findProject() {
+ if (currentProjectLocation != null) {
+ FileObject fo = FileUtil.toFileObject(currentProjectLocation);
+ if (fo != null) {
+ try {
+ return ProjectManager.getDefault().findProject(fo);
+ } catch (IOException ex) {
+ //Exceptions.printStackTrace(ex);
Review Comment:
Why this should not be logged ?
##########
java/maven/src/org/netbeans/modules/maven/execute/model/NetbeansActionMapping.java:
##########
@@ -264,6 +290,16 @@ public void removePackaging(String string)
getPackagings().remove( string );
} //-- void removePackaging(String)
+ /**
+ * Method removeOptions.
+ *
+ * @param string
+ */
+ public void removeOptions(String string)
+ {
Review Comment:
singular `removeOption` ?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Action for running tests in parallel [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7979:
URL: https://github.com/apache/netbeans/pull/7979#discussion_r1863452805
##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java:
##########
@@ -374,6 +377,9 @@ private static ExplicitProcessParameters createExplicitProcessParameters(Map<Str
if (!env.isEmpty()) {
bld.environmentVariables(env);
}
+ if (!projects.isEmpty()) {
Review Comment:
So, the goal here is to pass list of projects the **project action** should apply to, rather than configure the launched application; the rest of `ExplicitProcessParameters` is used to setup environment for the executed VM.
We do not have much support for "umbrella" actions in container projects at this moment; so it's maybe better to introduce such explicit support in `projects.api` module, given that the concept of `ProjectContainerProvider` is here.
A simple bean like
```
public final class ContainedProjectFilter {
private List<Project> projectsToProcess;
}
```
would be sufficient, and usable (possibly!) for other actions. I thought about an `Predicate<Project>`, but the caller can inspect the project structure in advance and generate explicit information which is easier to process during build.
The bean should be final with a nonpublic constructor + static factory, so that it can be easily extended in the future (e.g. with a Builder).
When changing an API, bump version, record the change in `apichanges.xml`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists