Merge ~jslarraz/review-tools:use-container-in-review-classes into review-tools:master

[Jorge Sancho Larraz]

remove mappings `self.pkg_* = self.snap.*` in sr_common and make use of self.snap.* directly in review classes

[Alex Murray (alexmurray)]

More nice cleanups - thanks @jslarraz, you are on fire 🔥

review: Approve

diff --git a/reviewtools/sr_common.py b/reviewtools/sr_common.py
index 5b2f2ec..d4f74e7 100644
--- a/reviewtools/sr_common.py
+++ b/reviewtools/sr_common.py
@@ -398,12 +398,6 @@ class SnapReview(ReviewBase):
             self.snap = SnapContainer(fn)
         except ContainerException as e:
             error(str(e))
-        # TODO: update code to directly use self.snap.xxx instead of assigning here
-        self.pkg_filename = self.snap.filename
-        self.unpack_dir = self.snap.unpack_dir
-        self.pkg_files = self.snap.get_files_list(abs=True)
-        self.pkg_bin_files = self.snap.get_compiled_binaries_list(abs=True)
-        self.tmp_dir = self.snap.tmp_dir
 
         try:
             self.snap_yaml = self.snap.get_file_as_yaml(
@@ -512,12 +506,6 @@ class SnapReview(ReviewBase):
         hdr, entries = unsquashfs_lln_parse(out)
         return hdr, entries
 
-    # Since coverage is looked at via the testsuite and the testsuite mocks
-    # this out, don't cover this
-    def _get_unpack_dir(self):  # pragma: nocover
-        """Get unpack directory"""
-        return self.unpack_dir
-
     def _verify_pkgname(self, n):
         """Verify package name"""
         # From validSnapName in snapd/snap/validate.go:
diff --git a/reviewtools/sr_functional.py b/reviewtools/sr_functional.py
index 6823b8a..32261e8 100644
--- a/reviewtools/sr_functional.py
+++ b/reviewtools/sr_functional.py
@@ -86,8 +86,8 @@ class SnapReviewFunctional(SnapReview):
 
     def _cmd_nm(self, args):  # pragma: nocover
         """_cmd_nm indirection for unittests"""
-        real_path = os.path.join(self.unpack_dir, args[-1])
-        if real_path not in self.pkg_bin_files:
+        real_path = os.path.join(self.snap.unpack_dir, args[-1])
+        if args[-1] not in self.snap.bin_files:
             return -1, ""
         args[-1] = real_path
 
@@ -300,9 +300,11 @@ class SnapReviewFunctional(SnapReview):
         for p in func_execstack_skipped_pats:
             skipped_pats.append(re.compile(r"%s" % p))
 
-        for i in self.pkg_bin_files:
-            if has_execstack(i) and not self._in_patterns(skipped_pats, i):
-                bins.append(os.path.relpath(i, self.unpack_dir))
+        for i in self.snap.bin_files:
+            if has_execstack(
+                os.path.join(self.snap.unpack_dir, i)
+            ) and not self._in_patterns(skipped_pats, i):
+                bins.append(i)
 
         if len(bins) > 0:
             bins.sort()
@@ -338,7 +340,7 @@ class SnapReviewFunctional(SnapReview):
 
         for i in self.base_required_dirs:
             # self.base_required_dirs are absolute paths
-            mp = os.path.join(self.unpack_dir, i[1:])
+            mp = os.path.join(self.snap.unpack_dir, i[1:])
             if not os.path.isdir(mp):
                 missing.append(i)
 
diff --git a/reviewtools/sr_lint.py b/reviewtools/sr_lint.py
index 6df02e1..f5bc8ae 100644
--- a/reviewtools/sr_lint.py
+++ b/reviewtools/sr_lint.py
@@ -111,14 +111,14 @@ class SnapReviewLint(SnapReview):
 
     def check_valid_hook(self):
         """Check valid hook"""
-        hooks = glob.glob("%s/meta/hooks/*" % self._get_unpack_dir())
+        hooks = glob.glob("%s/meta/hooks/*" % self.snap.unpack_dir)
         if len(hooks) == 0:
             return
 
         unknown = []
         for fn in hooks:
             hook = os.path.basename(fn)
-            rel = os.path.relpath(fn, self._get_unpack_dir())
+            rel = os.path.relpath(fn, self.snap.unpack_dir)
             t = "info"
             n = self._get_check_name("hook_executable", app=hook)
             s = "OK"
@@ -198,8 +198,7 @@ class SnapReviewLint(SnapReview):
         t = "info"
         n = self._get_check_name("icon_exists")
         s = "OK"
-        fn = os.path.join(self._get_unpack_dir(), self.snap_yaml["icon"])
-        if fn not in self.pkg_files:
+        if self.snap_yaml["icon"] not in self.snap.files:
             t = "error"
             s = "icon entry '%s' does not exist" % self.snap_yaml["icon"]
         self._add_result(t, n, s)
@@ -415,8 +414,7 @@ class SnapReviewLint(SnapReview):
             ):
                 val = val.lstrip("/")
 
