[pacman-dev] [PATCH 0/3] unit tests for -Qk and -Qkk

Jeremy Heiner

2 Oct 2013 2 Oct '13

5:48 p.m.

The hooks mechanism created here could potentially be used to write unit tests for other scenarios... For example: reinstalling to repair damaged files. The genhook would do the install (so the snapshot would match the desired final state), the snaphook would damage a file or two, and the reinstall-from-local-db would be the test action proper. Jeremy Heiner (3): Add pmtest.rootjoin() and clean up some duplicate joins. Simplify pmtest.generate and .run API (removed pacman parameter). Added tests for -Q --check (both fast(files) and full(mtree)). test/pacman/pmdb.py | 2 +- test/pacman/pmenv.py | 28 ++++++- test/pacman/pmpkg.py | 49 +++++++++++- test/pacman/pmrule.py | 10 +-- test/pacman/pmtest.py | 150 ++++++++++++++++++++++--------------- test/pacman/tests/querycheck001.py | 18 +++++ test/pacman/tests/querycheck002.py | 24 ++++++ test/pacman/tests/querycheck003.py | 24 ++++++ test/pacman/tests/querycheck004.py | 27 +++++++ test/pacman/tests/querycheck005.py | 33 ++++++++ test/pacman/tests/querycheck006.py | 20 +++++ test/pacman/tests/querycheck007.py | 20 +++++ test/pacman/tests/querycheck008.py | 33 ++++++++ test/pacman/tests/querycheck009.py | 30 ++++++++ test/pacman/tests/querycheck010.py | 27 +++++++ test/pacman/util.py | 10 +-- 16 files changed, 431 insertions(+), 74 deletions(-) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py create mode 100644 test/pacman/tests/querycheck005.py create mode 100644 test/pacman/tests/querycheck006.py create mode 100644 test/pacman/tests/querycheck007.py create mode 100644 test/pacman/tests/querycheck008.py create mode 100644 test/pacman/tests/querycheck009.py create mode 100644 test/pacman/tests/querycheck010.py -- 1.8.4

Show replies by date

Jeremy Heiner

2 Oct 2 Oct

5:48 p.m.

New subject: [pacman-dev] [PATCH 1/3] Add pmtest.rootjoin() and clean up some duplicate joins.

Many calls to os.path.join(root, _) were sprinkled around, in some cases redundantly calculating the same exact path. The rootjoin method was added to do this specific join and the "sprinkles" redirected. The parameters to util.mkfile were tweaked to discourage any more of these redundancies from creeping in. The inverse calculation was being done in two places in two different ways, so pmtest.rootremove() was added to unify them. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmdb.py | 2 +- test/pacman/pmpkg.py | 2 +- test/pacman/pmtest.py | 52 +++++++++++++++++++++++++++++++-------------------- test/pacman/util.py | 9 ++++----- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index b7b3522..c3290e3 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -237,7 +237,7 @@ def generate(self): path = os.path.join(self.dbdir, pkg.fullname()) util.mkdir(path) for name, data in entry.iteritems(): - util.mkfile(path, name, data) + util.mkfile(os.path.join(path, name), data) if self.dbfile: tar = tarfile.open(self.dbfile, "w:gz") diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 9b3147a..9c9e447 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -180,8 +180,8 @@ def makepkg(self, path): def install_package(self, root): """Install the package in the given root.""" for f in self.files: - util.mkfile(root, f, f) path = os.path.join(root, f) + util.mkfile(path, f) if os.path.isfile(path): os.utime(path, (355, 355)) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index b343d55..4101546 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -74,6 +74,17 @@ def addrule(self, rulename): rule = pmrule.pmrule(rulename) self.rules.append(rule) + def rootjoin(self, path): + return os.path.join(self.root, path) + + def rootremove(self, path): + end = len(self.root) + if ( end < len(path) + and os.sep == path[end] + and path.startswith(self.root) ): + return path[end+1:] + raise ValueError("path not under root: %s" % path) + def load(self): # Reset test parameters self.result = { @@ -114,27 +125,27 @@ def generate(self, pacman): # Create directory structure vprint(" Creating directory structure:") - dbdir = os.path.join(self.root, util.PM_SYNCDBPATH) - cachedir = os.path.join(self.root, util.PM_CACHEDIR) - syncdir = os.path.join(self.root, util.SYNCREPO) - tmpdir = os.path.join(self.root, util.TMPDIR) - logdir = os.path.join(self.root, os.path.dirname(util.LOGFILE)) - etcdir = os.path.join(self.root, os.path.dirname(util.PACCONF)) - bindir = os.path.join(self.root, "bin") + dbdir = self.rootjoin(util.PM_SYNCDBPATH) + cachedir = self.rootjoin(util.PM_CACHEDIR) + syncdir = self.rootjoin(util.SYNCREPO) + tmpdir = self.rootjoin(util.TMPDIR) + logdir = self.rootjoin(os.path.dirname(util.LOGFILE)) + etcdir = self.rootjoin(os.path.dirname(util.PACCONF)) + bindir = self.rootjoin("bin") ldconfig = os.path.basename(pacman["ldconfig"]) - ldconfigdir = os.path.join(self.root, os.path.dirname(pacman["ldconfig"][1:])) + ldconfigdir = self.rootjoin(os.path.dirname(pacman["ldconfig"][1:])) shell = pacman["scriptlet-shell"][1:] - shelldir = os.path.join(self.root, os.path.dirname(shell)) + shelldir = self.rootjoin(os.path.dirname(shell)) sys_dirs = [dbdir, cachedir, syncdir, tmpdir, logdir, etcdir, bindir, ldconfigdir, shelldir] for sys_dir in sys_dirs: if not os.path.isdir(sys_dir): - vprint("\t%s" % sys_dir[len(self.root)+1:]) + vprint("\t%s" % self.rootremove(sys_dir)) os.makedirs(sys_dir, 0755) # Only the dynamically linked binary is needed for fakechroot shutil.copy("/bin/sh", bindir) if shell != "bin/sh": - shutil.copy("/bin/sh", os.path.join(self.root, shell)) + shutil.copy("/bin/sh", self.rootjoin(shell)) shutil.copy(os.path.join(util.SELFPATH, "ldconfig.stub"), os.path.join(ldconfigdir, ldconfig)) ld_so_conf = open(os.path.join(etcdir, "ld.so.conf"), "w") @@ -174,8 +185,8 @@ def generate(self, pacman): vprint(" Populating file system") for f in self.filesystem: vprint("\t%s" % f) - util.mkfile(self.root, f, f) - path = os.path.join(self.root, f) + path = self.rootjoin(f) + util.mkfile(path, f) if os.path.isfile(path): os.utime(path, (355, 355)) for pkg in self.db["local"].pkgs: @@ -187,7 +198,7 @@ def generate(self, pacman): for roots, dirs, files in os.walk(self.root): for i in files: filename = os.path.join(roots, i) - f = pmfile.PacmanFile(self.root, filename.replace(self.root + "/", "")) + f = pmfile.PacmanFile(self.root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name) @@ -223,26 +234,27 @@ def run(self, pacman): "--show-reachable=yes", "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], - "--config", os.path.join(self.root, util.PACCONF), + "--config", self.rootjoin(util.PACCONF), "--root", self.root, - "--dbpath", os.path.join(self.root, util.PM_DBPATH), - "--cachedir", os.path.join(self.root, util.PM_CACHEDIR)]) + "--dbpath", self.rootjoin(util.PM_DBPATH), + "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) if not pacman["manual-confirm"]: cmd.append("--noconfirm") if pacman["debug"]: cmd.append("--debug=%s" % pacman["debug"]) cmd.extend(shlex.split(self.args)) if not (pacman["gdb"] or pacman["valgrind"] or pacman["nolog"]): - output = open(os.path.join(self.root, util.LOGFILE), 'w') + output = open(self.rootjoin(util.LOGFILE), 'w') else: output = None vprint("\trunning: %s" % " ".join(cmd)) # Change to the tmp dir before running pacman, so that local package # archives are made available more easily. + tmpdir = self.rootjoin(util.TMPDIR) time_start = time.time() self.retcode = subprocess.call(cmd, stdout=output, stderr=output, - cwd=os.path.join(self.root, util.TMPDIR), env={'LC_ALL': 'C'}) + cwd=tmpdir, env={'LC_ALL': 'C'}) time_end = time.time() vprint("\ttime elapsed: %.2fs" % (time_end - time_start)) @@ -256,7 +268,7 @@ def run(self, pacman): tap.diag("\tERROR: %s not removed" % util.PM_LOCK) os.unlink(util.PM_LOCK) # Look for a core file - if os.path.isfile(os.path.join(self.root, util.TMPDIR, "core")): + if os.path.isfile(os.path.join(tmpdir, "core")): tap.diag("\tERROR: pacman dumped a core file") def check(self): diff --git a/test/pacman/util.py b/test/pacman/util.py index 14035d7..5c9a0c0 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -77,11 +77,10 @@ def getfileinfo(filename): data["filename"] = filename return data -def mkfile(base, name, data=""): - info = getfileinfo(name) - filename = info["filename"] +def mkfile(path, data=""): + info = getfileinfo(path) + path = info["filename"] - path = os.path.join(base, filename) if info["isdir"]: if not os.path.isdir(path): os.makedirs(path, 0755) @@ -129,7 +128,7 @@ def mkcfgfile(filename, root, option, db): for optkey, optval in value.option.iteritems(): data.extend(["%s = %s" % (optkey, j) for j in optval]) - mkfile(root, filename, "\n".join(data)) + mkfile(os.path.join(root, filename), "\n".join(data)) # -- 1.8.4

