Merge pull request #334 from mathworks/improve_teardown

Improve teardown

Author: sameagen-MW

src/main/java/com/mathworks/ci/actions/RunMatlabBuildAction.java

@@ -81,6 +81,14 @@ public void run() throws IOException, InterruptedException, MatlabExecutionExcep
             throw(e);
         } finally {
             annotator.forceEol();
+
+            try {
+                this.runner.removeTempFolder();
+            } catch (Exception e) {
+                // Don't want to override more important error
+                // thrown in catch block
+                System.err.println(e.toString());
+            }
         }
 
         // Handle build result

src/main/java/com/mathworks/ci/actions/RunMatlabCommandAction.java

@@ -31,6 +31,14 @@ public void run() throws IOException, InterruptedException, MatlabExecutionExcep
             this.params.getTaskListener().getLogger()
                 .println(e.getMessage());
             throw(e);
-        } 
+        } finally {
+            try {
+                this.runner.removeTempFolder();
+            } catch (Exception e) {
+                // Don't want to override more important error
+                // thrown in catch block
+                System.err.println(e.toString());
+            }
+        }
     }
 }

src/main/java/com/mathworks/ci/actions/RunMatlabTestsAction.java

@@ -50,7 +50,15 @@ public void run() throws IOException, InterruptedException, MatlabExecutionExcep
                 .getLogger()
                 .println(e.getMessage());
             throw(e);
-        }
+        } finally {
+            try {
+                this.runner.removeTempFolder();
+            } catch (Exception e) {
+                // Don't want to override more important error
+                // thrown in catch block
+                System.err.println(e.toString());
+            }
+        } 
     }
 
     private String singleQuotify(String in) {

src/main/java/com/mathworks/ci/utilities/MatlabCommandRunner.java

@@ -48,17 +48,6 @@ public MatlabCommandRunner(MatlabActionParameters params) throws IOException, In
 
         // Create temp folder
         this.tempFolder = matlabFolder.createTempDir("tempDir", null);
-        
-        // If we hit an error during shutdown while cleaning up
-        // there's not too much that we can do.
-        Runtime.getRuntime().addShutdownHook(
-                new Thread(() -> {
-                    try {
-                        tempFolder.deleteRecursive();
-                    } catch(Exception e) {
-                        System.err.println(e.toString());
-                    }
-                }));
     }
 
     /** 
@@ -67,9 +56,6 @@ public MatlabCommandRunner(MatlabActionParameters params) throws IOException, In
      * @param command The command to run
      */
     public void runMatlabCommand(String command) throws IOException, InterruptedException, MatlabExecutionException {
-
-        System.err.println("START");
-
         this.params.getTaskListener().getLogger()
             .println("\n#################### Starting command output ####################");
 
@@ -159,6 +145,12 @@ public FilePath getTempFolder() {
         return tempFolder;
     }
 
+    public void removeTempFolder() throws IOException, InterruptedException {
+        if (tempFolder.exists()) {
+            tempFolder.deleteRecursive();
+        }
+    }
+
     /**
      * Creates a file with the specified content in the temporary folder.
      *

src/test/java/unit/com/mathworks/ci/actions/RunMatlabBuildActionTest.java

@@ -168,4 +168,11 @@ public void shouldCopyBuildResultsToRootAndAddsAction() throws IOException, Inte
         // Should have copied file to root dir
         assertTrue(new File(tmp, "buildArtifact.json").exists());
     }
+
+    @Test
+    public void shouldRemoveTempFolder() throws IOException, InterruptedException, MatlabExecutionException {
+        action.run();
+
+        verify(runner).removeTempFolder();
+    }
 }

src/test/java/unit/com/mathworks/ci/actions/RunMatlabCommandActionTest.java

@@ -66,4 +66,11 @@ public void printsAndRethrowsMessage() throws IOException, InterruptedException,
             assertEquals(12, e.getExitCode());
         };
     }
+
+    @Test
+    public void shouldRemoveTempFolder() throws IOException, InterruptedException, MatlabExecutionException {
+        action.run();
+
+        verify(runner).removeTempFolder();
+    }
 }

src/test/java/unit/com/mathworks/ci/actions/RunMatlabTestsActionTest.java

@@ -170,4 +170,11 @@ public void printsAndRethrowsMessage() throws IOException, InterruptedException,
             assertEquals(12, e.getExitCode());
         };
     }
+
+    @Test
+    public void shouldRemoveTempFolder() throws IOException, InterruptedException, MatlabExecutionException {
+        action.run();
+
+        verify(runner).removeTempFolder();
+    }
 }

src/test/java/unit/com/mathworks/ci/utilities/MatlabCommandRunnerTest.java

@@ -98,6 +98,22 @@ public void correctTempFolderLocation() throws IOException, InterruptedException
                 startsWith(expected.getRemote()));
     }
 
+    @Test
+    public void removeTempFolderDeletesContents() throws IOException, InterruptedException {
+        runner = new MatlabCommandRunner(params);
+
+        FilePath t = runner.getTempFolder();
+        FilePath f = runner.copyFileToTempFolder("testcontent.txt", "target.txt");
+
+        Assert.assertTrue(t.exists());
+        Assert.assertTrue(f.exists());
+
+        runner.removeTempFolder();
+
+        Assert.assertFalse(t.exists());
+        Assert.assertFalse(f.exists());
+    }
+
     @Test
     public void prepareRunnerExecutableMaci() throws IOException, InterruptedException {
         runner = new MatlabCommandRunner(params);