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