Andrew Gregory

4 Oct 4 Oct

5:20 p.m.

New subject: [pacman-dev] [PATCH 1/3] Add pmtest.rootjoin() and clean up some duplicate joins.

On 10/02/13 at 11:48am, Jeremy Heiner wrote:

...

Many calls to os.path.join(root, _) were sprinkled around, in some cases redundantly calculating the same exact path. The rootjoin method was added to do this specific join and the "sprinkles" redirected. The parameters to util.mkfile were tweaked to discourage any more of these redundancies from creeping in.

The inverse calculation was being done in two places in two different ways, so pmtest.rootremove() was added to unify them.

Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmdb.py | 2 +- test/pacman/pmpkg.py | 2 +- test/pacman/pmtest.py | 52 +++++++++++++++++++++++++++++++-------------------- test/pacman/util.py | 9 ++++----- 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index b7b3522..c3290e3 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -237,7 +237,7 @@ def generate(self): path = os.path.join(self.dbdir, pkg.fullname()) util.mkdir(path) for name, data in entry.iteritems(): - util.mkfile(path, name, data) + util.mkfile(os.path.join(path, name), data)

if self.dbfile: tar = tarfile.open(self.dbfile, "w:gz") diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 9b3147a..9c9e447 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -180,8 +180,8 @@ def makepkg(self, path): def install_package(self, root): """Install the package in the given root.""" for f in self.files: - util.mkfile(root, f, f) path = os.path.join(root, f) + util.mkfile(path, f) if os.path.isfile(path): os.utime(path, (355, 355))

diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index b343d55..4101546 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -74,6 +74,17 @@ def addrule(self, rulename): rule = pmrule.pmrule(rulename) self.rules.append(rule)

+ def rootjoin(self, path): + return os.path.join(self.root, path) + + def rootremove(self, path): + end = len(self.root) + if ( end < len(path) + and os.sep == path[end] + and path.startswith(self.root) ): + return path[end+1:] + raise ValueError("path not under root: %s" % path) +

I'm not sure about the utility of these functions. The checks in rootremove are unnecessary because both places it's used are guaranteed to start with root. If those are removed, both of these are basically just aliases.

...

def load(self): # Reset test parameters self.result = { @@ -114,27 +125,27 @@ def generate(self, pacman):

# Create directory structure vprint(" Creating directory structure:") - dbdir = os.path.join(self.root, util.PM_SYNCDBPATH) - cachedir = os.path.join(self.root, util.PM_CACHEDIR) - syncdir = os.path.join(self.root, util.SYNCREPO) - tmpdir = os.path.join(self.root, util.TMPDIR) - logdir = os.path.join(self.root, os.path.dirname(util.LOGFILE)) - etcdir = os.path.join(self.root, os.path.dirname(util.PACCONF)) - bindir = os.path.join(self.root, "bin") + dbdir = self.rootjoin(util.PM_SYNCDBPATH) + cachedir = self.rootjoin(util.PM_CACHEDIR) + syncdir = self.rootjoin(util.SYNCREPO) + tmpdir = self.rootjoin(util.TMPDIR) + logdir = self.rootjoin(os.path.dirname(util.LOGFILE)) + etcdir = self.rootjoin(os.path.dirname(util.PACCONF)) + bindir = self.rootjoin("bin") ldconfig = os.path.basename(pacman["ldconfig"]) - ldconfigdir = os.path.join(self.root, os.path.dirname(pacman["ldconfig"][1:])) + ldconfigdir = self.rootjoin(os.path.dirname(pacman["ldconfig"][1:])) shell = pacman["scriptlet-shell"][1:] - shelldir = os.path.join(self.root, os.path.dirname(shell)) + shelldir = self.rootjoin(os.path.dirname(shell)) sys_dirs = [dbdir, cachedir, syncdir, tmpdir, logdir, etcdir, bindir, ldconfigdir, shelldir] for sys_dir in sys_dirs: if not os.path.isdir(sys_dir): - vprint("\t%s" % sys_dir[len(self.root)+1:]) + vprint("\t%s" % self.rootremove(sys_dir)) os.makedirs(sys_dir, 0755) # Only the dynamically linked binary is needed for fakechroot shutil.copy("/bin/sh", bindir) if shell != "bin/sh": - shutil.copy("/bin/sh", os.path.join(self.root, shell)) + shutil.copy("/bin/sh", self.rootjoin(shell)) shutil.copy(os.path.join(util.SELFPATH, "ldconfig.stub"), os.path.join(ldconfigdir, ldconfig)) ld_so_conf = open(os.path.join(etcdir, "ld.so.conf"), "w") @@ -174,8 +185,8 @@ def generate(self, pacman): vprint(" Populating file system") for f in self.filesystem: vprint("\t%s" % f) - util.mkfile(self.root, f, f) - path = os.path.join(self.root, f) + path = self.rootjoin(f) + util.mkfile(path, f) if os.path.isfile(path): os.utime(path, (355, 355)) for pkg in self.db["local"].pkgs: @@ -187,7 +198,7 @@ def generate(self, pacman): for roots, dirs, files in os.walk(self.root): for i in files: filename = os.path.join(roots, i) - f = pmfile.PacmanFile(self.root, filename.replace(self.root + "/", "")) + f = pmfile.PacmanFile(self.root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name)

@@ -223,26 +234,27 @@ def run(self, pacman): "--show-reachable=yes", "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], - "--config", os.path.join(self.root, util.PACCONF), + "--config", self.rootjoin(util.PACCONF), "--root", self.root, - "--dbpath", os.path.join(self.root, util.PM_DBPATH), - "--cachedir", os.path.join(self.root, util.PM_CACHEDIR)]) + "--dbpath", self.rootjoin(util.PM_DBPATH), + "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) if not pacman["manual-confirm"]: cmd.append("--noconfirm") if pacman["debug"]: cmd.append("--debug=%s" % pacman["debug"]) cmd.extend(shlex.split(self.args)) if not (pacman["gdb"] or pacman["valgrind"] or pacman["nolog"]): - output = open(os.path.join(self.root, util.LOGFILE), 'w') + output = open(self.rootjoin(util.LOGFILE), 'w') else: output = None vprint("\trunning: %s" % " ".join(cmd))

# Change to the tmp dir before running pacman, so that local package # archives are made available more easily. + tmpdir = self.rootjoin(util.TMPDIR) time_start = time.time() self.retcode = subprocess.call(cmd, stdout=output, stderr=output, - cwd=os.path.join(self.root, util.TMPDIR), env={'LC_ALL': 'C'}) + cwd=tmpdir, env={'LC_ALL': 'C'}) time_end = time.time() vprint("\ttime elapsed: %.2fs" % (time_end - time_start))

@@ -256,7 +268,7 @@ def run(self, pacman): tap.diag("\tERROR: %s not removed" % util.PM_LOCK) os.unlink(util.PM_LOCK) # Look for a core file - if os.path.isfile(os.path.join(self.root, util.TMPDIR, "core")): + if os.path.isfile(os.path.join(tmpdir, "core")): tap.diag("\tERROR: pacman dumped a core file")

