Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Graph deleted before aptly exits #1214

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 11 additions & 22 deletions cmd/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"path/filepath"
"runtime"
"strings"
"time"

"github.com/aptly-dev/aptly/deb"
"github.com/aptly-dev/aptly/utils"
Expand Down Expand Up @@ -79,36 +78,26 @@
return err
}

defer func() {
_ = os.Remove(tempfilename)
}()

if output != "" {
err = utils.CopyFile(tempfilename, output)
_ = os.Remove(tempfilename)

Check warning on line 83 in cmd/graph.go

View check run for this annotation

Codecov / codecov/patch

cmd/graph.go#L83

Added line #L83 was not covered by tests
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

Check warning on line 89 in cmd/graph.go

View check run for this annotation

Codecov / codecov/patch

cmd/graph.go#L89

Added line #L89 was not covered by tests
}

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

Check warning on line 100 in cmd/graph.go

View check run for this annotation

Codecov / codecov/patch

cmd/graph.go#L92-L100

Added lines #L92 - L100 were not covered by tests
return err
}

Expand Down
6 changes: 6 additions & 0 deletions system/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ class BaseTest(object):
gpgFinder = GPGFinder()
dotFinder = DotFinder()

output: bytes | None = None

def test(self):
self.prepare()
try:
Expand Down Expand Up @@ -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, ))
Expand Down
1 change: 1 addition & 0 deletions system/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ flake8
termcolor
etcd3
leveldb
pillow
3 changes: 3 additions & 0 deletions system/t13_graph/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"""
Testing Aptly Graph Command
"""
93 changes: 93 additions & 0 deletions system/t13_graph/graph.py
Original file line number Diff line number Diff line change
@@ -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)
Loading