[pacman-dev] [PATCH 0/1] Change to the way pmenv and pmtest connect.
This change was needed in a rescinded patch, and even though it is no longer required I still think it is an improvement that may be worth keeping. Prior to this change each pmtest searched the PATH envar to get the full paths to the fakeroot and fakechroot commands (used to fork the pacman binary being tested). That search logically fits better in the pmenv. And that change leads naturally to a slightly less verbose API for when the pmenv is calling pmtest methods. Jeremy Heiner (1): Simplify pmtest.generate and .run API (removed pacman parameter). test/pacman/pmenv.py | 13 ++++++++++--- test/pacman/pmtest.py | 30 +++++++++++++----------------- 2 files changed, 23 insertions(+), 20 deletions(-) -- 1.8.4.2
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 | 13 ++++++++++--- test/pacman/pmtest.py | 30 +++++++++++++----------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py index f358285..2da6566 100644 --- a/test/pacman/pmenv.py +++ b/test/pacman/pmenv.py @@ -21,6 +21,7 @@ import pmtest import tap +import util class pmenv(object): @@ -42,6 +43,12 @@ def __init__(self, root = "root"): "valgrind": 0, "nolog": 0 } + self.fakecmds = [] if os.geteuid() == 0 else [ + cmd + for cmd in ["fakeroot", "fakechroot"] + for found in [util.which(cmd) or # diag returns None... + tap.diag("WARNING: %s not found!" % cmd)] + if found] def __str__(self): return "root = %s\n" \ @@ -53,7 +60,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 +71,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/pmtest.py b/test/pacman/pmtest.py index a1f3645d..6c02ac5 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -35,12 +35,16 @@ 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 + @property + def root(self): + return self.env.root + def __str__(self): return "name = %s\n" \ "testname = %s\n" \ @@ -105,7 +109,9 @@ def load(self): else: raise IOError("file %s does not exist!" % self.name) - def generate(self, pacman): + def generate(self): + pacman = self.env.pacman + tap.diag("==> Generating test environment") # Cleanup leftover files from a previous test session @@ -192,7 +198,9 @@ def generate(self, pacman): self.files.append(f) vprint("\t%s" % f.name) - def run(self, pacman): + def run(self): + pacman = self.env.pacman + if os.path.isfile(util.PM_LOCK): tap.bail("\tERROR: another pacman session is on-going -- skipping") return @@ -200,19 +208,7 @@ 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") + cmd = self.env.fakecmds[:] if pacman["gdb"]: cmd.extend(["libtool", "execute", "gdb", "--args"]) -- 1.8.4.2
On Mon, Nov 11, 2013 at 09:16:42AM -0500, 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 | 13 ++++++++++--- test/pacman/pmtest.py | 30 +++++++++++++----------------- 2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py index f358285..2da6566 100644 --- a/test/pacman/pmenv.py +++ b/test/pacman/pmenv.py @@ -21,6 +21,7 @@
import pmtest import tap +import util
class pmenv(object): @@ -42,6 +43,12 @@ def __init__(self, root = "root"): "valgrind": 0, "nolog": 0 } + self.fakecmds = [] if os.geteuid() == 0 else [ + cmd + for cmd in ["fakeroot", "fakechroot"] + for found in [util.which(cmd) or # diag returns None... + tap.diag("WARNING: %s not found!" % cmd)] + if found]
Nested list comprehensions are completely unreadable at a glance. There's no gain in doing this. Can we get rid of the some of the clever for the sake of readability? Something like... self.fakecmds = [] if os.geteuid() != 0: for cmd in ("fakeroot", "fakechroot"): resolved_path = util.which(cmd) if resolved_path: self.fakecmds.append(resolved) else: tap.diag("WARNING: %s not found!" % cmd)
def __str__(self): return "root = %s\n" \ @@ -53,7 +60,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 +71,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/pmtest.py b/test/pacman/pmtest.py index a1f3645d..6c02ac5 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -35,12 +35,16 @@ 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
+ @property + def root(self): + return self.env.root + def __str__(self): return "name = %s\n" \ "testname = %s\n" \ @@ -105,7 +109,9 @@ def load(self): else: raise IOError("file %s does not exist!" % self.name)
- def generate(self, pacman): + def generate(self): + pacman = self.env.pacman + tap.diag("==> Generating test environment")
# Cleanup leftover files from a previous test session @@ -192,7 +198,9 @@ def generate(self, pacman): self.files.append(f) vprint("\t%s" % f.name)
- def run(self, pacman): + def run(self): + pacman = self.env.pacman + if os.path.isfile(util.PM_LOCK): tap.bail("\tERROR: another pacman session is on-going -- skipping") return @@ -200,19 +208,7 @@ 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") + cmd = self.env.fakecmds[:]
if pacman["gdb"]: cmd.extend(["libtool", "execute", "gdb", "--args"]) -- 1.8.4.2
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 | 14 +++++++++++--- test/pacman/pmtest.py | 30 +++++++++++++----------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py index f358285..93223f4 100644 --- a/test/pacman/pmenv.py +++ b/test/pacman/pmenv.py @@ -21,6 +21,7 @@ import pmtest import tap +import util class pmenv(object): @@ -42,6 +43,13 @@ def __init__(self, root = "root"): "valgrind": 0, "nolog": 0 } + self.fakecmds = [] + if os.geteuid() != 0: + for cmd in ["fakeroot", "fakechroot"]: + if not util.which(cmd): + tap.diag("WARNING: %s not found!" % cmd) + else: + self.fakecmds.append(cmd) def __str__(self): return "root = %s\n" \ @@ -53,7 +61,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 +72,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/pmtest.py b/test/pacman/pmtest.py index a1f3645d..6c02ac5 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -35,12 +35,16 @@ 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 + @property + def root(self): + return self.env.root + def __str__(self): return "name = %s\n" \ "testname = %s\n" \ @@ -105,7 +109,9 @@ def load(self): else: raise IOError("file %s does not exist!" % self.name) - def generate(self, pacman): + def generate(self): + pacman = self.env.pacman + tap.diag("==> Generating test environment") # Cleanup leftover files from a previous test session @@ -192,7 +198,9 @@ def generate(self, pacman): self.files.append(f) vprint("\t%s" % f.name) - def run(self, pacman): + def run(self): + pacman = self.env.pacman + if os.path.isfile(util.PM_LOCK): tap.bail("\tERROR: another pacman session is on-going -- skipping") return @@ -200,19 +208,7 @@ 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") + cmd = self.env.fakecmds[:] if pacman["gdb"]: cmd.extend(["libtool", "execute", "gdb", "--args"]) -- 1.8.4.2
participants (2)
-
Dave Reisner
-
Jeremy Heiner