def check(self): diff --git a/test/pacman/util.py b/test/pacman/util.py index 14035d7..5c9a0c0 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -77,11 +77,10 @@ def getfileinfo(filename): data["filename"] = filename return data

-def mkfile(base, name, data=""): - info = getfileinfo(name) - filename = info["filename"] +def mkfile(path, data=""): + info = getfileinfo(path) + path = info["filename"]

- path = os.path.join(base, filename) if info["isdir"]: if not os.path.isdir(path): os.makedirs(path, 0755) @@ -129,7 +128,7 @@ def mkcfgfile(filename, root, option, db): for optkey, optval in value.option.iteritems(): data.extend(["%s = %s" % (optkey, j) for j in optval])

- mkfile(root, filename, "\n".join(data)) + mkfile(os.path.join(root, filename), "\n".join(data))

# -- 1.8.4

Jeremy Heiner

2 Oct 2 Oct

5:48 p.m.

New subject: [pacman-dev] [PATCH 2/3] Simplify pmtest.generate and .run API (removed pacman parameter).

Previously pmtest.__init__ received and cached the pmenv.root, and generate and run received the pmenv.pacman. Now __init__ receives and caches the pmenv, allowing any method to fetch root, pacman, or other pmenv goodies directly instead of needing the extra parameter. Moved the "which fakeroot" and "which fakechroot" logic from pmtest up into pmenv. Repeating the probes and warnings for each test was a bit redundant. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmenv.py | 28 +++++++++++++++++++++++--- test/pacman/pmrule.py | 10 +++++----- test/pacman/pmtest.py | 55 ++++++++++++++++++++++----------------------------- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py index f358285..3cfcfa5 100644 --- a/test/pacman/pmenv.py +++ b/test/pacman/pmenv.py @@ -21,6 +21,7 @@ import pmtest import tap +import util class pmenv(object): @@ -43,6 +44,27 @@ def __init__(self, root = "root"): "nolog": 0 } + if os.geteuid() != 0: + self.fakeroot = util.which("fakeroot") + if not self.fakeroot: + tap.diag("WARNING: fakeroot not found!") + else: + self.fakeroot = "fakeroot" + + self.fakechroot = util.which("fakechroot") + if not self.fakechroot: + tap.diag("WARNING: fakechroot not found!") + else: + self.fakechroot = "fakechroot" + + def cmdroot(self): + cmd = [] + if self.fakeroot: + cmd.append(self.fakeroot) + if self.fakechroot: + cmd.append(self.fakechroot) + return cmd + def __str__(self): return "root = %s\n" \ "pacman = %s" \ @@ -53,7 +75,7 @@ def addtest(self, testcase): """ if not os.path.isfile(testcase): raise IOError("test file %s not found" % testcase) - test = pmtest.pmtest(testcase, self.root) + test = pmtest.pmtest(testcase, self) self.testcases.append(test) def run(self): @@ -64,8 +86,8 @@ def run(self): tap.diag("Running '%s'" % t.testname) t.load() - t.generate(self.pacman) - t.run(self.pacman) + t.generate() + t.run() tap.diag("==> Checking rules") tap.todo = t.expectfailure diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index c97a158..718c87c 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -56,7 +56,7 @@ def check(self, test): if test.retcode != int(key): success = 0 elif case == "OUTPUT": - logfile = os.path.join(test.root, util.LOGFILE) + logfile = os.path.join(test.env.root, util.LOGFILE) if not os.access(logfile, os.F_OK): tap.diag("LOGFILE not found, cannot validate 'OUTPUT' rule") success = 0 @@ -112,7 +112,7 @@ def check(self, test): tap.diag("PKG rule '%s' not found" % case) success = -1 elif kind == "FILE": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.isfile(filename): success = 0 @@ -152,7 +152,7 @@ def check(self, test): tap.diag("FILE rule '%s' not found" % case) success = -1 elif kind == "DIR": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.isdir(filename): success = 0 @@ -160,7 +160,7 @@ def check(self, test): tap.diag("DIR rule '%s' not found" % case) success = -1 elif kind == "LINK": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.islink(filename): success = 0 @@ -168,7 +168,7 @@ def check(self, test): tap.diag("LINK rule '%s' not found" % case) success = -1 elif kind == "CACHE": - cachedir = os.path.join(test.root, util.PM_CACHEDIR) + cachedir = os.path.join(test.env.root, util.PM_CACHEDIR) if case == "EXISTS": pkg = test.findpkg(key, value, allow_local=True) if not pkg or not os.path.isfile( diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index 4101546..a0a1455 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -35,20 +35,20 @@ class pmtest(object): """Test object """ - def __init__(self, name, root): + def __init__(self, name, env): self.name = name self.testname = os.path.basename(name).replace('.py', '') - self.root = root + self.env = env self.cachepkgs = True def __str__(self): return "name = %s\n" \ "testname = %s\n" \ - "root = %s" % (self.name, self.testname, self.root) + "root = %s" % (self.name, self.testname, self.env.root) def addpkg2db(self, treename, pkg): if not treename in self.db: - self.db[treename] = pmdb.pmdb(treename, self.root) + self.db[treename] = pmdb.pmdb(treename, self.env.root) self.db[treename].pkgs.append(pkg) def addpkg(self, pkg): @@ -75,13 +75,13 @@ def addrule(self, rulename): self.rules.append(rule) def rootjoin(self, path): - return os.path.join(self.root, path) + return os.path.join(self.env.root, path) def rootremove(self, path): - end = len(self.root) + end = len(self.env.root) if ( end < len(path) and os.sep == path[end] - and path.startswith(self.root) ): + and path.startswith(self.env.root) ): return path[end+1:] raise ValueError("path not under root: %s" % path) @@ -94,7 +94,7 @@ def load(self): self.args = "" self.retcode = 0 self.db = { - "local": pmdb.pmdb("local", self.root) + "local": pmdb.pmdb("local", self.env.root) } self.localpkgs = [] self.createlocalpkgs = False @@ -115,13 +115,16 @@ def load(self): else: raise IOError("file %s does not exist!" % self.name) - def generate(self, pacman): + def generate(self): tap.diag("==> Generating test environment") + root = self.env.root + pacman = self.env.pacman + # Cleanup leftover files from a previous test session - if os.path.isdir(self.root): - shutil.rmtree(self.root) - vprint("\t%s" % self.root) + if os.path.isdir(root): + shutil.rmtree(root) + vprint("\t%s" % root) # Create directory structure vprint(" Creating directory structure:") @@ -153,7 +156,7 @@ def generate(self, pacman): # Configuration file vprint(" Creating configuration file") - util.mkcfgfile(util.PACCONF, self.root, self.option, self.db) + util.mkcfgfile(util.PACCONF, root, self.option, self.db) # Creating packages vprint(" Creating package archives") @@ -191,18 +194,18 @@ def generate(self, pacman): os.utime(path, (355, 355)) for pkg in self.db["local"].pkgs: vprint("\tinstalling %s" % pkg.fullname()) - pkg.install_package(self.root) + pkg.install_package(root) # Done. vprint(" Taking a snapshot of the file system") - for roots, dirs, files in os.walk(self.root): + for roots, dirs, files in os.walk(root): for i in files: filename = os.path.join(roots, i) - f = pmfile.PacmanFile(self.root, self.rootremove(filename)) + f = pmfile.PacmanFile(root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name) - def run(self, pacman): + def run(self): if os.path.isfile(util.PM_LOCK): tap.bail("\tERROR: another pacman session is on-going -- skipping") return @@ -210,19 +213,9 @@ def run(self, pacman): tap.diag("==> Running test") vprint("\tpacman %s" % self.args) - cmd = [] - if os.geteuid() != 0: - fakeroot = util.which("fakeroot") - if not fakeroot: - tap.diag("WARNING: fakeroot not found!") - else: - cmd.append("fakeroot") - - fakechroot = util.which("fakechroot") - if not fakechroot: - tap.diag("WARNING: fakechroot not found!") - else: - cmd.append("fakechroot") + root = self.env.root + pacman = self.env.pacman + cmd = self.env.cmdroot() if pacman["gdb"]: cmd.extend(["libtool", "execute", "gdb", "--args"]) @@ -235,7 +228,7 @@ def run(self, pacman): "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], "--config", self.rootjoin(util.PACCONF), - "--root", self.root, + "--root", root, "--dbpath", self.rootjoin(util.PM_DBPATH), "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) if not pacman["manual-confirm"]: -- 1.8.4

Andrew Gregory

4 Oct 4 Oct

5:21 p.m.

New subject: [pacman-dev] [PATCH 2/3] Simplify pmtest.generate and .run API (removed pacman parameter).

On 10/02/13 at 11:48am, Jeremy Heiner wrote:

...

Previously pmtest.__init__ received and cached the pmenv.root, and generate and run received the pmenv.pacman. Now __init__ receives and caches the pmenv, allowing any method to fetch root, pacman, or other pmenv goodies directly instead of needing the extra parameter.

Moved the "which fakeroot" and "which fakechroot" logic from pmtest up into pmenv. Repeating the probes and warnings for each test was a bit redundant.

Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmenv.py | 28 +++++++++++++++++++++++--- test/pacman/pmrule.py | 10 +++++----- test/pacman/pmtest.py | 55 ++++++++++++++++++++++----------------------------- 3 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py index f358285..3cfcfa5 100644 --- a/test/pacman/pmenv.py +++ b/test/pacman/pmenv.py @@ -21,6 +21,7 @@

import pmtest import tap +import util

class pmenv(object): @@ -43,6 +44,27 @@ def __init__(self, root = "root"): "nolog": 0 }

+ if os.geteuid() != 0: + self.fakeroot = util.which("fakeroot") + if not self.fakeroot: + tap.diag("WARNING: fakeroot not found!") + else: + self.fakeroot = "fakeroot" + + self.fakechroot = util.which("fakechroot") + if not self.fakechroot: + tap.diag("WARNING: fakechroot not found!") + else: + self.fakechroot = "fakechroot"

I know this was a copy-paste, but let's improve it while we're at it. Saving the result of util.which only to overwrite it is kind of confusing. Let's change these to: if util.which("fakeroot"): self.fakeroot = "fakeroot" else: ...

...

+ + def cmdroot(self): + cmd = [] + if self.fakeroot: + cmd.append(self.fakeroot) + if self.fakechroot: + cmd.append(self.fakechroot) + return cmd + def __str__(self): return "root = %s\n" \ "pacman = %s" \ @@ -53,7 +75,7 @@ def addtest(self, testcase): """ if not os.path.isfile(testcase): raise IOError("test file %s not found" % testcase) - test = pmtest.pmtest(testcase, self.root) + test = pmtest.pmtest(testcase, self) self.testcases.append(test)

