You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2021/02/18 15:31:08 UTC
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2766: Support for visualizing test results in VSCode extension over LSP.
JaroslavTulach commented on a change in pull request #2766:
URL: https://github.com/apache/netbeans/pull/2766#discussion_r578483440
##########
File path: ide/gsf.testrunner.ui/src/org/netbeans/modules/gsf/testrunner/ui/api/TestResultDisplayHandler.java
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.gsf.testrunner.ui.api;
+
+import org.netbeans.modules.gsf.testrunner.api.Report;
+import org.netbeans.modules.gsf.testrunner.api.TestSession;
+import org.netbeans.modules.gsf.testrunner.api.TestSuite;
+import org.netbeans.modules.gsf.testrunner.ui.ResultDisplayHandler;
+import org.openide.util.Lookup;
+
+/**
+ * Common base class for handlers displaying test results.
+ *
+ * @since 1.22
+ * @author Dusan Balek
+ */
+public abstract class TestResultDisplayHandler {
+
+ /**
+ * Get the {@link TestResultDisplayHandler} for the test session.
+ *
+ * @param session test session
+ * @return {@link TestResultDisplayHandler} instance
+ */
+ public static final TestResultDisplayHandler get(TestSession session) {
Review comment:
How this method is supposed to behave when it is called multiple times?
##########
File path: ide/gsf.testrunner/manifest.mf
##########
@@ -3,5 +3,5 @@ AutoUpdate-Show-In-Client: false
OpenIDE-Module: org.netbeans.modules.gsf.testrunner/2
OpenIDE-Module-Localizing-Bundle: org/netbeans/modules/gsf/testrunner/Bundle.properties
OpenIDE-Module-Layer: org/netbeans/modules/gsf/testrunner/layer.xml
-OpenIDE-Module-Specification-Version: 2.21
+OpenIDE-Module-Specification-Version: 2.22
Review comment:
Please make a note in the `apichanges.xml` file.
##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/Server.java
##########
@@ -452,6 +451,11 @@ public void showStatusBarMessage(ShowStatusMessageParams params) {
return CompletableFuture.completedFuture(params.getValue());
}
+ @Override
+ public void notifyTestProgress(TestProgressParams params) {
+ logWarning(params);
+ }
+
@Override
public NbCodeClientCapabilities getNbCodeCapabilities() {
Review comment:
Shouldn't the additional messages in the LSP protocol be reflected in the capabilities? CCing @sdedic ...
##########
File path: ide/gsf.testrunner.ui/src/org/netbeans/modules/gsf/testrunner/ui/api/TestResultDisplayHandler.java
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.gsf.testrunner.ui.api;
+
+import org.netbeans.modules.gsf.testrunner.api.Report;
+import org.netbeans.modules.gsf.testrunner.api.TestSession;
+import org.netbeans.modules.gsf.testrunner.api.TestSuite;
+import org.netbeans.modules.gsf.testrunner.ui.ResultDisplayHandler;
+import org.openide.util.Lookup;
+
+/**
+ * Common base class for handlers displaying test results.
+ *
+ * @since 1.22
+ * @author Dusan Balek
+ */
+public abstract class TestResultDisplayHandler {
+
+ /**
+ * Get the {@link TestResultDisplayHandler} for the test session.
+ *
+ * @param session test session
+ * @return {@link TestResultDisplayHandler} instance
+ */
+ public static final TestResultDisplayHandler get(TestSession session) {
+ TestResultDisplayHandler.Provider provider = Lookup.getDefault().lookup(TestResultDisplayHandler.Provider.class);
+ if (provider != null) {
+ return provider.create(session);
+ }
+ return new ResultDisplayHandler(session);
+ }
+
+ public abstract void displayOutput(String text, boolean error);
+
+ /**
+ * Display information that a test suite is running.
+ *
+ * @param suiteName name of the running suite; or {@code null} in the case
+ * of anonymous suite
+ */
+ public abstract void displaySuiteRunning(String suiteName);
+
+ /**
+ * Display information that a test suite is running.
+ *
+ * @param suite the running suite
+ */
+ public abstract void displaySuiteRunning(TestSuite suite);
+
+ /**
+ * Display test results.
+ *
+ * @param report summary report to display
+ */
+ public abstract void displayReport(Report report);
+
+ /**
+ * Display message produced by running test.
+ *
+ * @param message message to display
+ */
+ public abstract void displayMessage(String message);
+
+ /**
+ * Display information that a test session has finished.
+ *
+ * @param message message to display
+ */
+ public abstract void displayMessageSessionFinished(String message);
+
+ /**
+ * Return total number of tests in session if known.
+ *
+ * @return number of tests
+ */
+ public abstract int getTotalTests();
+
+ /**
+ * Interface providing factory method for creating {@link TestResultDisplayHandler}s.
+ * Instances should be registered in the default lookup.
+ */
+ public static interface Provider {
Review comment:
How do you plan to evolve these two types?
I'd suggest to use [singletonizer pattern](http://wiki.apidesign.org/wiki/Singletonizer). E.g.
* make `TestResultDisplayHandler` constructor package private
* add package private `class Impl<T> extends TestResultDisplayHandler` that will have two fields
* `final T data` and `final Provider<T> provider`
* Add parameter `<T>` to `Provider` and let the `create` method `T`
* for each method in `TestResultDisplayHandler` method create the same method in `Provider` with first parameter being `T`.
Here is a simplified example:
```java
public static interface Provider<T> {
T create(TestSession session);
int getTotalTests(T data);
}
```
the `Impl` class should then have
```java
class Impl<T> extends TestResultDisplayHandler {
private final T data;
private final Provider<T> provider;
public int getTotalTests() { return provider.getTotalTests(data); }
static <T> Impl create(Provider<T> p, TestSession s) {
return new Impl(p, p.create(s));
}
}
```
the `get` method in the `TestResultDisplayHandler` would then become:
```java
public static final TestResultDisplayHandler get(TestSession session) {
TestResultDisplayHandler.Provider provider = Lookup.getDefault().lookup(TestResultDisplayHandler.Provider.class);
if (provider != null) {
return Impl.create(provider, session);
}
return Impl.create(new ResultDisplayHandler());
}
```
Evolution story would then be:
- add new method into the `TestResultDisplayHandler` (safe as the class isn't publicly subclassable)
- add another `Provider2 extends Provider` SPI interface with the new method(s)
##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/progress/TestProgressHandler.java
##########
@@ -0,0 +1,140 @@
+/*
+ * 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;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.netbeans.api.extexecution.print.LineConvertors;
+import org.netbeans.modules.gsf.testrunner.api.Report;
+import org.netbeans.modules.gsf.testrunner.api.TestSession;
+import org.netbeans.modules.gsf.testrunner.api.TestSuite;
+import org.netbeans.modules.gsf.testrunner.ui.api.TestResultDisplayHandler;
+import org.netbeans.modules.java.lsp.server.Utils;
+import org.netbeans.modules.java.lsp.server.protocol.NbCodeLanguageClient;
+import org.netbeans.modules.java.lsp.server.protocol.TestProgressParams;
+import org.netbeans.modules.java.lsp.server.protocol.TestSuiteInfo;
+import org.openide.filesystems.FileObject;
+
+/**
+ *
+ * @author Dusan Balek
+ */
+public final class TestProgressHandler extends TestResultDisplayHandler implements TestResultDisplayHandler.Provider {
Review comment:
If the _singletonizer_ idea is implemented, this could become `class TestProgressHandler implements TestResultDisplayHandler.Provider<TestProgressHandler>`, but it is a bit strange that `create` method returns `this`! Why do we have `Provider` factory pattern then?
##########
File path: ide/gsf.testrunner/nbproject/project.xml
##########
@@ -79,6 +79,7 @@
<friend>org.netbeans.modules.groovy.support</friend>
<friend>org.netbeans.modules.gsf.testrunner.ui</friend>
<friend>org.netbeans.modules.hudson.ui</friend>
+ <friend>org.netbeans.modules.java.lsp.server</friend>
Review comment:
I suggest to open up the API to public rather than adding yet another friend.
----------------------------------------------------------------
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.
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