-            fn = os.path.join(self._get_unpack_dir(), os.path.normpath(val))
-            if fn not in self.pkg_files:
+            if os.path.normpath(val) not in self.snap.files:
                 t = "error"
                 s = "%s does not exist" % (self.snap_yaml["apps"][app][key])
         self._add_result(t, n, s)
@@ -486,8 +484,7 @@ class SnapReviewLint(SnapReview):
             ):
                 val = val.lstrip("/")
 
-            fn = os.path.join(self._get_unpack_dir(), os.path.normpath(val))
-            if fn not in self.pkg_files:
+            if os.path.normpath(val) not in self.snap.files:
                 nonexistent.append(val)
 
             if len(nonexistent) != 0:
@@ -1353,8 +1350,8 @@ class SnapReviewLint(SnapReview):
         n = self._get_check_name("external_symlinks")
         s = "OK"
         links = find_external_symlinks(
-            self._get_unpack_dir(),
-            self.pkg_files,
+            self.snap.unpack_dir,
+            [os.path.join(self.snap.unpack_dir, f) for f in self.snap.files],
             self.snap_yaml["name"],
             prefix_ok=prefix_ok,
         )
@@ -1378,11 +1375,11 @@ class SnapReviewLint(SnapReview):
 
         # look for compiled code
         x_binaries = []
-        for i in self.pkg_bin_files:
+        for i in self.snap.bin_files:
             # .pyc files are arch-independent
             if i.endswith(".pyc"):
                 continue
-            x_binaries.append(os.path.relpath(i, self._get_unpack_dir()))
+            x_binaries.append(i)
         if len(x_binaries) > 0:
             x_binaries.sort()
             # gadget snap is specified with 'all' but has binaries. Don't complain
@@ -1410,7 +1407,7 @@ class SnapReviewLint(SnapReview):
             t = "info"
             n = self._get_check_name("architecture_specified_needed", extra=arch)
             s = "OK"
-            if len(self.pkg_bin_files) == 0:
+            if len(self.snap.bin_files) == 0:
                 # This should be a warning but it causes friction for uploads
                 t = "info"
                 s = "Could not find compiled binaries for architecture '%s'" % arch
@@ -1423,10 +1420,10 @@ class SnapReviewLint(SnapReview):
         s = "OK"
         found = []
         for d in self.vcs_files:
-            entries = glob.glob("%s/%s" % (self._get_unpack_dir(), d))
+            entries = glob.glob("%s/%s" % (self.snap.unpack_dir, d))
             if len(entries) > 0:
                 for i in entries:
-                    found.append(os.path.relpath(i, self.unpack_dir))
+                    found.append(os.path.relpath(i, self.snap.unpack_dir))
         if len(found) > 0:
             t = "warn"
             s = "found VCS files in package: %s" % ", ".join(found)
@@ -1439,11 +1436,11 @@ class SnapReviewLint(SnapReview):
         s = "OK"
         found = []
 
-        for f in self.pkg_files:
+        for f in self.snap.files:
             fn = os.path.basename(f)
             for d in self.iffy_files:
                 if re.search(r"%s" % d, fn):
-                    found.append(os.path.relpath(f, self.unpack_dir))
+                    found.append(f)
 
         if len(found) > 0:
             t = "warn"
@@ -1839,14 +1836,14 @@ class SnapReviewLint(SnapReview):
             # consider snap-controlled paths that symlink outside of the snap
             # (${SNAP}/ is in the read-only area, so it is fine).
             if fn.startswith("${SNAP}/"):