def run(self): @@ -64,8 +86,8 @@ def run(self): tap.diag("Running '%s'" % t.testname)

t.load() - t.generate(self.pacman) - t.run(self.pacman) + t.generate() + t.run()

tap.diag("==> Checking rules") tap.todo = t.expectfailure diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index c97a158..718c87c 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -56,7 +56,7 @@ def check(self, test): if test.retcode != int(key): success = 0 elif case == "OUTPUT": - logfile = os.path.join(test.root, util.LOGFILE) + logfile = os.path.join(test.env.root, util.LOGFILE) if not os.access(logfile, os.F_OK): tap.diag("LOGFILE not found, cannot validate 'OUTPUT' rule") success = 0 @@ -112,7 +112,7 @@ def check(self, test): tap.diag("PKG rule '%s' not found" % case) success = -1 elif kind == "FILE": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.isfile(filename): success = 0 @@ -152,7 +152,7 @@ def check(self, test): tap.diag("FILE rule '%s' not found" % case) success = -1 elif kind == "DIR": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.isdir(filename): success = 0 @@ -160,7 +160,7 @@ def check(self, test): tap.diag("DIR rule '%s' not found" % case) success = -1 elif kind == "LINK": - filename = os.path.join(test.root, key) + filename = os.path.join(test.env.root, key) if case == "EXIST": if not os.path.islink(filename): success = 0 @@ -168,7 +168,7 @@ def check(self, test): tap.diag("LINK rule '%s' not found" % case) success = -1 elif kind == "CACHE": - cachedir = os.path.join(test.root, util.PM_CACHEDIR) + cachedir = os.path.join(test.env.root, util.PM_CACHEDIR) if case == "EXISTS": pkg = test.findpkg(key, value, allow_local=True) if not pkg or not os.path.isfile( diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index 4101546..a0a1455 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -35,20 +35,20 @@ class pmtest(object): """Test object """

- def __init__(self, name, root): + def __init__(self, name, env): self.name = name self.testname = os.path.basename(name).replace('.py', '') - self.root = root + self.env = env self.cachepkgs = True

def __str__(self): return "name = %s\n" \ "testname = %s\n" \ - "root = %s" % (self.name, self.testname, self.root) + "root = %s" % (self.name, self.testname, self.env.root)

def addpkg2db(self, treename, pkg): if not treename in self.db: - self.db[treename] = pmdb.pmdb(treename, self.root) + self.db[treename] = pmdb.pmdb(treename, self.env.root) self.db[treename].pkgs.append(pkg)

def addpkg(self, pkg): @@ -75,13 +75,13 @@ def addrule(self, rulename): self.rules.append(rule)

def rootjoin(self, path): - return os.path.join(self.root, path) + return os.path.join(self.env.root, path)

def rootremove(self, path): - end = len(self.root) + end = len(self.env.root) if ( end < len(path) and os.sep == path[end] - and path.startswith(self.root) ): + and path.startswith(self.env.root) ): return path[end+1:] raise ValueError("path not under root: %s" % path)

@@ -94,7 +94,7 @@ def load(self): self.args = "" self.retcode = 0 self.db = { - "local": pmdb.pmdb("local", self.root) + "local": pmdb.pmdb("local", self.env.root) } self.localpkgs = [] self.createlocalpkgs = False @@ -115,13 +115,16 @@ def load(self): else: raise IOError("file %s does not exist!" % self.name)

- def generate(self, pacman): + def generate(self): tap.diag("==> Generating test environment")

+ root = self.env.root + pacman = self.env.pacman + # Cleanup leftover files from a previous test session - if os.path.isdir(self.root): - shutil.rmtree(self.root) - vprint("\t%s" % self.root) + if os.path.isdir(root): + shutil.rmtree(root) + vprint("\t%s" % root)

# Create directory structure vprint(" Creating directory structure:") @@ -153,7 +156,7 @@ def generate(self, pacman):

# Configuration file vprint(" Creating configuration file") - util.mkcfgfile(util.PACCONF, self.root, self.option, self.db) + util.mkcfgfile(util.PACCONF, root, self.option, self.db)

# Creating packages vprint(" Creating package archives") @@ -191,18 +194,18 @@ def generate(self, pacman): os.utime(path, (355, 355)) for pkg in self.db["local"].pkgs: vprint("\tinstalling %s" % pkg.fullname()) - pkg.install_package(self.root) + pkg.install_package(root)

# Done. vprint(" Taking a snapshot of the file system") - for roots, dirs, files in os.walk(self.root): + for roots, dirs, files in os.walk(root): for i in files: filename = os.path.join(roots, i) - f = pmfile.PacmanFile(self.root, self.rootremove(filename)) + f = pmfile.PacmanFile(root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name)

- def run(self, pacman): + def run(self): if os.path.isfile(util.PM_LOCK): tap.bail("\tERROR: another pacman session is on-going -- skipping") return @@ -210,19 +213,9 @@ def run(self, pacman): tap.diag("==> Running test") vprint("\tpacman %s" % self.args)

- cmd = [] - if os.geteuid() != 0: - fakeroot = util.which("fakeroot") - if not fakeroot: - tap.diag("WARNING: fakeroot not found!") - else: - cmd.append("fakeroot") - - fakechroot = util.which("fakechroot") - if not fakechroot: - tap.diag("WARNING: fakechroot not found!") - else: - cmd.append("fakechroot") + root = self.env.root + pacman = self.env.pacman + cmd = self.env.cmdroot()

if pacman["gdb"]: cmd.extend(["libtool", "execute", "gdb", "--args"]) @@ -235,7 +228,7 @@ def run(self, pacman): "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], "--config", self.rootjoin(util.PACCONF), - "--root", self.root, + "--root", root, "--dbpath", self.rootjoin(util.PM_DBPATH), "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) if not pacman["manual-confirm"]: -- 1.8.4

Jeremy Heiner

2 Oct 2 Oct

5:48 p.m.

