Fix: Graph deleted before aptly exits

cmd/graph.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
-	"time"
 
 	"github.com/aptly-dev/aptly/deb"
 	"github.com/aptly-dev/aptly/utils"
@@ -79,36 +78,26 @@ func aptlyGraph(cmd *commander.Command, args []string) error {
 		return err
 	}
 
-	defer func() {
-		_ = os.Remove(tempfilename)
-	}()
-
 	if output != "" {
 		err = utils.CopyFile(tempfilename, output)
+		_ = os.Remove(tempfilename)
 		if err != nil {
 			return fmt.Errorf("unable to copy %s -> %s: %s", tempfilename, output, err)
 		}
 
 		fmt.Printf("Output saved to %s\n", output)
-	} else {
-		command := getOpenCommand()
-		fmt.Printf("Rendered to %s file: %s, trying to open it with: %s %s...\n", format, tempfilename, command, tempfilename)
-
-		args := strings.Split(command, " ")
-
-		viewer := exec.Command(args[0], append(args[1:], tempfilename)...)
-		viewer.Stderr = os.Stderr
-		if err = viewer.Start(); err == nil {
-			// Wait for a second so that the visualizer has a chance to
-			// open the input file. This needs to be done even if we're
-			// waiting for the visualizer as it can be just a wrapper that
-			// spawns a browser tab and returns right away.
-			defer func(t <-chan time.Time) {
-				<-t
-			}(time.After(time.Second))
-		}
+		return nil
 	}
 
+	openCommand := getOpenCommand()
+	fmt.Printf("Rendered to %s file: %s, trying to open it with: %s %s...\n", format, tempfilename, openCommand, tempfilename)
+
+	openCommandArgs := strings.Split(openCommand, " ")
+
+	// The process will continue running even after aptly has exited.
+	viewer := exec.Command(openCommandArgs[0], append(openCommandArgs[1:], tempfilename)...)
+	viewer.Stderr = os.Stderr
+	err = viewer.Start()
 	return err
 }
 

system/lib.py

@@ -157,6 +157,8 @@ class BaseTest(object):
     gpgFinder = GPGFinder()
     dotFinder = DotFinder()
 
+    output: bytes | None = None
+
     def test(self):
         self.prepare()
         try:
@@ -442,6 +444,10 @@ def check_exists(self, path):
         if not os.path.exists(os.path.join(os.environ["HOME"], self.aptlyDir, path)):
             raise Exception("path %s doesn't exist" % (path, ))
 
+    def check_exists_in_cwd(self, path):
+        if not os.path.exists(path):
+            raise Exception(f"path {path} doesn't exist")
+
     def check_not_exists(self, path):
         if os.path.exists(os.path.join(os.environ["HOME"], self.aptlyDir, path)):
             raise Exception("path %s exists" % (path, ))

system/requirements.txt

@@ -7,3 +7,4 @@ flake8
 termcolor
 etcd3
 leveldb
+pillow

system/t13_graph/__init__.py

@@ -0,0 +1,3 @@
+"""
+Testing Aptly Graph Command
+"""

system/t13_graph/graph.py

@@ -0,0 +1,93 @@
+from PIL import Image
+import time
+import re
+import os
+from lib import BaseTest
+
+
+class GraphTest1(BaseTest):
+    """
+    Test that graph is generated correctly and accessible at the specified output path.
+    """
+
+    fixtureDB = True
+    fixturePool = True
+    fixtureCmds = [
+        "aptly snapshot create snap1 from mirror gnuplot-maverick",
+        "aptly publish snapshot -skip-signing snap1",
+    ]
+    runCmd = "aptly graph -output=graph.png -layout=horizontal"
+
+    def check(self):
+        self.check_exists_in_cwd("graph.png")
+
+        with Image.open("graph.png") as img:
+            (width, height) = img.size
+            # check is horizontal
+            self.check_gt(width, height)
+            img.verify()
+
+        os.remove("graph.png")
+
+
+class GraphTest2(BaseTest):
+    """
+    Test that the graph is correctly generate when vertical layout is specified.
+    """
+
+    fixtureDB = True
+    fixturePool = True
+    fixtureCmds = [
+        "aptly snapshot create snap2 from mirror gnuplot-maverick",
+        "aptly publish snapshot -skip-signing snap2",
+    ]
+    runCmd = "aptly graph -output=graph.png -layout=vertical"
+
+    def check(self):
+        self.check_exists_in_cwd("graph.png")
+
+        with Image.open("graph.png") as img:
+            (width, height) = img.size
+            # check is horizontal
+            self.check_gt(height, width)
+            img.verify()
+
+        os.remove("graph.png")
+
+
+class GraphTest3(BaseTest):
+    """
+    Test that the graph is accessible at the temporary
+    file path aptly prints.
+    """
+
+    fixtureDB = True
+    fixturePool = True
+    fixtureCmds = [
+        "aptly snapshot create snap3 from mirror gnuplot-maverick",
+        "aptly publish snapshot -skip-signing snap3",
+    ]
+    runCmd = "aptly graph"
+
+    def check(self):
+        assert self.output is not None
+
+        file_regex = re.compile(r": (\S+).png")
+        temp_file = file_regex.search(self.output.decode())
+
+        assert temp_file is not None
+        temp_file = temp_file.group(1) + ".png"
+
+        self.check_exists(temp_file)
+        with Image.open(temp_file) as img:
+            (width, height) = img.size
+            # check is horizontal
+            self.check_gt(width, height)
+            img.verify()
+
+        # wait 1s to make sure it still exists
+        time.sleep(1)
+
+        assert os.path.isfile(temp_file)
+
+        os.remove(temp_file)