-                return True, os.path.join(self._get_unpack_dir(), fn.split("/", 1)[1])
+                return True, os.path.join(self.snap.unpack_dir, fn.split("/", 1)[1])
 
             # As a special consideration to not break existing snaps, allow
             # /snap/<snap name>/.... People should use ${SNAP}/ instead since
             # /snap is not portable (eg, Fedora puts this in /var/lib/snap
             # instead of /snap), but '${SNAP}' wasn't always available.
             if fn.startswith("/snap/%s/current/" % self.snap_yaml["name"]):
-                return True, os.path.join(self._get_unpack_dir(), fn.split("/", 4)[4])
+                return True, os.path.join(self.snap.unpack_dir, fn.split("/", 4)[4])
 
             return False, None
         elif fn.startswith("snap."):
@@ -2017,8 +2014,7 @@ class SnapReviewLint(SnapReview):
         s = "OK"
 
         has_desktop_files = False
-        for f in self.pkg_files:
-            fn = os.path.relpath(f, self._get_unpack_dir())
+        for fn in self.snap.files:
             if fn.startswith("meta/gui/") and fn.endswith(".desktop"):
                 try:
                     self._verify_desktop_file(fn)
@@ -3049,16 +3045,14 @@ class SnapReviewLint(SnapReview):
 
     def check_valid_icon_sets(self):
         """Check valid icon sets"""
-        icons = glob.glob(
-            "%s/meta/gui/icons/**" % self._get_unpack_dir(), recursive=True
-        )
+        icons = glob.glob("%s/meta/gui/icons/**" % self.snap.unpack_dir, recursive=True)
         bad_names = []
         unsnap_names = []
         toobig_names = []
         wrongext_names = []
         count = 0
         for fn in icons:
-            rel = os.path.relpath(fn, self._get_unpack_dir())
+            rel = os.path.relpath(fn, self.snap.unpack_dir)
             if rel != shlex.quote(rel):
                 bad_names.append(rel)
                 continue
diff --git a/reviewtools/sr_security.py b/reviewtools/sr_security.py
index eb2d202..5bc49b2 100644
--- a/reviewtools/sr_security.py
+++ b/reviewtools/sr_security.py
@@ -187,7 +187,7 @@ class SnapReviewSecurity(SnapReview):
 
     def check_squashfs_resquash(self):
         """Check resquash of squashfs"""
-        fn = os.path.abspath(self.pkg_filename)
+        fn = os.path.abspath(self.snap.filename)
 
         # Verify squashfs has no fragments. If it does, it will not resquash
         # properly (LP: #1576763). This stat output for fragments has been
@@ -267,7 +267,8 @@ class SnapReviewSecurity(SnapReview):
             self._add_result(t, n, s)
             return
 
-        tmpdir = self.tmp_dir  # this is autocleaned
+        # TODO: either use SnapContainer for the test itself or use other tmp directory
+        tmpdir = self.snap.tmp_dir  # this is autocleaned
         tmp_unpack = os.path.join(tmpdir, "squashfs-root")
         tmp_repack = os.path.join(tmpdir, "repack.snap")
         fakeroot_env = os.path.join(tmpdir, "fakeroot.env")
diff --git a/reviewtools/sr_tests.py b/reviewtools/sr_tests.py
index eaea513..1b5f93d 100644
--- a/reviewtools/sr_tests.py
+++ b/reviewtools/sr_tests.py
@@ -26,7 +26,6 @@ TEST_SNAP_YAML = ""
 TEST_SNAP_MANIFEST_YAML = ""
 TEST_PKGFMT_TYPE = "snap"
 TEST_PKGFMT_VERSION = "16.04"
-TEST_UNPACK_DIR = "/fake"
 TEST_UNSQUASHFS_LLN_HDR = ""
 TEST_UNSQUASHFS_LLN_ENTRIES = ("", None)
 TEST_CMD_NM = (0, "")
@@ -62,11 +61,6 @@ def _pkgfmt_type(self):
     return TEST_PKGFMT_TYPE
 
 
-def __get_unpack_dir(self):
-    """Pretend we found the unpack dir"""
-    return TEST_UNPACK_DIR
-
-
 def _unsquashfs_lln(self, fn):
     """Pretend we ran unsquashfs -lln fn"""
     return (TEST_UNSQUASHFS_LLN_HDR, TEST_UNSQUASHFS_LLN_ENTRIES)