New subject: [pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

Two new "hooks" were made available in pmtest: genhook is called after everything is generated (just before the snapshot is taken), and snaphook is called just after the snapshot. A hook is a list of strings. Calling a hook is "exec"ing each string in the list. Some helper functions were added to pmtest, notably pacmanrun and addrule__pacman_warned. pacmanrun and rootjoin are used in a hook to install a package and then intentionally mess it up, which then allows the "-Qk" test output to be verified (with addrule__pacman_warned). Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmpkg.py | 47 ++++++++++++++++++++++++++++ test/pacman/pmtest.py | 63 +++++++++++++++++++++++++++----------- test/pacman/tests/querycheck001.py | 18 +++++++++++ test/pacman/tests/querycheck002.py | 24 +++++++++++++++ test/pacman/tests/querycheck003.py | 24 +++++++++++++++ test/pacman/tests/querycheck004.py | 27 ++++++++++++++++ test/pacman/tests/querycheck005.py | 33 ++++++++++++++++++++ test/pacman/tests/querycheck006.py | 20 ++++++++++++ test/pacman/tests/querycheck007.py | 20 ++++++++++++ test/pacman/tests/querycheck008.py | 33 ++++++++++++++++++++ test/pacman/tests/querycheck009.py | 30 ++++++++++++++++++ test/pacman/tests/querycheck010.py | 27 ++++++++++++++++ test/pacman/util.py | 1 + 13 files changed, 349 insertions(+), 18 deletions(-) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py create mode 100644 test/pacman/tests/querycheck005.py create mode 100644 test/pacman/tests/querycheck006.py create mode 100644 test/pacman/tests/querycheck007.py create mode 100644 test/pacman/tests/querycheck008.py create mode 100644 test/pacman/tests/querycheck009.py create mode 100644 test/pacman/tests/querycheck010.py diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 9c9e447..bb0facf 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -23,6 +23,8 @@ import shutil from StringIO import StringIO import tarfile +import hashlib +import zlib import util @@ -59,6 +61,7 @@ def __init__(self, name, version = "1.0-1"): # files self.files = [] self.backup = [] + self.mtree = False # install self.install = { "pre_install": "", @@ -104,6 +107,33 @@ def parse_filename(name): filename, extra = filename.split("|") return filename + def mtreefile(self, info, data): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=file size=%d md5digest=%s sha256digest=%s" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF, + info.size, hashlib.md5(data).hexdigest(), + hashlib.sha256(data).hexdigest())) + + def mtreedir(self, info): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=dir" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF)) + + def mtreelink(self, info): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=link link=%s" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF, + info.linkname)) + def makepkg(self, path): """Creates an Arch Linux package archive. @@ -148,11 +178,14 @@ def makepkg(self, path): util.mkdir(os.path.dirname(self.path)) # Generate package metadata + if self.mtree: + self.mtree = ["#mtree"] tar = tarfile.open(self.path, "w:gz") for name, data in archive_files: info = tarfile.TarInfo(name) info.size = len(data) tar.addfile(info, StringIO(data)) + self.mtreefile(info, data) # Generate package file system for name in self.files: @@ -162,18 +195,32 @@ def makepkg(self, path): info.mode = fileinfo["perms"] elif fileinfo["isdir"]: info.mode = 0755 + elif fileinfo["islink"]: + info.mode = 0777 if fileinfo["isdir"]: info.type = tarfile.DIRTYPE tar.addfile(info) + self.mtreedir(info) elif fileinfo["islink"]: info.type = tarfile.SYMTYPE info.linkname = fileinfo["link"] tar.addfile(info) + self.mtreelink(info) else: # TODO wow what a hack, adding a newline to match mkfile? filedata = name + "\n" info.size = len(filedata) tar.addfile(info, StringIO(filedata)) + self.mtreefile(info, filedata) + + # .MTREE + if self.mtree: + filedata = "\n".join(self.mtree)+"\n" # zlib.compress(filedata)? + # but that causes "Unrecognized archive format" error, and this + # seems to work anyway (libalpm is happy with uncompressed file) + info = tarfile.TarInfo(".MTREE") + info.size = len(filedata) + tar.addfile(info, StringIO(filedata)) tar.close() diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index a0a1455..0afb222 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -18,6 +18,7 @@ import os +import re import shlex import shutil import stat @@ -99,6 +100,9 @@ def load(self): self.localpkgs = [] self.createlocalpkgs = False self.filesystem = [] + self.rundir = self.rootjoin(util.TMPDIR) + self.genhook = [] + self.snaphook = [] self.description = "" self.option = {} @@ -198,12 +202,50 @@ def generate(self): # Done. vprint(" Taking a snapshot of the file system") + for cmd in self.genhook: + vprint("\texec "+cmd) + exec cmd for roots, dirs, files in os.walk(root): for i in files: filename = os.path.join(roots, i) f = pmfile.PacmanFile(root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name) + for cmd in self.snaphook: + vprint("\texec "+cmd) + exec cmd + + def pacmanbin(self): + return([self.env.pacman["bin"], + "--config", self.rootjoin(util.PACCONF), + "--root", self.env.root, + "--dbpath", self.rootjoin(util.PM_DBPATH), + "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) + + def pacmansub(self, cmd, output): + vprint("\trunning: %s" % " ".join(cmd)) + time_start = time.time() + retcode = subprocess.call(cmd, stdout=output, stderr=output, + cwd=self.rundir, env={'LC_ALL': 'C'}) + time_end = time.time() + vprint("\ttime elapsed: %.2fs" % (time_end - time_start)) + vprint("\tretcode = %s" % retcode) + return retcode + + def pacmanrun(self, args): + cmd = self.env.cmdroot() + cmd.extend(self.pacmanbin()) + cmd.append("--noconfirm") + cmd.extend(shlex.split(args)) + output = open(self.rootjoin(util.GENFILE), 'a') + self.pacmansub(cmd, output) + output.close() + + def addrule__pacman_warned(self, pkg, path, msg): + if not path.startswith(self.env.root): + path = self.rootjoin(path) + match = re.escape("warning: %s: %s (%s)" % (pkg.name, path, msg)) + self.addrule("PACMAN_OUTPUT=^"+match+"$") def run(self): if os.path.isfile(util.PM_LOCK): @@ -226,11 +268,7 @@ def run(self): "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", "--suppressions=%s" % suppfile]) - cmd.extend([pacman["bin"], - "--config", self.rootjoin(util.PACCONF), - "--root", root, - "--dbpath", self.rootjoin(util.PM_DBPATH), - "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) + cmd.extend(self.pacmanbin()) if not pacman["manual-confirm"]: cmd.append("--noconfirm") if pacman["debug"]: @@ -240,28 +278,17 @@ def run(self): output = open(self.rootjoin(util.LOGFILE), 'w') else: output = None - vprint("\trunning: %s" % " ".join(cmd)) - - # Change to the tmp dir before running pacman, so that local package - # archives are made available more easily. - tmpdir = self.rootjoin(util.TMPDIR) - time_start = time.time() - self.retcode = subprocess.call(cmd, stdout=output, stderr=output, - cwd=tmpdir, env={'LC_ALL': 'C'}) - time_end = time.time() - vprint("\ttime elapsed: %.2fs" % (time_end - time_start)) + self.retcode = self.pacmansub(cmd, output) if output: output.close() - vprint("\tretcode = %s" % self.retcode) - # Check if the lock is still there if os.path.isfile(util.PM_LOCK): tap.diag("\tERROR: %s not removed" % util.PM_LOCK) os.unlink(util.PM_LOCK) # Look for a core file - if os.path.isfile(os.path.join(tmpdir, "core")): + if os.path.isfile(os.path.join(self.rundir, "core")): tap.diag("\tERROR: pacman dumped a core file") def check(self): diff --git a/test/pacman/tests/querycheck001.py b/test/pacman/tests/querycheck001.py new file mode 100644 index 0000000..8587b50 --- /dev/null +++ b/test/pacman/tests/querycheck001.py @@ -0,0 +1,18 @@ +self.description = "Query--check files, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] + +self.addpkg(pkg) +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename()]) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=0") diff --git a/test/pacman/tests/querycheck002.py b/test/pacman/tests/querycheck002.py new file mode 100644 index 0000000..c385893 --- /dev/null +++ b/test/pacman/tests/querycheck002.py @@ -0,0 +1,24 @@ +self.description = "Query--check files, missing a file" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] + +self.addpkg(pkg) + +zap = pkg.files[1] + +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename(), + "os.unlink(self.rootjoin('%s'))" % zap]) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule__pacman_warned(pkg, zap, "No such file or directory") +self.addrule("PACMAN_OUTPUT= 1 missing file$") diff --git a/test/pacman/tests/querycheck003.py b/test/pacman/tests/querycheck003.py new file mode 100644 index 0000000..07e1bc9 --- /dev/null +++ b/test/pacman/tests/querycheck003.py @@ -0,0 +1,24 @@ +self.description = "Query--check files, missing a link" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] + +self.addpkg(pkg) + +zap = pkg.files[2].split(' -> ')[0] + +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename(), + "os.unlink(self.rootjoin('%s'))" % zap]) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule__pacman_warned(pkg, zap, "No such file or directory") +self.addrule("PACMAN_OUTPUT= 1 missing file$") diff --git a/test/pacman/tests/querycheck004.py b/test/pacman/tests/querycheck004.py new file mode 100644 index 0000000..dcf8173 --- /dev/null +++ b/test/pacman/tests/querycheck004.py @@ -0,0 +1,27 @@ +self.description = "Query--check files, missing a dir" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] + +self.addpkg(pkg) + +zap = pkg.files[3] + +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename(), + "shutil.rmtree(self.rootjoin('%s'))" % zap]) + +self.args = "-Qk" + +missing = "No such file or directory" + +self.addrule("PACMAN_RETCODE=1") +self.addrule__pacman_warned(pkg, zap, missing) +self.addrule__pacman_warned(pkg, pkg.files[4], missing) +self.addrule("PACMAN_OUTPUT= 2 missing files$") diff --git a/test/pacman/tests/querycheck005.py b/test/pacman/tests/querycheck005.py new file mode 100644 index 0000000..e673cc6 --- /dev/null +++ b/test/pacman/tests/querycheck005.py @@ -0,0 +1,33 @@ +self.description = "Query--check files, missing several" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] + +self.addpkg(pkg) + +zap1 = pkg.files[1] +zap2 = pkg.files[2].split(' -> ')[0] +zap3 = pkg.files[3] + +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename(), + "os.unlink(self.rootjoin('%s'))" % zap1, + "os.unlink(self.rootjoin('%s'))" % zap2, + "shutil.rmtree(self.rootjoin('%s'))" % zap3]) + +self.args = "-Qk" + +missing = "No such file or directory" + +self.addrule("PACMAN_RETCODE=1") +self.addrule__pacman_warned(pkg, zap1, missing) +self.addrule__pacman_warned(pkg, zap2, missing) +self.addrule__pacman_warned(pkg, zap3, missing) +self.addrule__pacman_warned(pkg, pkg.files[4], missing) +self.addrule("PACMAN_OUTPUT= 4 missing files$") diff --git a/test/pacman/tests/querycheck006.py b/test/pacman/tests/querycheck006.py new file mode 100644 index 0000000..cf178f4 --- /dev/null +++ b/test/pacman/tests/querycheck006.py @@ -0,0 +1,20 @@ +self.description = "Query--check mtree, no mtree" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] +pkg.mtree = False + +self.addpkg(pkg) +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename()]) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file") diff --git a/test/pacman/tests/querycheck007.py b/test/pacman/tests/querycheck007.py new file mode 100644 index 0000000..d43d846 --- /dev/null +++ b/test/pacman/tests/querycheck007.py @@ -0,0 +1,20 @@ +self.description = "Query--check mtree, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] +pkg.mtree = True + +self.addpkg(pkg) +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename()]) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=dummy: no mtree file") diff --git a/test/pacman/tests/querycheck008.py b/test/pacman/tests/querycheck008.py new file mode 100644 index 0000000..ddf9260 --- /dev/null +++ b/test/pacman/tests/querycheck008.py @@ -0,0 +1,33 @@ +self.description = "Query--check mtree, missing several" + +pkg = pmpkg("dummy") +pkg.files = [ + "etc/dummy.conf", + "lib/libdummy.so.0", + "lib/libdummy.so -> ./libdummy.so.0", + "lib/dummy/", + "lib/dummy/stuff", + "bin/dummy"] +pkg.mtree = True + +zap1 = pkg.files[1] +zap2 = pkg.files[2].split(' -> ')[0] +zap3 = pkg.files[3] + +self.addpkg(pkg) +self.genhook.extend([ + "self.pacmanrun('-U %s')" % pkg.filename(), + "os.unlink(self.rootjoin('%s'))" % zap1, + "os.unlink(self.rootjoin('%s'))" % zap2, + "shutil.rmtree(self.rootjoin('%s'))" % zap3]) + +self.args = "-Qkk" + +missing = "No such file or directory" + +self.addrule("PACMAN_RETCODE=1") +self.addrule__pacman_warned(pkg, zap1, missing) +self.addrule__pacman_warned(pkg, zap2, missing) +self.addrule__pacman_warned(pkg, zap3, missing) +self.addrule__pacman_warned(pkg, pkg.files[4], missing) +self.addrule("PACMAN_OUTPUT= 4 altered files$") diff --git a/test/pacman/tests/querycheck009.py b/test/pacman/tests/querycheck009.py new file mode 100644 index 0000000..6caf733 --- /dev/null +++ b/test/pacman/tests/querycheck009.py @@ -0,0 +1,30 @@ +self.description = "Query--check mtree, bad types" + +pkg = pmpkg("dummy") +pkg.mtree = True +pkg.files = ["dummy/z"] +for file, mangle in [ + ("dummy/f1", ["os.unlink('%s')", "os.mkdir('%s')"]), + ("dummy/f2", ["os.unlink('%s')", "os.symlink('z','%s')"]), + ("dummy/l1 -> z", ["os.unlink('%s')", "os.mkdir('%s')"]), + ("dummy/l2 -> z", ["os.unlink('%s')", "util.mkfile('%s')"]), + ("dummy/d1/", ["os.rmdir('%s')", "util.mkfile('%s')"]), + ("dummy/d2/", ["os.rmdir('%s')", "os.symlink('z','%s')"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file) + noslash = parsed.rstrip('/') + err = "File type mismatch" + if noslash != parsed: + err = "Not a directory" + rooted = self.rootjoin(noslash) + for cmd in mangle: + self.genhook.append(cmd % rooted) + self.addrule__pacman_warned(pkg, parsed, err) + +self.addpkg(pkg) +self.genhook.insert(0, "self.pacmanrun('-U %s')" % pkg.filename()) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT= 6 altered files$") diff --git a/test/pacman/tests/querycheck010.py b/test/pacman/tests/querycheck010.py new file mode 100644 index 0000000..caba37b --- /dev/null +++ b/test/pacman/tests/querycheck010.py @@ -0,0 +1,27 @@ +self.description = "Query--check mtree, bad perms+time+size+link" + +pkg = pmpkg("dummy") +pkg.mtree = True +pkg.files = ["dummy/"] +for file, err, mangle in [ + #("dummy/uid", "UID mismatch", ["os.chown('%s',?,-1)"]),#needs root + #("dummy/gid", "GID mismatch", ["os.chown('%s',-1,?)"]),#needs root + ("dummy/mode", "Permissions mismatch", ["os.chmod('%s',0400)"]), + ("dummy/mtime", "Modification time mismatch", ["os.utime('%s',None)"]), + ("dummy/size", "Size mismatch", ["util.mkfile('%s')"]), + ("dummy/link -> size", "Symlink path mismatch", [ + "os.unlink('%s')", "os.symlink('z','%s')"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file) + rooted = self.rootjoin(parsed) + for cmd in mangle: + self.genhook.append(cmd % rooted) + self.addrule__pacman_warned(pkg, parsed, err) + +self.addpkg(pkg) +self.genhook.insert(0, "self.pacmanrun('-U %s')" % pkg.filename()) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT= 4 altered files$") diff --git a/test/pacman/util.py b/test/pacman/util.py index 5c9a0c0..d2df8dc 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -40,6 +40,7 @@ TMPDIR = "tmp" SYNCREPO = "var/pub" LOGFILE = "var/log/pactest.log" +GENFILE = "var/log/testgen.log" verbose = 0 -- 1.8.4

Andrew Gregory

4 Oct 4 Oct

5:22 p.m.

New subject: [pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

On 10/02/13 at 11:48am, Jeremy Heiner wrote:

...

Two new "hooks" were made available in pmtest: genhook is called after everything is generated (just before the snapshot is taken), and snaphook is called just after the snapshot. A hook is a list of strings. Calling a hook is "exec"ing each string in the list.

Some helper functions were added to pmtest, notably pacmanrun and addrule__pacman_warned. pacmanrun and rootjoin are used in a hook to install a package and then intentionally mess it up, which then allows the "-Qk" test output to be verified (with addrule__pacman_warned).

Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmpkg.py | 47 ++++++++++++++++++++++++++++ test/pacman/pmtest.py | 63 +++++++++++++++++++++++++++----------- test/pacman/tests/querycheck001.py | 18 +++++++++++ test/pacman/tests/querycheck002.py | 24 +++++++++++++++ test/pacman/tests/querycheck003.py | 24 +++++++++++++++ test/pacman/tests/querycheck004.py | 27 ++++++++++++++++ test/pacman/tests/querycheck005.py | 33 ++++++++++++++++++++ test/pacman/tests/querycheck006.py | 20 ++++++++++++ test/pacman/tests/querycheck007.py | 20 ++++++++++++ test/pacman/tests/querycheck008.py | 33 ++++++++++++++++++++ test/pacman/tests/querycheck009.py | 30 ++++++++++++++++++ test/pacman/tests/querycheck010.py | 27 ++++++++++++++++ test/pacman/util.py | 1 + 13 files changed, 349 insertions(+), 18 deletions(-) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py create mode 100644 test/pacman/tests/querycheck005.py create mode 100644 test/pacman/tests/querycheck006.py create mode 100644 test/pacman/tests/querycheck007.py create mode 100644 test/pacman/tests/querycheck008.py create mode 100644 test/pacman/tests/querycheck009.py create mode 100644 test/pacman/tests/querycheck010.py

There is A LOT going on in this patch. Please try to break your patches up into smaller changes. At least mtree and hook support definitely should have been individual patches. Before I get into specifics, pacman already has "hooks" of a sort: install scripts. I think I would prefer to see support added for install scripts in local db packages than the ability to run arbitrary python code. You could then modify files as needed from post_install(). This would match our existing use of install scripts as hooks and could be run within fakechroot. Anybody else have any thoughts on this?

...

diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 9c9e447..bb0facf 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -23,6 +23,8 @@ import shutil from StringIO import StringIO import tarfile +import hashlib +import zlib

import util

@@ -59,6 +61,7 @@ def __init__(self, name, version = "1.0-1"): # files self.files = [] self.backup = [] + self.mtree = False # install self.install = { "pre_install": "", @@ -104,6 +107,33 @@ def parse_filename(name): filename, extra = filename.split("|") return filename

+ def mtreefile(self, info, data): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=file size=%d md5digest=%s sha256digest=%s" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF, + info.size, hashlib.md5(data).hexdigest(), + hashlib.sha256(data).hexdigest())) + + def mtreedir(self, info): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=dir" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF)) + + def mtreelink(self, info): + if self.mtree: + self.mtree.append( + "./%s uid=%d gid=%d mode=%o time=%d.%d" + " type=link link=%s" + % (info.name, info.uid, info.gid, info.mode, + info.mtime >> 32, info.mtime & 0xFFFFFFFF, + info.linkname)) +

