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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to