@@ -93,9 +87,6 @@ def create_patches():
     )
     # sr_common
     patches.append(
-        patch("reviewtools.sr_common.SnapReview._get_unpack_dir", __get_unpack_dir)
-    )
-    patches.append(
         patch("reviewtools.sr_common.SnapReview._unsquashfs_lln", _unsquashfs_lln)
     )
 
@@ -184,10 +175,6 @@ class TestSnapReview(TestCase):
         TEST_PKGFMT_TYPE = t
         TEST_PKGFMT_VERSION = v
 
-    def set_test_unpack_dir(self, d):
-        global TEST_UNPACK_DIR
-        TEST_UNPACK_DIR = d
-
     def set_test_unsquashfs_lln(self, hdr, entries):
         global TEST_UNSQUASHFS_LLN_HDR
         global TEST_UNSQUASHFS_LLN_ENTRIES
@@ -216,8 +203,6 @@ class TestSnapReview(TestCase):
         TEST_PKGFMT_TYPE = "snap"
         global TEST_PKGFMT_VERSION
         TEST_PKGFMT_VERSION = "16.04"
-        global TEST_UNPACK_DIR
-        TEST_UNPACK_DIR = "/fake"
         global TEST_UNSQUASHFS_LLN_HDR
         TEST_UNSQUASHFS_LLN_HDR = ""
         global TEST_UNSQUASHFS_LLN_ENTRIES
diff --git a/reviewtools/tests/schemas/test_schema_snap.py b/reviewtools/tests/schemas/test_schema_snap.py
index 59cf0f7..d4e2b29 100644
--- a/reviewtools/tests/schemas/test_schema_snap.py
+++ b/reviewtools/tests/schemas/test_schema_snap.py
@@ -283,4 +283,3 @@ class TestSchemaSnap(TestSchemaBase):
             with self.subTest(value=value):
                 error = error.replace("{value}", str(value)) if error else error
                 self._test_value("assumes", value, error)
-
diff --git a/reviewtools/tests/test_sr_functional.py b/reviewtools/tests/test_sr_functional.py
index 2e2e3d5..8935fbc 100644
--- a/reviewtools/tests/test_sr_functional.py
+++ b/reviewtools/tests/test_sr_functional.py
@@ -53,7 +53,6 @@ class TestSnapReviewFunctional(sr_tests.TestSnapReview):
         self.set_test_unsquashfs_lln(hdr, entries)
 
     def _set_default_state(self):
-        self.set_test_unpack_dir = "/nonexistent"
         lln = """drwxr-xr-x 0/0               215 2020-03-23 14:23 squashfs-root
 drwxr-xr-x 0/0                73 2020-03-23 14:23 squashfs-root/bin
 -rwxr-xr-x 0/0             43416 2020-03-23 14:23 squashfs-root/bin/cat
@@ -1375,7 +1374,7 @@ class TestSnapReviewFunctionalNoMock(TestCase):
 
         package = utils.make_snap2(output_dir=output_dir)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = [fn]
+        c.snap._unpack_dir = output_dir
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": None, "warn": 1, "error": 0}
@@ -1415,7 +1414,7 @@ confinement: devmode
 """
         package = utils.make_snap2(output_dir=output_dir, yaml=yaml)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = [fn]
+        c.snap._unpack_dir = output_dir
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1447,7 +1446,7 @@ confinement: devmode
         cmd(["execstack", "--set-execstack", fn])
         package = utils.make_snap2(name="test-override", output_dir=output_dir)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = [fn]
+        c.snap._unpack_dir = output_dir
 
         # update overrides for our snap
         from reviewtools.overrides import func_execstack_overrides
