[GitHub] [netbeans] eirikbakke commented on a change in pull request #3193: [NETBEANS-6065] Make Gradle Project test close connections to Gradle Daemon

[GitBox <gi...@apache.org>]

eirikbakke commented on a change in pull request #3193:
URL: https://github.com/apache/netbeans/pull/3193#discussion_r716229311



##########
File path: extide/gradle/test/unit/src/org/netbeans/modules/gradle/NbGradleProjectImplTest.java
##########
@@ -49,6 +52,25 @@ public NbGradleProjectImplTest(String name) {
     }
     
     private FileObject projectDir;
+    private Project prj;
+    
+    @Before
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        prj = createProject();
+    }
+    
+    @After
+    @Override
+    public void tearDown() throws Exception {
+        ProjectConnection pconn = prj.getLookup().lookup(ProjectConnection.class);
+        if (pconn instanceof GradleProjectConnection) {
+            GradleProjectConnection gpconn = (GradleProjectConnection) pconn;
+            gpconn.close();
+        }
+        super.tearDown();

Review comment:
       Might be good to set prj = null for symmetry with setUp. But shouldn't matter.

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/GradleProjectConnection.java
##########
@@ -111,6 +111,10 @@ public synchronized void close() {
         compatConn = null;
     }
 
+    boolean hasConnection() {

Review comment:
       If this class is meant to be thread-safe, which seems to be the case, then this method should be synchronized. Though it probably doesn't matter for the curent use, which is just for the single test class.

##########
File path: extide/gradle/test/unit/src/org/netbeans/modules/gradle/NbGradleProjectImplTest.java
##########
@@ -220,18 +244,18 @@ public void testIncreaseAimedQualityChangesProject() throws Exception {
      * the evaluated state does not exist.
      */
     public void testEvaluateTrustedDoesNotExecuteScript() throws Exception {
-        Project prj = createProject();
         NbGradleProject ngp = NbGradleProject.get(prj);
         ProjectTrust.getDefault().trustProject(prj);
         // initializes the project
         assertTrue(ngp.getQuality().worseThan(NbGradleProject.Quality.EVALUATED));
-        
+        assertHasNoConnection(prj);
         NbGradleProjectImpl prjImpl = prj.getLookup().lookup(NbGradleProjectImpl.class);
         GradleProject curProject = prjImpl.getGradleProject();
         prjImpl.setAimedQuality(Quality.EVALUATED);
         GradleProject newProject = prjImpl.getGradleProject();
         assertTrue(newProject.getQuality().worseThan(NbGradleProject.Quality.EVALUATED));
         assertSame(newProject, curProject);
+        assertHasNoConnection(prj);

Review comment:
       What kinds of actions can cause the project to have a connection? I assume there is at least one test that causes hasConnection to become true, since it was necessary to add logic to close the connection in tearDown? If the latter is true, does this mean we are now depending on the various test methods to execute in a specific order, so that the connection is only made later?




-- 
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