[pacman-dev] [PATCH 0/5] Unit tests for -Qk and -Qkk
There are two changes to the test framework to allow these new unit tests: PATCH 2 adds code to run pre and post install scripts for "local" packages, and PATCH 4 adds code to generate mtree data (if the unit test requests it). Thanks again to everyone who provided feedback on my previous attempt at this... hopefully the improvement your suggestions inspired will be obvious. Jeremy Heiner (5): Add the unit tests for -Qk and -Qkk that are possible now. Add unit tests for -Qk and -Qkk with missing files. Simplify the Python code that sets utime of test package files. Add mtree code to test framework and basic unit tests for -Qkk. Add unit tests for -Qkk with altered files. test/pacman/pmdb.py | 3 ++ test/pacman/pmpkg.py | 36 +++++++++++++++++++--- test/pacman/pmtest.py | 5 +--- test/pacman/tests/TESTS | 8 +++++ test/pacman/tests/querycheck001.py | 15 ++++++++++ test/pacman/tests/querycheck002.py | 15 ++++++++++ test/pacman/tests/querycheck003.py | 19 ++++++++++++ test/pacman/tests/querycheck004.py | 17 +++++++++++ test/pacman/tests/querycheck005.py | 16 ++++++++++ test/pacman/tests/querycheck006.py | 20 +++++++++++++ test/pacman/tests/querycheck007.py | 25 ++++++++++++++++ test/pacman/tests/querycheck008.py | 25 ++++++++++++++++ test/pacman/util.py | 61 +++++++++++++++++++++++++++++++++++++- 13 files changed, 256 insertions(+), 9 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 -- 1.8.4.2
The -Qk test (001) validates the existence of the package files (which were installed to the filesystem by the framework because the package was added to the "local" db). The -Qkk test (002) does not validate any file's properties - it can only check that the pacman run produces the expected warning message saying that the package lacks an mtree. Further tests will require modifications to the testing framework to allow intentional damage to the filesystem and generating an mtree. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck001.py | 15 +++++++++++++++ test/pacman/tests/querycheck002.py | 15 +++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index fc6a7e8..0997f58 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -88,6 +88,8 @@ TESTS += test/pacman/tests/query007.py TESTS += test/pacman/tests/query010.py TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py +TESTS += test/pacman/tests/querycheck001.py +TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck001.py b/test/pacman/tests/querycheck001.py new file mode 100644 index 0000000..21bcba2 --- /dev/null +++ b/test/pacman/tests/querycheck001.py @@ -0,0 +1,15 @@ +self.description = "Query--check files, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=warning") diff --git a/test/pacman/tests/querycheck002.py b/test/pacman/tests/querycheck002.py new file mode 100644 index 0000000..579d4d1 --- /dev/null +++ b/test/pacman/tests/querycheck002.py @@ -0,0 +1,15 @@ +self.description = "Query--check mtree, no mtree" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file") -- 1.8.4.2
On 11/11/13 21:47, Jeremy Heiner wrote:
The -Qk test (001) validates the existence of the package files (which were installed to the filesystem by the framework because the package was added to the "local" db).
The -Qkk test (002) does not validate any file's properties - it can only check that the pacman run produces the expected warning message saying that the package lacks an mtree.
Further tests will require modifications to the testing framework to allow intentional damage to the filesystem and generating an mtree.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> ---
Pulled.
test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck001.py | 15 +++++++++++++++ test/pacman/tests/querycheck002.py | 15 +++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index fc6a7e8..0997f58 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -88,6 +88,8 @@ TESTS += test/pacman/tests/query007.py TESTS += test/pacman/tests/query010.py TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py +TESTS += test/pacman/tests/querycheck001.py +TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck001.py b/test/pacman/tests/querycheck001.py new file mode 100644 index 0000000..21bcba2 --- /dev/null +++ b/test/pacman/tests/querycheck001.py @@ -0,0 +1,15 @@ +self.description = "Query--check files, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=warning") diff --git a/test/pacman/tests/querycheck002.py b/test/pacman/tests/querycheck002.py new file mode 100644 index 0000000..579d4d1 --- /dev/null +++ b/test/pacman/tests/querycheck002.py @@ -0,0 +1,15 @@ +self.description = "Query--check mtree, no mtree" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file")
On 11/11/13 at 06:47am, Jeremy Heiner wrote:
The -Qk test (001) validates the existence of the package files (which were installed to the filesystem by the framework because the package was added to the "local" db).
The -Qkk test (002) does not validate any file's properties - it can only check that the pacman run produces the expected warning message saying that the package lacks an mtree.
Further tests will require modifications to the testing framework to allow intentional damage to the filesystem and generating an mtree.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck001.py | 15 +++++++++++++++ test/pacman/tests/querycheck002.py | 15 +++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index fc6a7e8..0997f58 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -88,6 +88,8 @@ TESTS += test/pacman/tests/query007.py TESTS += test/pacman/tests/query010.py TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py +TESTS += test/pacman/tests/querycheck001.py +TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck001.py b/test/pacman/tests/querycheck001.py new file mode 100644 index 0000000..21bcba2 --- /dev/null +++ b/test/pacman/tests/querycheck001.py @@ -0,0 +1,15 @@ +self.description = "Query--check files, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=warning")
This fails if pacman is built without libcurl.
diff --git a/test/pacman/tests/querycheck002.py b/test/pacman/tests/querycheck002.py new file mode 100644 index 0000000..579d4d1 --- /dev/null +++ b/test/pacman/tests/querycheck002.py @@ -0,0 +1,15 @@ +self.description = "Query--check mtree, no mtree" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file") -- 1.8.4.2
On 31/12/13 04:17, Andrew Gregory wrote:
On 11/11/13 at 06:47am, Jeremy Heiner wrote:
The -Qk test (001) validates the existence of the package files (which were installed to the filesystem by the framework because the package was added to the "local" db).
The -Qkk test (002) does not validate any file's properties - it can only check that the pacman run produces the expected warning message saying that the package lacks an mtree.
Further tests will require modifications to the testing framework to allow intentional damage to the filesystem and generating an mtree.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck001.py | 15 +++++++++++++++ test/pacman/tests/querycheck002.py | 15 +++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 test/pacman/tests/querycheck001.py create mode 100644 test/pacman/tests/querycheck002.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index fc6a7e8..0997f58 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -88,6 +88,8 @@ TESTS += test/pacman/tests/query007.py TESTS += test/pacman/tests/query010.py TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py +TESTS += test/pacman/tests/querycheck001.py +TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck001.py b/test/pacman/tests/querycheck001.py new file mode 100644 index 0000000..21bcba2 --- /dev/null +++ b/test/pacman/tests/querycheck001.py @@ -0,0 +1,15 @@ +self.description = "Query--check files, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=warning")
This fails if pacman is built without libcurl.
Change made: -self.addrule("!PACMAN_OUTPUT=warning") +self.addrule("!PACMAN_OUTPUT=warning.*(No such file or directory)")
pmpkg.install_package (which is called by the framework to populate the filesystem with "local" db packages) now executes any pre_install and/or post_install scripts it finds. Prior to this commit no unit test had a "local" package with such a script. The two new tests in this commit use post_install to remove some installed files. As with the previous commit, the -Qk test (003) validates the filesystem (warning about the missing files), while the -Qkk test (004) can only check for the warning about the lack of an mtree. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmpkg.py | 13 +++++++++++++ test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck003.py | 19 +++++++++++++++++++ test/pacman/tests/querycheck004.py | 17 +++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 6f7ae6e..fd469b1 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -22,6 +22,7 @@ import stat import shutil from StringIO import StringIO +import subprocess import tarfile import util @@ -177,13 +178,25 @@ def makepkg(self, path): tar.close() + def runscript(self, root, name): + script = self.install[name] + if script: + util.vprint("running %s script: %s" % (name, script)) + # note: there are differences between the environment the script + # gets from here versus the one the pacman binary would give it. + # in particular there's no chroot, so use relative paths. + subprocess.call(["sh", "-c", script], + cwd=os.path.join(root,util.TMPDIR)) + def install_package(self, root): """Install the package in the given root.""" + self.runscript(root, "pre_install") for f in self.files: util.mkfile(root, f, f) path = os.path.join(root, f) if os.path.isfile(path): os.utime(path, (355, 355)) + self.runscript(root, "post_install") def filelist(self): """Generate a list of package files.""" diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 0997f58..f4e6caf 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -90,6 +90,8 @@ TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py TESTS += test/pacman/tests/querycheck001.py TESTS += test/pacman/tests/querycheck002.py +TESTS += test/pacman/tests/querycheck003.py +TESTS += test/pacman/tests/querycheck004.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck003.py b/test/pacman/tests/querycheck003.py new file mode 100644 index 0000000..3181188 --- /dev/null +++ b/test/pacman/tests/querycheck003.py @@ -0,0 +1,19 @@ +self.description = "Query--check files, missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/ .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/msgs .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/en .No such file") +self.addrule("PACMAN_OUTPUT=, 3 missing files") diff --git a/test/pacman/tests/querycheck004.py b/test/pacman/tests/querycheck004.py new file mode 100644 index 0000000..0d2015a --- /dev/null +++ b/test/pacman/tests/querycheck004.py @@ -0,0 +1,17 @@ +self.description = "Query--check mtree, no mtree + missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +# the existence of files is not checked when there is no mtree +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file") -- 1.8.4.2
On 11/11/13 at 06:47am, Jeremy Heiner wrote:
pmpkg.install_package (which is called by the framework to populate the filesystem with "local" db packages) now executes any pre_install and/or post_install scripts it finds. Prior to this commit no unit test had a "local" package with such a script. The two new tests in this commit use post_install to remove some installed files.
As with the previous commit, the -Qk test (003) validates the filesystem (warning about the missing files), while the -Qkk test (004) can only check for the warning about the lack of an mtree.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmpkg.py | 13 +++++++++++++ test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck003.py | 19 +++++++++++++++++++ test/pacman/tests/querycheck004.py | 17 +++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 6f7ae6e..fd469b1 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -22,6 +22,7 @@ import stat import shutil from StringIO import StringIO +import subprocess import tarfile
import util @@ -177,13 +178,25 @@ def makepkg(self, path):
tar.close()
+ def runscript(self, root, name): + script = self.install[name] + if script: + util.vprint("running %s script: %s" % (name, script)) + # note: there are differences between the environment the script + # gets from here versus the one the pacman binary would give it. + # in particular there's no chroot, so use relative paths. + subprocess.call(["sh", "-c", script], + cwd=os.path.join(root,util.TMPDIR)) +
One of the main reasons I suggested adding install script support for this was so that they would work the same as scripts run by pacman and therefore be easy to write and understand. At the very least, they should be run in root, not TMPDIR and honor env["scriptlet-shell"]. This should also be run with fakeroot (use -i and -s to save state between invocations) and be run inside a fake chroot. The only downside I see is the need to copy binaries into the test chroot. It looks like your tests would require us to copy at least chown, chmod, rm, touch, and ln into the chroot. Is there any problem with copying the additional binaries?
def install_package(self, root): """Install the package in the given root.""" + self.runscript(root, "pre_install") for f in self.files: util.mkfile(root, f, f) path = os.path.join(root, f) if os.path.isfile(path): os.utime(path, (355, 355)) + self.runscript(root, "post_install")
def filelist(self): """Generate a list of package files."""
On Sat, Dec 14, 2013 at 4:03 PM, Andrew Gregory <andrew.gregory.8@gmail.com> wrote:
One of the main reasons I suggested adding install script support for this was so that they would work the same as scripts run by pacman and therefore be easy to write and understand. At the very least, they should be run in root, not TMPDIR and honor env["scriptlet-shell"]. This should also be run with fakeroot (use -i and -s to save state between invocations) and be run inside a fake chroot. The only downside I see is the need to copy binaries into the test chroot. It looks like your tests would require us to copy at least chown, chmod, rm, touch, and ln into the chroot. Is there any problem with copying the additional binaries?
Hey Andrew, thanks for the code review! I obviously wrote these patches following the "less is more" principle. Every added line of code is a future potential headache. And if I don't have a solid use-case argument backing up my edits I'm not going to put my name up for blame. I can get my head around the argument that running scripts from the test root is much less surprising. That gives me what I need to put my name on that additional bit of code. But I don't have enough yet for me to feel comfortable taking responsibility for your other suggestions... The --scriptlet-shell code makes zero sense to me. The help string in configure doesn't match the implementation. There is a hard-coded ref to '/bin/sh' in pmtest.generate, so it is guaranteed that 'bin/sh' is available under the test root. All --scriptlet-shell does is install a 2nd copy of '/bin/sh' under a different path, and maybe I'm missing something because that seems to accomplish nothing. What is the point of telling the test framework to use an alias? Anyway, I would feel uncomfortable submitting a patch that perpetuates this confusion, and I am unsure of how to resolve it (which would seem best done in its own patch set). I can see what the 'fake' commands accomplish for the binaries, but can't figure out a scenario where they would be of use for the test framework. These scripts are run in the phase of a test where the framework unpacks the 'local' stuff in preparation for eventually running the binaries. What commands would a test writer invoke during that phase which would need the 'fake' environment set up? I would feel weird submitting a patch that adds even the small complexity of the 'fake' commands without also submitting a positive test case that demonstrates it does what it is supposed to. And I don't know what that test case would be. The downside you mention isn't nothing. Sure, the test filesystem gets built to RAM (typically), so copying all those extra commands for each test is pretty fast. But not copying is still faster. And simpler: there is the ongoing maintenance cost for the list of commands to be copied as new tests are written. Again, these scripts are explicitly written to be invoked during a specific phase, where it is known that a test root is being built, so what advantage is there to having this one little part of the test framework pretend that it doesn't know about the real root? Setting up that pretense just amounts to extra busywork with zero gain. I'm very willing to be convinced I've got it wrong. My familiarity with the code is increasing, but still a long way from where you are at. And if I don't yet know enough to see the use-case justifying the requested feature then I'm probably not the right one to trust to implement it correctly. The conservative approach seems to me to add only what is required by the current tests. It is always easier to add more complexity as more tests are written than it is to ever yank out code that doesn't seem to do anything but maybe one day will be used. Jeremy
On 12/16/13 at 01:38pm, Jeremy Heiner wrote:
On Sat, Dec 14, 2013 at 4:03 PM, Andrew Gregory <andrew.gregory.8@gmail.com> wrote:
One of the main reasons I suggested adding install script support for this was so that they would work the same as scripts run by pacman and therefore be easy to write and understand. At the very least, they should be run in root, not TMPDIR and honor env["scriptlet-shell"]. This should also be run with fakeroot (use -i and -s to save state between invocations) and be run inside a fake chroot. The only downside I see is the need to copy binaries into the test chroot. It looks like your tests would require us to copy at least chown, chmod, rm, touch, and ln into the chroot. Is there any problem with copying the additional binaries?
Hey Andrew, thanks for the code review! I obviously wrote these patches following the "less is more" principle. Every added line of code is a future potential headache. And if I don't have a solid use-case argument backing up my edits I'm not going to put my name up for blame.
I can get my head around the argument that running scripts from the test root is much less surprising. That gives me what I need to put my name on that additional bit of code. But I don't have enough yet for me to feel comfortable taking responsibility for your other suggestions...
The --scriptlet-shell code makes zero sense to me. The help string in configure doesn't match the implementation. There is a hard-coded ref to '/bin/sh' in pmtest.generate, so it is guaranteed that 'bin/sh' is available under the test root. All --scriptlet-shell does is install a 2nd copy of '/bin/sh' under a different path, and maybe I'm missing something because that seems to accomplish nothing. What is the point of telling the test framework to use an alias? Anyway, I would feel uncomfortable submitting a patch that perpetuates this confusion, and I am unsure of how to resolve it (which would seem best done in its own patch set).
The alias isn't for the test runner, it's for pacman. If it didn't exist, pacman wouldn't be able to find the configured SCRIPTLET_SHELL. I was slightly mistaken in my original comment; which path should be used for local scripts actually depends on whether or not they end up being run in a chroot. Running in a chroot, they would use --scriptlet-shell because we know it will be suitable to use for scripts because pacman itself needs it. Outside a chroot, they should continue using '/bin/sh' because that's what the sync scripts in the tests currently get regardless of what scriptlet-shell refers to.
I can see what the 'fake' commands accomplish for the binaries, but can't figure out a scenario where they would be of use for the test framework. These scripts are run in the phase of a test where the framework unpacks the 'local' stuff in preparation for eventually running the binaries. What commands would a test writer invoke during that phase which would need the 'fake' environment set up? I would feel weird submitting a patch that adds even the small complexity of the 'fake' commands without also submitting a positive test case that demonstrates it does what it is supposed to. And I don't know what that test case would be.
fakeroot will let you call chown in querycheck008. Using a chroot allows us to use --scriptlet-shell, which we know will be suitable for running scripts, instead of 'bin/sh' and improves compatibility with the scripts run by pacman, which can contain absolute paths. Though, paths in tests should generally be relative anyway and I doubt we'll be moving away from using 'bin/sh' for sync scripts anytime soon. The gains are fairly minor, so I'm not terribly concerned about using a chroot.
The downside you mention isn't nothing. Sure, the test filesystem gets built to RAM (typically), so copying all those extra commands for each test is pretty fast. But not copying is still faster. And simpler: there is the ongoing maintenance cost for the list of commands to be copied as new tests are written. Again, these scripts are explicitly written to be invoked during a specific phase, where it is known that a test root is being built, so what advantage is there to having this one little part of the test framework pretend that it doesn't know about the real root? Setting up that pretense just amounts to extra busywork with zero gain.
I'm very willing to be convinced I've got it wrong. My familiarity with the code is increasing, but still a long way from where you are at. And if I don't yet know enough to see the use-case justifying the requested feature then I'm probably not the right one to trust to implement it correctly. The conservative approach seems to me to add only what is required by the current tests. It is always easier to add more complexity as more tests are written than it is to ever yank out code that doesn't seem to do anything but maybe one day will be used. Jeremy
On Mon, Dec 16, 2013 at 2:45 PM, Andrew Gregory <andrew.gregory.8@gmail.com> wrote:
fakeroot will let you call chown in querycheck008.
Ah, the lightbulb finally came on for me, at least for fakeroot... now I understand your '-i' / '-s' comment. I'm still in the dark for --scriptlet-shell, but I'll just meditate on it for a while and hopefully it will also click. Or I'll at least figure out what question to ask. Thanks for the help, and I'll get back to you after I can set aside a little time to devote to pondering this. Jeremy
Hey, Andrew, sorry for the delay. I can only scrounge up a certain number of cycles for pacman stuff, and those were consumed by my recent Doxygen experiments. In this brief time, after I popped that off my stack, and before I must respond to the inevitable feedback, I turned back to what I promised to meditate on, and I think I have a good question: Isn't it the job of the test framework to expose the candidate binary to different SCRIPTLET_SHELLs and verify that it behaves correctly under each? If that is the case then the --scriptlet-shell arg is just wrong.
On 22/12/13 15:29, Jeremy Heiner wrote:
Hey, Andrew, sorry for the delay. I can only scrounge up a certain number of cycles for pacman stuff, and those were consumed by my recent Doxygen experiments. In this brief time, after I popped that off my stack, and before I must respond to the inevitable feedback, I turned back to what I promised to meditate on, and I think I have a good question: Isn't it the job of the test framework to expose the candidate binary to different SCRIPTLET_SHELLs and verify that it behaves correctly under each? If that is the case then the --scriptlet-shell arg is just wrong.
No. Pacman can only have one scriptlet shell configured/built into it. The --scriptlet-shell argument is there to pass the correct shell path to the test suite. Given our testsuite is built around shell scripts, we just copy /bin/sh to whatever path the build pacman expects is shell to be at. Allan
Ah! I think I see. But then why is this not handled by configure.ac?
On 22/12/13 15:49, Jeremy Heiner wrote:
Ah! I think I see. But then why is this not handled by configure.ac?
It is when you run make check. Note that you need to specify -p when running ./pactest to use anything other than the system pacman (i.e. to test with the in tree pacman). The handling of --scriptlet-shell is the same.
On Sun, Dec 22, 2013 at 12:56 AM, Allan McRae <allan@archlinux.org> wrote:
Note that you need to specify -p
Sounds to me like another opportunity for autoconf to help simplify. How often does one need to run pactest on the system pacman?
On 22/12/13 16:03, Jeremy Heiner wrote:
On Sun, Dec 22, 2013 at 12:56 AM, Allan McRae <allan@archlinux.org> wrote:
Note that you need to specify -p
Sounds to me like another opportunity for autoconf to help simplify. How often does one need to run pactest on the system pacman?
Anything non-zero is a reason to keep the options. I would not mind if autoconf changed the defaults. Particularly pointing at the built pacman... A
On Dec 22, 2013, Allan McRae <allan@archlinux.org> wrote:
I would not mind if autoconf changed the defaults.
To?
On 22/12/13 16:22, Jeremy Heiner wrote:
On Dec 22, 2013, Allan McRae <allan@archlinux.org> wrote:
I would not mind if autoconf changed the defaults.
To?
-p $(SRCDIR)/src/pacman/pacman --scriptlet-shell=$(SCRIPTLET_SHELL) or whatever autoconf syntax is... Allan
On 11/11/13 21:47, Jeremy Heiner wrote:
pmpkg.install_package (which is called by the framework to populate the filesystem with "local" db packages) now executes any pre_install and/or post_install scripts it finds. Prior to this commit no unit test had a "local" package with such a script. The two new tests in this commit use post_install to remove some installed files.
As with the previous commit, the -Qk test (003) validates the filesystem (warning about the missing files), while the -Qkk test (004) can only check for the warning about the lack of an mtree.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com>
I have just spent time reading through this and the following patches. I am not a fan of overloading the install scriptlet functionality to do this. I agree with Andrew that it would be much preferable to add something like a self.prepacman variable to add a batch of shell scripts to be run inside fakeroot+fakechroot before the pacman call. Also, the querycheck004.py test here is useless, as it does nothing that querycheck002.py does not (just detects missing .MTREE file.
--- test/pacman/pmpkg.py | 13 +++++++++++++ test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck003.py | 19 +++++++++++++++++++ test/pacman/tests/querycheck004.py | 17 +++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 test/pacman/tests/querycheck003.py create mode 100644 test/pacman/tests/querycheck004.py
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 6f7ae6e..fd469b1 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -22,6 +22,7 @@ import stat import shutil from StringIO import StringIO +import subprocess import tarfile
import util @@ -177,13 +178,25 @@ def makepkg(self, path):
tar.close()
+ def runscript(self, root, name): + script = self.install[name] + if script: + util.vprint("running %s script: %s" % (name, script)) + # note: there are differences between the environment the script + # gets from here versus the one the pacman binary would give it. + # in particular there's no chroot, so use relative paths. + subprocess.call(["sh", "-c", script], + cwd=os.path.join(root,util.TMPDIR)) + def install_package(self, root): """Install the package in the given root.""" + self.runscript(root, "pre_install") for f in self.files: util.mkfile(root, f, f) path = os.path.join(root, f) if os.path.isfile(path): os.utime(path, (355, 355)) + self.runscript(root, "post_install")
def filelist(self): """Generate a list of package files.""" diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 0997f58..f4e6caf 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -90,6 +90,8 @@ TESTS += test/pacman/tests/query011.py TESTS += test/pacman/tests/query012.py TESTS += test/pacman/tests/querycheck001.py TESTS += test/pacman/tests/querycheck002.py +TESTS += test/pacman/tests/querycheck003.py +TESTS += test/pacman/tests/querycheck004.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck003.py b/test/pacman/tests/querycheck003.py new file mode 100644 index 0000000..3181188 --- /dev/null +++ b/test/pacman/tests/querycheck003.py @@ -0,0 +1,19 @@ +self.description = "Query--check files, missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/ .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/msgs .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/en .No such file") +self.addrule("PACMAN_OUTPUT=, 3 missing files") diff --git a/test/pacman/tests/querycheck004.py b/test/pacman/tests/querycheck004.py new file mode 100644 index 0000000..0d2015a --- /dev/null +++ b/test/pacman/tests/querycheck004.py @@ -0,0 +1,17 @@ +self.description = "Query--check mtree, no mtree + missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +# the existence of files is not checked when there is no mtree +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=dummy: no mtree file")
Making util.mkfile responsible for calling os.utime avoids duplication of the code in both pmpkg and pmtest, and also the unnecessary repetition of the os.path.join (inside util.mkfile and again in pmpkg/pmtest). This change also eases the way forward for the more complicated utime code which is needed to support mtree testing. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmpkg.py | 5 +---- test/pacman/pmtest.py | 5 +---- test/pacman/util.py | 4 +++- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index fd469b1..db97449 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -192,10 +192,7 @@ def install_package(self, root): """Install the package in the given root.""" self.runscript(root, "pre_install") for f in self.files: - util.mkfile(root, f, f) - path = os.path.join(root, f) - if os.path.isfile(path): - os.utime(path, (355, 355)) + util.mkfile(root, f, f, True) self.runscript(root, "post_install") def filelist(self): diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index a1f3645d..9968dce 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -175,10 +175,7 @@ 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) - if os.path.isfile(path): - os.utime(path, (355, 355)) + util.mkfile(self.root, f, f, True) for pkg in self.db["local"].pkgs: vprint("\tinstalling %s" % pkg.fullname()) pkg.install_package(self.root) diff --git a/test/pacman/util.py b/test/pacman/util.py index ab5a6f4..beb7e3d 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -77,7 +77,7 @@ def getfileinfo(filename): data["filename"] = filename return data -def mkfile(base, name, data=""): +def mkfile(base, name, data="", utime=False): info = getfileinfo(name) filename = info["filename"] @@ -95,6 +95,8 @@ def mkfile(base, name, data=""): os.symlink(info["link"], path) else: writedata(path, data) + if utime: + os.utime(path, (355, 355)) if info["perms"]: os.chmod(path, info["perms"]) -- 1.8.4.2
On 11/11/13 21:47, Jeremy Heiner wrote:
Making util.mkfile responsible for calling os.utime avoids duplication of the code in both pmpkg and pmtest, and also the unnecessary repetition of the os.path.join (inside util.mkfile and again in pmpkg/pmtest).
This change also eases the way forward for the more complicated utime code which is needed to support mtree testing.
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com>
This seems fine to me. It appears dependent on changes Patch 2/5 in this series (or at least it did not apply cleanly here...), so it will need adjusted.
--- test/pacman/pmpkg.py | 5 +---- test/pacman/pmtest.py | 5 +---- test/pacman/util.py | 4 +++- 3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index fd469b1..db97449 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -192,10 +192,7 @@ def install_package(self, root): """Install the package in the given root.""" self.runscript(root, "pre_install") for f in self.files: - util.mkfile(root, f, f) - path = os.path.join(root, f) - if os.path.isfile(path): - os.utime(path, (355, 355)) + util.mkfile(root, f, f, True) self.runscript(root, "post_install")
def filelist(self): diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index a1f3645d..9968dce 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -175,10 +175,7 @@ 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) - if os.path.isfile(path): - os.utime(path, (355, 355)) + util.mkfile(self.root, f, f, True) for pkg in self.db["local"].pkgs: vprint("\tinstalling %s" % pkg.fullname()) pkg.install_package(self.root) diff --git a/test/pacman/util.py b/test/pacman/util.py index ab5a6f4..beb7e3d 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -77,7 +77,7 @@ def getfileinfo(filename): data["filename"] = filename return data
-def mkfile(base, name, data=""): +def mkfile(base, name, data="", utime=False): info = getfileinfo(name) filename = info["filename"]
@@ -95,6 +95,8 @@ def mkfile(base, name, data=""): os.symlink(info["link"], path) else: writedata(path, data) + if utime: + os.utime(path, (355, 355))
if info["perms"]: os.chmod(path, info["perms"])
The framework will now generate mtree data when the pmpkg.makemtree is set to True, but only for "local" packages. Since the mtree data is only used by -Qkk, and this change is sufficient to test that function, there seems to be no reason to generate mtree data for other packages. If that becomes a requirement in the future there are comments in the code that sketch out an implemention for that. The two new unit tests are similar to the previous -Qkk tests (005 is like 002, and 006 is like 004), but with mtree data. So the filesystem is actually validated against the mtree (instead of merely checking for the warning about a lacking mtree). Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmdb.py | 3 ++ test/pacman/pmpkg.py | 18 ++++++++++++ test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck005.py | 16 +++++++++++ test/pacman/tests/querycheck006.py | 20 +++++++++++++ test/pacman/util.py | 57 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+) create mode 100644 test/pacman/tests/querycheck005.py create mode 100644 test/pacman/tests/querycheck006.py diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index 53de91e..088e284 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -224,6 +224,9 @@ def db_write(self, pkg): make_section(data, "BACKUP", pkg.local_backup_entries()) entry["files"] = "\n".join(data) + if pkg.mtreedata: + entry["mtree"] = pkg.mtreedata + if any(pkg.install.values()): entry["install"] = pkg.installfile() diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index db97449..25e841a 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -60,6 +60,8 @@ def __init__(self, name, version = "1.0-1"): # files self.files = [] self.backup = [] + self.makemtree = False # True => finalize generates mtreedata + self.mtreedata = "" # you can set this, but makemtree is easier # install self.install = { "pre_install": "", @@ -145,6 +147,19 @@ def makepkg(self, path): if any(self.install.values()): archive_files.append((".INSTALL", self.installfile())) + # .MTREE + if self.mtreedata: + raise NotImplementedError("attempt to put mtree into tar") + # TODO: they work great in local dbs, but not in tar'd pkgs. + # it seems unlikely that any test will ever need to, but: + # easiest solution seems to be to hack the 'Generate package + # file system' loop just below to copy the attributes from + # the mtreedata over to the TarInfo fields. pulling the data + # out with re.search shouldn't be too hard. put that code + # over in util so it gets updated with the other mtree code. + # then remove the raise and uncomment the following line: + # archive_files.append((".MTREE", self.mtreedata)) + self.path = os.path.join(path, self.filename()) util.mkdir(os.path.dirname(self.path)) @@ -226,6 +241,9 @@ def finalize(self): name = os.path.dirname(name) self.files.sort() + if self.makemtree: + self.mtreedata = util.mtree(self.files) + self.finalized = True def local_backup_entries(self): diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index f4e6caf..c460fe0 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -92,6 +92,8 @@ TESTS += test/pacman/tests/querycheck001.py TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/querycheck003.py TESTS += test/pacman/tests/querycheck004.py +TESTS += test/pacman/tests/querycheck005.py +TESTS += test/pacman/tests/querycheck006.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck005.py b/test/pacman/tests/querycheck005.py new file mode 100644 index 0000000..95c776d --- /dev/null +++ b/test/pacman/tests/querycheck005.py @@ -0,0 +1,16 @@ +self.description = "Query--check mtree, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.makemtree = True +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=dummy: no mtree file") diff --git a/test/pacman/tests/querycheck006.py b/test/pacman/tests/querycheck006.py new file mode 100644 index 0000000..879fd00 --- /dev/null +++ b/test/pacman/tests/querycheck006.py @@ -0,0 +1,20 @@ +self.description = "Query--check mtree, missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.makemtree = True +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/msgs .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/en .No such file") +self.addrule("PACMAN_OUTPUT=, 3 altered files") diff --git a/test/pacman/util.py b/test/pacman/util.py index beb7e3d..2158b80 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -20,6 +20,8 @@ import os import re import hashlib +import subprocess +import sys import tap @@ -93,6 +95,11 @@ def mkfile(base, name, data="", utime=False): if info["islink"]: os.symlink(info["link"], path) + if utime: + if sys.hexversion < 0x03030000: + subprocess.call(["touch","-ht","196912311905.55",path]) + else: + os.utime(path, (355,355), follow_symlinks=False) else: writedata(path, data) if utime: @@ -158,6 +165,56 @@ def mkmd5sum(data): # +# mtree +# obviously not a full implementation of the spec (e.g. no +# quoting of paths), but just enough for a few tests. +# freebsd mtree(5) has a bit more detail than arch or openbsd. +# scripts/makepkg calls bsdtar to build mtrees at line 1890. +# + +def mtree(files): + + lines = ["#mtree"] # no version (ala bsdtar) + for name in files: + info = getfileinfo(name) + filename = info["filename"] + + if info["isdir"]: + filename = filename.rstrip("/") + mode = info["perms"] if info["hasperms"] else 0o755 + attrs = ("type=dir mode=%o" + % (mode)) + + elif info["islink"]: + mode = info["perms"] if info["hasperms"] else 0o777 + attrs = ("type=link time=355.000000000" + " mode=%o link=%s" + % (mode, info["link"])) + + else: + mode = info["perms"] if info["hasperms"] else 0o644 + contents = name + "\n" + md5 = hashlib.md5(contents).hexdigest() + sha256 = hashlib.sha256(contents).hexdigest() + attrs = ("type=file time=355.000000000" + " mode=%o size=%d md5digest=%s sha256digest=%s" + % (mode, len(contents), md5, sha256)) + + lines.append("./%s uid=0 gid=0 %s" % (filename, attrs)) + + mtree = "\n".join(lines)+"\n" + + # scripts/makepkg compresses the mtree (via bsdtar using gzip). + # python's gzip always opens a file (can't make it use StringIO). + # gzip uses zlib, but if we zlib.compress(mtree) it causes libalpm + # to reject the mtree with "Unrecognized archive format". + # fortunately libalpm reads the uncompressed mtree files just fine, + # so we don't have to jump through any hoops to get it compressed. + + return mtree + + +# # Miscellaneous # -- 1.8.4.2
On 11/11/13 21:47, Jeremy Heiner wrote:
The framework will now generate mtree data when the pmpkg.makemtree is set to True, but only for "local" packages. Since the mtree data is only used by -Qkk, and this change is sufficient to test that function, there seems to be no reason to generate mtree data for other packages. If that becomes a requirement in the future there are comments in the code that sketch out an implemention for that.
The two new unit tests are similar to the previous -Qkk tests (005 is like 002, and 006 is like 004), but with mtree data. So the filesystem is actually validated against the mtree (instead of merely checking for the warning about a lacking mtree).
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/pmdb.py | 3 ++ test/pacman/pmpkg.py | 18 ++++++++++++ test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck005.py | 16 +++++++++++ test/pacman/tests/querycheck006.py | 20 +++++++++++++ test/pacman/util.py | 57 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+) create mode 100644 test/pacman/tests/querycheck005.py create mode 100644 test/pacman/tests/querycheck006.py
diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index 53de91e..088e284 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -224,6 +224,9 @@ def db_write(self, pkg): make_section(data, "BACKUP", pkg.local_backup_entries()) entry["files"] = "\n".join(data)
+ if pkg.mtreedata: + entry["mtree"] = pkg.mtreedata + if any(pkg.install.values()): entry["install"] = pkg.installfile()
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index db97449..25e841a 100644 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -60,6 +60,8 @@ def __init__(self, name, version = "1.0-1"): # files self.files = [] self.backup = [] + self.makemtree = False # True => finalize generates mtreedata + self.mtreedata = "" # you can set this, but makemtree is easier # install self.install = { "pre_install": "", @@ -145,6 +147,19 @@ def makepkg(self, path): if any(self.install.values()): archive_files.append((".INSTALL", self.installfile()))
+ # .MTREE + if self.mtreedata: + raise NotImplementedError("attempt to put mtree into tar") + # TODO: they work great in local dbs, but not in tar'd pkgs. + # it seems unlikely that any test will ever need to, but: + # easiest solution seems to be to hack the 'Generate package + # file system' loop just below to copy the attributes from + # the mtreedata over to the TarInfo fields. pulling the data + # out with re.search shouldn't be too hard. put that code + # over in util so it gets updated with the other mtree code. + # then remove the raise and uncomment the following line: + # archive_files.append((".MTREE", self.mtreedata)) + self.path = os.path.join(path, self.filename()) util.mkdir(os.path.dirname(self.path))
@@ -226,6 +241,9 @@ def finalize(self): name = os.path.dirname(name) self.files.sort()
+ if self.makemtree: + self.mtreedata = util.mtree(self.files) + self.finalized = True
def local_backup_entries(self): diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index f4e6caf..c460fe0 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -92,6 +92,8 @@ TESTS += test/pacman/tests/querycheck001.py TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/querycheck003.py TESTS += test/pacman/tests/querycheck004.py +TESTS += test/pacman/tests/querycheck005.py +TESTS += test/pacman/tests/querycheck006.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck005.py b/test/pacman/tests/querycheck005.py new file mode 100644 index 0000000..95c776d --- /dev/null +++ b/test/pacman/tests/querycheck005.py @@ -0,0 +1,16 @@ +self.description = "Query--check mtree, all there" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.makemtree = True +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PACMAN_OUTPUT=dummy: no mtree file") diff --git a/test/pacman/tests/querycheck006.py b/test/pacman/tests/querycheck006.py new file mode 100644 index 0000000..879fd00 --- /dev/null +++ b/test/pacman/tests/querycheck006.py @@ -0,0 +1,20 @@ +self.description = "Query--check mtree, missing" + +pkg = pmpkg("dummy") +pkg.files = [ + "usr/lib/libdummy.so.0", + "usr/lib/libdummy.so -> libdummy.so.0", + "usr/share/dummy/C/", + "usr/share/dummy/C/msgs", + "usr/share/dummy/en -> C"] +pkg.makemtree = True +pkg.install['post_install'] = "rm -r ../usr/share/dummy/*" +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/C/msgs .No such file") +self.addrule("PACMAN_OUTPUT=/usr/share/dummy/en .No such file") +self.addrule("PACMAN_OUTPUT=, 3 altered files") diff --git a/test/pacman/util.py b/test/pacman/util.py index beb7e3d..2158b80 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -20,6 +20,8 @@ import os import re import hashlib +import subprocess +import sys
import tap
@@ -93,6 +95,11 @@ def mkfile(base, name, data="", utime=False):
if info["islink"]: os.symlink(info["link"], path) + if utime: + if sys.hexversion < 0x03030000: + subprocess.call(["touch","-ht","196912311905.55",path]) + else: + os.utime(path, (355,355), follow_symlinks=False) else: writedata(path, data) if utime: @@ -158,6 +165,56 @@ def mkmd5sum(data):
# +# mtree +# obviously not a full implementation of the spec (e.g. no +# quoting of paths), but just enough for a few tests. +# freebsd mtree(5) has a bit more detail than arch or openbsd. +# scripts/makepkg calls bsdtar to build mtrees at line 1890.
Comments referencing lines are awful because the line number will change. Can we not just call bsdtar to generate these using the exact command makepkg does?
+# + +def mtree(files): + + lines = ["#mtree"] # no version (ala bsdtar) + for name in files: + info = getfileinfo(name) + filename = info["filename"] + + if info["isdir"]: + filename = filename.rstrip("/") + mode = info["perms"] if info["hasperms"] else 0o755 + attrs = ("type=dir mode=%o" + % (mode)) + + elif info["islink"]: + mode = info["perms"] if info["hasperms"] else 0o777 + attrs = ("type=link time=355.000000000" + " mode=%o link=%s" + % (mode, info["link"])) + + else: + mode = info["perms"] if info["hasperms"] else 0o644 + contents = name + "\n" + md5 = hashlib.md5(contents).hexdigest() + sha256 = hashlib.sha256(contents).hexdigest() + attrs = ("type=file time=355.000000000" + " mode=%o size=%d md5digest=%s sha256digest=%s" + % (mode, len(contents), md5, sha256)) + + lines.append("./%s uid=0 gid=0 %s" % (filename, attrs)) + + mtree = "\n".join(lines)+"\n" + + # scripts/makepkg compresses the mtree (via bsdtar using gzip). + # python's gzip always opens a file (can't make it use StringIO). + # gzip uses zlib, but if we zlib.compress(mtree) it causes libalpm + # to reject the mtree with "Unrecognized archive format". + # fortunately libalpm reads the uncompressed mtree files just fine, + # so we don't have to jump through any hoops to get it compressed. + + return mtree + + +# # Miscellaneous #
The first test (007) checks every permutation of file type mismatch that -Qkk warns about. The second (008) checks for the other types of mismatch (permissions, modification time, size, and symlink path) that the test framework can set up without root permission. Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck007.py | 25 +++++++++++++++++++++++++ test/pacman/tests/querycheck008.py | 25 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 test/pacman/tests/querycheck007.py create mode 100644 test/pacman/tests/querycheck008.py diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index c460fe0..abe0558 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -94,6 +94,8 @@ TESTS += test/pacman/tests/querycheck003.py TESTS += test/pacman/tests/querycheck004.py TESTS += test/pacman/tests/querycheck005.py TESTS += test/pacman/tests/querycheck006.py +TESTS += test/pacman/tests/querycheck007.py +TESTS += test/pacman/tests/querycheck008.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck007.py b/test/pacman/tests/querycheck007.py new file mode 100644 index 0000000..9a2c765 --- /dev/null +++ b/test/pacman/tests/querycheck007.py @@ -0,0 +1,25 @@ +self.description = "Query--check mtree, bad types" + +pkg = pmpkg("dummy") +pkg.files = ["dummy/z"] +pkg.makemtree = True +script = [] +for file, mangle in [ + ("dummy/d1/", ["rmdir", "touch"]), + ("dummy/d2/", ["rmdir", "ln -s z"]), + ("dummy/f1", ["rm", "mkdir"]), + ("dummy/f2", ["rm", "ln -s z"]), + ("dummy/l1 -> z", ["rm", "mkdir"]), + ("dummy/l2 -> z", ["rm", "touch"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file).rstrip('/') + for cmd in mangle: + script.append(cmd +" ../"+parsed) + self.addrule("PACMAN_OUTPUT=%s .File type mismatch" % parsed) +pkg.install['post_install'] = ";".join(script) +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=, 6 altered files") diff --git a/test/pacman/tests/querycheck008.py b/test/pacman/tests/querycheck008.py new file mode 100644 index 0000000..538eb75 --- /dev/null +++ b/test/pacman/tests/querycheck008.py @@ -0,0 +1,25 @@ +self.description = "Query--check mtree, bad perms+time+size+link" + +pkg = pmpkg("dummy") +pkg.files = ["dummy/z"] +pkg.makemtree = True +script = [] +for file, err, mangle in [ + #("dummy/uid", "UID", ["chown ?"]),#needs root + #("dummy/gid", "GID", ["chgrp ?"]),#needs root + ("dummy/mode", "Permissions", ["chmod 600"]), + ("dummy/mtime", "Modification time", ["touch"]), + ("dummy/size", "Size", ["echo x >"]), + ("dummy/link -> size", "Symlink path", ["rm", "ln -s z"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file) + for cmd in mangle: + script.append(cmd +" ../"+parsed) + self.addrule("PACMAN_OUTPUT=%s .%s mismatch" % (parsed,err)) +pkg.install['post_install'] = ";".join(script) +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=, 4 altered files") -- 1.8.4.2
On 11/11/13 21:47, Jeremy Heiner wrote:
The first test (007) checks every permutation of file type mismatch that -Qkk warns about. The second (008) checks for the other types of mismatch (permissions, modification time, size, and symlink path) that the test framework can set up without root permission.
These look fine in principle. I'd like a pactest that tests each difference individually, as that will help us limit down any failures. Allan
Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com> --- test/pacman/tests/TESTS | 2 ++ test/pacman/tests/querycheck007.py | 25 +++++++++++++++++++++++++ test/pacman/tests/querycheck008.py | 25 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 test/pacman/tests/querycheck007.py create mode 100644 test/pacman/tests/querycheck008.py
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index c460fe0..abe0558 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -94,6 +94,8 @@ TESTS += test/pacman/tests/querycheck003.py TESTS += test/pacman/tests/querycheck004.py TESTS += test/pacman/tests/querycheck005.py TESTS += test/pacman/tests/querycheck006.py +TESTS += test/pacman/tests/querycheck007.py +TESTS += test/pacman/tests/querycheck008.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove001.py TESTS += test/pacman/tests/remove002.py diff --git a/test/pacman/tests/querycheck007.py b/test/pacman/tests/querycheck007.py new file mode 100644 index 0000000..9a2c765 --- /dev/null +++ b/test/pacman/tests/querycheck007.py @@ -0,0 +1,25 @@ +self.description = "Query--check mtree, bad types" + +pkg = pmpkg("dummy") +pkg.files = ["dummy/z"] +pkg.makemtree = True +script = [] +for file, mangle in [ + ("dummy/d1/", ["rmdir", "touch"]), + ("dummy/d2/", ["rmdir", "ln -s z"]), + ("dummy/f1", ["rm", "mkdir"]), + ("dummy/f2", ["rm", "ln -s z"]), + ("dummy/l1 -> z", ["rm", "mkdir"]), + ("dummy/l2 -> z", ["rm", "touch"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file).rstrip('/') + for cmd in mangle: + script.append(cmd +" ../"+parsed) + self.addrule("PACMAN_OUTPUT=%s .File type mismatch" % parsed) +pkg.install['post_install'] = ";".join(script) +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=, 6 altered files") diff --git a/test/pacman/tests/querycheck008.py b/test/pacman/tests/querycheck008.py new file mode 100644 index 0000000..538eb75 --- /dev/null +++ b/test/pacman/tests/querycheck008.py @@ -0,0 +1,25 @@ +self.description = "Query--check mtree, bad perms+time+size+link" + +pkg = pmpkg("dummy") +pkg.files = ["dummy/z"] +pkg.makemtree = True +script = [] +for file, err, mangle in [ + #("dummy/uid", "UID", ["chown ?"]),#needs root + #("dummy/gid", "GID", ["chgrp ?"]),#needs root + ("dummy/mode", "Permissions", ["chmod 600"]), + ("dummy/mtime", "Modification time", ["touch"]), + ("dummy/size", "Size", ["echo x >"]), + ("dummy/link -> size", "Symlink path", ["rm", "ln -s z"])]: + pkg.files.append(file) + parsed = pkg.parse_filename(file) + for cmd in mangle: + script.append(cmd +" ../"+parsed) + self.addrule("PACMAN_OUTPUT=%s .%s mismatch" % (parsed,err)) +pkg.install['post_install'] = ";".join(script) +self.addpkg2db("local",pkg) + +self.args = "-Qkk" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("PACMAN_OUTPUT=, 4 altered files")
On 11/11/13 21:47, Jeremy Heiner wrote:
There are two changes to the test framework to allow these new unit tests: PATCH 2 adds code to run pre and post install scripts for "local" packages, and PATCH 4 adds code to generate mtree data (if the unit test requests it).
Thanks again to everyone who provided feedback on my previous attempt at this... hopefully the improvement your suggestions inspired will be obvious.
Jeremy Heiner (5): Add the unit tests for -Qk and -Qkk that are possible now. Add unit tests for -Qk and -Qkk with missing files. Simplify the Python code that sets utime of test package files. Add mtree code to test framework and basic unit tests for -Qkk. Add unit tests for -Qkk with altered files.
Thanks, and sorry these have not been reviewed yet. They are not forgotten! Speaking of review - is anyone able to do one for these patches? My time is limited lately and reviewing python code would take me too long. Allan
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Jeremy Heiner