From b42385eca43677cc6e2a9352dac103184d521db0 Mon Sep 17 00:00:00 2001 From: Mauro Regli Date: Wed, 13 Sep 2023 07:46:23 +0200 Subject: [PATCH 1/3] Fix: Graph deleted before aptly exits The temporary output file is now only deleted after copying it to the output location. Fixes: #1213 --- cmd/graph.go | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/cmd/graph.go b/cmd/graph.go index 4425d087b..dba652b03 100644 --- a/cmd/graph.go +++ b/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 } From 9c4b20443ae0e7b3edaf5b5b3c640114adfbc46d Mon Sep 17 00:00:00 2001 From: Mauro Regli Date: Wed, 13 Sep 2023 09:15:13 +0200 Subject: [PATCH 2/3] Tests: Add Basic test for graph command --- system/lib.py | 4 ++++ system/requirements.txt | 1 + system/t13_graph/__init__.py | 3 +++ system/t13_graph/graph.py | 25 +++++++++++++++++++++++++ 4 files changed, 33 insertions(+) create mode 100644 system/t13_graph/__init__.py create mode 100644 system/t13_graph/graph.py diff --git a/system/lib.py b/system/lib.py index 9206c06a1..72535982e 100644 --- a/system/lib.py +++ b/system/lib.py @@ -442,6 +442,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, )) diff --git a/system/requirements.txt b/system/requirements.txt index b06273400..b86e9978d 100644 --- a/system/requirements.txt +++ b/system/requirements.txt @@ -7,3 +7,4 @@ flake8 termcolor etcd3 leveldb +pillow diff --git a/system/t13_graph/__init__.py b/system/t13_graph/__init__.py new file mode 100644 index 000000000..9c738a636 --- /dev/null +++ b/system/t13_graph/__init__.py @@ -0,0 +1,3 @@ +""" +Testing Aptly Graph Command +""" diff --git a/system/t13_graph/graph.py b/system/t13_graph/graph.py new file mode 100644 index 000000000..1d4bd7050 --- /dev/null +++ b/system/t13_graph/graph.py @@ -0,0 +1,25 @@ +from PIL import Image +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() From bf34f8af858ce4f290b3325564c2a569e4abac52 Mon Sep 17 00:00:00 2001 From: Mauro Regli Date: Wed, 13 Dec 2023 09:45:04 +0100 Subject: [PATCH 3/3] Add output to BaseTest, add Graph Tests --- system/lib.py | 2 ++ system/t13_graph/graph.py | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/system/lib.py b/system/lib.py index 72535982e..dc47bc2fc 100644 --- a/system/lib.py +++ b/system/lib.py @@ -157,6 +157,8 @@ class BaseTest(object): gpgFinder = GPGFinder() dotFinder = DotFinder() + output: bytes | None = None + def test(self): self.prepare() try: diff --git a/system/t13_graph/graph.py b/system/t13_graph/graph.py index 1d4bd7050..0b5026957 100644 --- a/system/t13_graph/graph.py +++ b/system/t13_graph/graph.py @@ -1,4 +1,7 @@ from PIL import Image +import time +import re +import os from lib import BaseTest @@ -23,3 +26,68 @@ def check(self): # 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)