@@ -1494,7 +1493,7 @@ type: os
 """
         package = utils.make_snap2(output_dir=output_dir, yaml=yaml)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = [fn]
+        c.snap._unpack_dir = output_dir
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": 0, "warn": 0, "error": 0}
@@ -1508,7 +1507,7 @@ type: os
 
         package = utils.make_snap2(output_dir=self.mkdtemp())
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = ["path/to/nonexistent/file"]
+        c._bin_files = ["path/to/nonexistent/file"]
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1543,7 +1542,6 @@ type: os
         ]
         output_dir = self.mkdtemp()
 
-        pkg_bin_files = []
         for f in test_files:
             dir = os.path.join(output_dir, os.path.dirname(f))
             if not os.path.exists(dir):
@@ -1552,11 +1550,10 @@ type: os
             shutil.copyfile("/bin/ls", fn)
             # create a /bin/ls with executable stack
             cmd(["execstack", "--set-execstack", fn])
-            pkg_bin_files.append(fn)
 
         package = utils.make_snap2(output_dir=output_dir)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = pkg_bin_files
+        c.snap._unpack_dir = output_dir
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1574,7 +1571,6 @@ type: os
         test_files = ["hasexecstack.bin", "usr/lib/klibc/bin/cat"]
         output_dir = self.mkdtemp()
 
-        pkg_bin_files = []
         for f in test_files:
             dir = os.path.join(output_dir, os.path.dirname(f))
             if not os.path.exists(dir):
@@ -1583,11 +1579,10 @@ type: os
             shutil.copyfile("/bin/ls", fn)
             # create a /bin/ls with executable stack
             cmd(["execstack", "--set-execstack", fn])
-            pkg_bin_files.append(fn)
 
         package = utils.make_snap2(output_dir=output_dir)
         c = SnapReviewFunctional(package)
-        c.pkg_bin_files = pkg_bin_files
+        c.snap._unpack_dir = output_dir
         c.check_execstack()
         report = c.review_report
         expected_counts = {"info": None, "warn": 1, "error": 0}
diff --git a/reviewtools/tests/test_sr_lint.py b/reviewtools/tests/test_sr_lint.py
index 7107d65..9a413f6 100644
--- a/reviewtools/tests/test_sr_lint.py
+++ b/reviewtools/tests/test_sr_lint.py
@@ -312,9 +312,8 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         """Test check_icon()"""
         self.set_test_snap_yaml("icon", "someicon")
         self.set_test_snap_yaml("type", "gadget")
-        self.set_test_unpack_dir = "/nonexistent"
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join(c._get_unpack_dir(), "someicon"))
+        c.snap._files = c.snap.files + ["someicon"]
         c.check_icon()
         r = c.review_report
         expected_counts = {"info": 4, "warn": 0, "error": 0}
@@ -323,9 +322,8 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
     def test_check_icon_no_gadget(self):
         """Test check_icon() - no gadget"""
         self.set_test_snap_yaml("icon", "someicon")
-        self.set_test_unpack_dir = "/nonexistent"
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join(c._get_unpack_dir(), "someicon"))
+        c.snap._files = c.snap.files + ["someicon"]
         c.check_icon()
         r = c.review_report
         expected_counts = {"info": None, "warn": 1, "error": 0}
@@ -356,7 +354,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         self.set_test_snap_yaml("icon", "/foo/bar/someicon")
         self.set_test_snap_yaml("type", "gadget")
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append("/foo/bar/someicon")
+        c.snap._files = c.snap.files + ["/foo/bar/someicon"]
         c.check_icon()
         r = c.review_report
         expected_counts = {"info": None, "warn": 0, "error": 1}
@@ -366,9 +364,8 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         """Test check_icon() - canonical path"""
         self.set_test_snap_yaml("icon", "../foo/someicon")
         self.set_test_snap_yaml("type", "gadget")
-        self.set_test_unpack_dir = "/nonexistent"
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join(c._get_unpack_dir(), "../foo/someicon"))
+        c.snap._files = c.snap.files + ["../foo/someicon"]
         c.check_icon()
         r = c.review_report
         expected_counts = {"info": None, "warn": 0, "error": 1}
@@ -378,10 +375,8 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         """Test check_icon() - missing icon"""
         self.set_test_snap_yaml("icon", "someicon")
         self.set_test_snap_yaml("type", "gadget")
-        self.set_test_unpack_dir = "/nonexistent"
         c = SnapReviewLint(self.test_name)
-        # since the icon isn't in c.pkg_files, don't add it for this test
-        # c.pkg_files.append(os.path.join(c._get_unpack_dir(), 'someicon'))
+        # since the icon isn't in c.snap.files, don't add it for this test
         c.check_icon()
         r = c.review_report
         expected_counts = {"info": None, "warn": 0, "error": 1}
@@ -839,7 +834,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -850,7 +845,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command": "./" + cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -861,7 +856,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "/bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append("/fake" + cmd)
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 0, "warn": 0, "error": 1}
@@ -873,7 +868,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         self.set_test_snap_yaml("apps", {"foo": {"command": cmd}})
         self.set_test_snap_yaml("base", "nix-base")
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append("/fake" + cmd)
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -921,7 +916,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command": "%s -c bar" % cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -932,7 +927,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo bar"
         self.set_test_snap_yaml("apps", {"foo": {"command": "'%s'" % cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -943,7 +938,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo bar"
         self.set_test_snap_yaml("apps", {"foo": {"command": "'%s' -c foo" % cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -972,7 +967,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": [cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -983,7 +978,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "/bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": [cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append("/fake" + cmd)
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 0, "warn": 0, "error": 1}
@@ -995,7 +990,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": [cmd]}})
         self.set_test_snap_yaml("base", "nix-base")
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append("/fake" + cmd)
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1006,7 +1001,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": ["./" + cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1072,7 +1067,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": ["%s -c bar" % cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1083,7 +1078,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo bar"
         self.set_test_snap_yaml("apps", {"foo": {"command-chain": ["'%s'" % cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1096,7 +1091,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
             "apps", {"foo": {"command-chain": ["'%s' -c foo" % cmd]}}
         )
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1127,7 +1122,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("hooks", {"foo": {"command-chain": [cmd]}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_hooks_command_chain()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1138,7 +1133,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("hooks", {"foo": {}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_hooks_command_chain()
         r = c.review_report
         expected_counts = {"info": 0, "warn": 0, "error": 0}
@@ -1149,7 +1144,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"completer": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_completer()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1160,7 +1155,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"completer": "./" + cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_completer()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1208,7 +1203,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"stop-command": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_stop_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1256,7 +1251,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"post-stop-command": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_post_stop_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -1304,7 +1299,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
         cmd = "bin/foo"
         self.set_test_snap_yaml("apps", {"foo": {"reload-command": cmd}})
         c = SnapReviewLint(self.test_name)
-        c.pkg_files.append(os.path.join("/fake", cmd))
+        c.snap._files = c.snap.files + [cmd.lstrip("/")]
         c.check_apps_reload_command()
         r = c.review_report
         expected_counts = {"info": 1, "warn": 0, "error": 0}
@@ -5786,6 +5781,7 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
     def test__verify_icon_path(self):
         """Test _verify_icon_path()"""
         c = SnapReviewLint(self.test_name)
+        unpack_dir = c.snap.unpack_dir
 
         invalid = [
             "/foo/../bar",
@@ -5815,14 +5811,14 @@ class TestSnapReviewLint(sr_tests.TestSnapReview):
             ("foo", None),
             ("foo.png", None),
             ("foo.svg", None),
-            ("${SNAP}/foo.png", "/fake/foo.png"),
-            ("${SNAP}/foo.svg", "/fake/foo.svg"),
-            ("${SNAP}/foo.ico", "/fake/foo.ico"),
-            ("${SNAP}/foo", "/fake/foo"),
-            ("/snap/foo/current/foo.png", "/fake/foo.png"),
-            ("/snap/foo/current/foo.svg", "/fake/foo.svg"),
-            ("/snap/foo/current/foo.ico", "/fake/foo.ico"),
-            ("/snap/foo/current/foo", "/fake/foo"),
+            ("${SNAP}/foo.png", "%s/foo.png" % unpack_dir),
+            ("${SNAP}/foo.svg", "%s/foo.svg" % unpack_dir),
+            ("${SNAP}/foo.ico", "%s/foo.ico" % unpack_dir),
+            ("${SNAP}/foo", "%s/foo" % unpack_dir),
+            ("/snap/foo/current/foo.png", "%s/foo.png" % unpack_dir),
+            ("/snap/foo/current/foo.svg", "%s/foo.svg" % unpack_dir),
+            ("/snap/foo/current/foo.ico", "%s/foo.ico" % unpack_dir),
+            ("/snap/foo/current/foo", "%s/foo" % unpack_dir),
             ("snap.foo.foo.png", None),
             ("snap.foo.bar.svg", None),
             ("snap.foo.bar.ico", None),