Since these alter the mtree, it would be nice to have 'add' in the function names to make that clear. A comment explaining the bit operations on mtime would also be nice

...

def makepkg(self, path): """Creates an Arch Linux package archive.

@@ -148,11 +178,14 @@ def makepkg(self, path): util.mkdir(os.path.dirname(self.path))

# Generate package metadata + if self.mtree: + self.mtree = ["#mtree"]

I don't like this dual use of mtree as a boolean and then a list. It would be clearer to use something like buildmtree for the boolean option.

...

tar = tarfile.open(self.path, "w:gz") for name, data in archive_files: info = tarfile.TarInfo(name) info.size = len(data) tar.addfile(info, StringIO(data)) + self.mtreefile(info, data)

# Generate package file system for name in self.files: @@ -162,18 +195,32 @@ def makepkg(self, path): info.mode = fileinfo["perms"] elif fileinfo["isdir"]: info.mode = 0755 + elif fileinfo["islink"]: + info.mode = 0777 if fileinfo["isdir"]: info.type = tarfile.DIRTYPE tar.addfile(info) + self.mtreedir(info) elif fileinfo["islink"]: info.type = tarfile.SYMTYPE info.linkname = fileinfo["link"] tar.addfile(info) + self.mtreelink(info) else: # TODO wow what a hack, adding a newline to match mkfile? filedata = name + "\n" info.size = len(filedata) tar.addfile(info, StringIO(filedata)) + self.mtreefile(info, filedata) + + # .MTREE + if self.mtree: + filedata = "\n".join(self.mtree)+"\n" # zlib.compress(filedata)? + # but that causes "Unrecognized archive format" error, and this + # seems to work anyway (libalpm is happy with uncompressed file) + info = tarfile.TarInfo(".MTREE") + info.size = len(filedata) + tar.addfile(info, StringIO(filedata))

tar.close()

diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index a0a1455..0afb222 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -18,6 +18,7 @@

import os +import re import shlex import shutil import stat @@ -99,6 +100,9 @@ def load(self): self.localpkgs = [] self.createlocalpkgs = False self.filesystem = [] + self.rundir = self.rootjoin(util.TMPDIR) + self.genhook = [] + self.snaphook = []

self.description = "" self.option = {} @@ -198,12 +202,50 @@ def generate(self):

# Done. vprint(" Taking a snapshot of the file system") + for cmd in self.genhook: + vprint("\texec "+cmd) + exec cmd for roots, dirs, files in os.walk(root): for i in files: filename = os.path.join(roots, i) f = pmfile.PacmanFile(root, self.rootremove(filename)) self.files.append(f) vprint("\t%s" % f.name) + for cmd in self.snaphook: + vprint("\texec "+cmd) + exec cmd + + def pacmanbin(self): + return([self.env.pacman["bin"], + "--config", self.rootjoin(util.PACCONF), + "--root", self.env.root, + "--dbpath", self.rootjoin(util.PM_DBPATH), + "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) + + def pacmansub(self, cmd, output): + vprint("\trunning: %s" % " ".join(cmd)) + time_start = time.time() + retcode = subprocess.call(cmd, stdout=output, stderr=output, + cwd=self.rundir, env={'LC_ALL': 'C'}) + time_end = time.time() + vprint("\ttime elapsed: %.2fs" % (time_end - time_start)) + vprint("\tretcode = %s" % retcode) + return retcode + + def pacmanrun(self, args): + cmd = self.env.cmdroot() + cmd.extend(self.pacmanbin()) + cmd.append("--noconfirm") + cmd.extend(shlex.split(args)) + output = open(self.rootjoin(util.GENFILE), 'a') + self.pacmansub(cmd, output) + output.close()

The only reason to refactor this code into functions seems to be so that you can run pacman from genhook, but you only do that to install packages, which should actually be done by adding them to the local db, so I'm not really sure what the point of this is. Furthermore, I'm not sure how wise it is to use pacman to set up its own test environment.

...

+ + def addrule__pacman_warned(self, pkg, path, msg): + if not path.startswith(self.env.root): + path = self.rootjoin(path) + match = re.escape("warning: %s: %s (%s)" % (pkg.name, path, msg)) + self.addrule("PACMAN_OUTPUT=^"+match+"$")

I am against this. For starters, not all of pacman's warnings match this format, so the name is not appropriate. Also, I'm not against helper functions for rules generally, but I'd rather them not be quite so specialized as this and they should be put under pmrule. self.addrule(pmrule.foo(...))

...

def run(self): if os.path.isfile(util.PM_LOCK): @@ -226,11 +268,7 @@ def run(self): "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", "--suppressions=%s" % suppfile]) - cmd.extend([pacman["bin"], - "--config", self.rootjoin(util.PACCONF), - "--root", root, - "--dbpath", self.rootjoin(util.PM_DBPATH), - "--cachedir", self.rootjoin(util.PM_CACHEDIR)]) + cmd.extend(self.pacmanbin()) if not pacman["manual-confirm"]: cmd.append("--noconfirm") if pacman["debug"]: @@ -240,28 +278,17 @@ def run(self): output = open(self.rootjoin(util.LOGFILE), 'w') else: output = None - vprint("\trunning: %s" % " ".join(cmd)) - - # Change to the tmp dir before running pacman, so that local package - # archives are made available more easily. - tmpdir = self.rootjoin(util.TMPDIR) - time_start = time.time() - self.retcode = subprocess.call(cmd, stdout=output, stderr=output, - cwd=tmpdir, env={'LC_ALL': 'C'}) - time_end = time.time() - vprint("\ttime elapsed: %.2fs" % (time_end - time_start))

+ self.retcode = self.pacmansub(cmd, output) if output: output.close()

- vprint("\tretcode = %s" % self.retcode) - # Check if the lock is still there if os.path.isfile(util.PM_LOCK): tap.diag("\tERROR: %s not removed" % util.PM_LOCK) os.unlink(util.PM_LOCK) # Look for a core file - if os.path.isfile(os.path.join(tmpdir, "core")): + if os.path.isfile(os.path.join(self.rundir, "core")): tap.diag("\tERROR: pacman dumped a core file")

def check(self):

Jeremy Heiner

7:18 p.m.

New subject: [pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

On Fri, Oct 4, 2013 at 11:22 AM, Andrew Gregory wrote:

...

There is A LOT going on in this patch. Please try to break your patches up into smaller changes. At least mtree and hook support definitely should have been individual patches.

Hi, Andrew, Thanks for the code review! Sorry about the size of this patch. It's really due to the fact that these things grew organically. If I had been known before I started exactly what I needed from the mtree and hook support in order to meet my goals, well, then I would have won several lotteries by now. I'd be happy to reverse-engineer some additional commits to make it look like I was more prescient :).

...

Before I get into specifics, pacman already has "hooks" of a sort: install scripts. I think I would prefer to see support added for install scripts in local db packages than the ability to run arbitrary python code. You could then modify files as needed from post_install(). This would match our existing use of install scripts as hooks and could be run within fakechroot. Anybody else have any thoughts on this?

I really like that idea. The reason I didn't go down that path was that I saw what pmpkg.install_package contained, or rather did not contain, and was scared off.

...

The only reason to refactor this code into functions seems to be so that you can run pacman from genhook, but you only do that to install packages, which should actually be done by adding them to the local db, so I'm not really sure what the point of this is. Furthermore, I'm not sure how wise it is to use pacman to set up its own test environment.

Testing -Qk(k) requires /var/lib/pacman/local/* to be populated, which is not something pmpkg does. So how much more does pmpkg need to do? The more it duplicates what pacman does, the higher the ongoing code maintenance cost. So where's the sweet spot? I would argue that pmpkg should do the minimum (i.e. only what it currently does). And that there is no danger in using pacman to set up its own test environment because there are extensive unit tests for the installation features. And if those pass, then where is the danger? It might be nice to add "this test should fail without even running unless that test has succeeded" to the specifications, but that seems a bit overkill. One more subtle thing to keep in mind is the timing of the snapshot. Now I didn't really dig deeply into how all the tests make use of the snapshot, but for some tests you want it taken between the install and the mangling. There is no way to achieve this effect using local pmpkgs. But I want to say again that I really like the idea. There's no question it is superior to my hooks mechanism in the complexity of the testing framework and even the generality. I like it so much that I'm going to delay responding to the other points from your code review to provide an opportunity for feedback. I think it should be possible to find workarounds for the issues I mentioned above, and if that's the case then I would happily abandon my hooks idea in favor of install scripts. Thanks again, Jeremy

Andrew Gregory

9:58 p.m.

New subject: [pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

On 10/04/13 at 01:18pm, Jeremy Heiner wrote:

...

On Fri, Oct 4, 2013 at 11:22 AM, Andrew Gregory wrote:

...
There is A LOT going on in this patch. Please try to break your patches up into smaller changes. At least mtree and hook support definitely should have been individual patches.

Hi, Andrew, Thanks for the code review! Sorry about the size of this patch. It's really due to the fact that these things grew organically. If I had been known before I started exactly what I needed from the mtree and hook support in order to meet my goals, well, then I would have won several lotteries by now. I'd be happy to reverse-engineer some additional commits to make it look like I was more prescient :).

...
Before I get into specifics, pacman already has "hooks" of a sort: install scripts. I think I would prefer to see support added for install scripts in local db packages than the ability to run arbitrary python code. You could then modify files as needed from post_install(). This would match our existing use of install scripts as hooks and could be run within fakechroot. Anybody else have any thoughts on this?

I really like that idea. The reason I didn't go down that path was that I saw what pmpkg.install_package contained, or rather did not contain, and was scared off.

...
The only reason to refactor this code into functions seems to be so that you can run pacman from genhook, but you only do that to install packages, which should actually be done by adding them to the local db, so I'm not really sure what the point of this is. Furthermore, I'm not sure how wise it is to use pacman to set up its own test environment.

Testing -Qk(k) requires /var/lib/pacman/local/* to be populated, which is not something pmpkg does. So how much more does pmpkg need to do? The more it duplicates what pacman does, the higher the ongoing code maintenance cost. So where's the sweet spot?

pmdb is responsible for populating /var/lib/pacman/local/. In fact, you'll need to update pmdb so it will create the mtree file, which I seem to have neglected to mention before.

...

I would argue that pmpkg should do the minimum (i.e. only what it currently does). And that there is no danger in using pacman to set up its own test environment because there are extensive unit tests for the installation features. And if those pass, then where is the danger? It might be nice to add "this test should fail without even running unless that test has succeeded" to the specifications, but that seems a bit overkill.

One more subtle thing to keep in mind is the timing of the snapshot. Now I didn't really dig deeply into how all the tests make use of the snapshot, but for some tests you want it taken between the install and the mangling. There is no way to achieve this effect using local pmpkgs.

The snapshot is used to determine if the file was modified after being installed. None of your tests needed a post-snapshot hook, and I can't think of any that would. Of course, if such a need does arise, we can revisit the idea of post-snapshot hooks at that time.

...

But I want to say again that I really like the idea. There's no question it is superior to my hooks mechanism in the complexity of the testing framework and even the generality. I like it so much that I'm going to delay responding to the other points from your code review to provide an opportunity for feedback. I think it should be possible to find workarounds for the issues I mentioned above, and if that's the case then I would happily abandon my hooks idea in favor of install scripts. Thanks again, Jeremy

Jeremy Heiner

5 Oct 5 Oct

12:02 p.m.

New subject: [pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

On Fri, Oct 4, 2013 at 3:58 PM, Andrew Gregory wrote:

...

pmdb is responsible for populating /var/lib/pacman/local/. In fact, you'll need to update pmdb so it will create the mtree file, which I seem to have neglected to mention before.

D'oh! Yes, indeed, it does! How embarrassing that I somehow missed that. I assumed incorrectly based on its name and by analogy to what pacman does that pmpkg.install_package would be responsible for populating /var/lib/pacman/local. Oops. Obviously this patch set is D.O.A.

...

The snapshot is used to determine if the file was modified after being installed. None of your tests needed a post-snapshot hook, and I can't think of any that would. Of course, if such a need does arise, we can revisit the idea of post-snapshot hooks at that time.

Correct, none of the -Qk(k) tests need that hook. But I mentioned in the cover-letter a scenario that I thought would: reinstalling to repair damaged files. Perhaps it's just that I'm still not completely understanding how the snapshot gets used, but that scenario still seems to me to need something more than what the local pmdb does. I am quite content, however, to put that aside until the day a test is written that actually requires it. So, I'll get to work on an improved version of this patch set. My hooks code is out. The mtree generation gets moved to where it belongs. And I need to figure out how to call the install script - which (without looking at the code at all, so I could be wrong) involves forking sh as a final step in pmpkg.install_package. And roll in your other suggested improvements as well. Piece o' cake! :) Thanks, Jeremy

Allan McRae

3 Oct 3 Oct

4:59 a.m.

On 03/10/13 01:48, Jeremy Heiner wrote:

...

The hooks mechanism created here could potentially be used to write unit tests for other scenarios... For example: reinstalling to repair damaged files. The genhook would do the install (so the snapshot would match the desired final state), the snaphook would damage a file or two, and the reinstall-from-local-db would be the test action proper.

Jeremy Heiner (3): Add pmtest.rootjoin() and clean up some duplicate joins. Simplify pmtest.generate and .run API (removed pacman parameter). Added tests for -Q --check (both fast(files) and full(mtree)).

I'd appreciate if someone else can review these for me. I rarely deal with the test suite and my python knowledge is limited, so reviewing test suite patches takes me too much time. Allan

3861

Age (days ago)

3864

Last active (days ago)

List overview

Download

10 comments

3 participants

participants (3)

Allan McRae
Andrew Gregory
Jeremy Heiner