Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Hey Simon, Simon McVittie [2015-01-21 14:31 +]: systemd currently has AM_INIT_AUTOMAKE([foreign 1.11 -Wall -Wno-portability silent-rules tar-pax no-dist-gzip dist-xz subdir-objects]) but Automake 1.11 and 1.12 use the old serial test harness by default. That doesn't understand the log compiler concept. You'll need to either depend on 1.13 (change the 1.11 to 1.13), or add the parallel-tests option. Thanks for pointing out! Fixed in http://cgit.freedesktop.org/systemd/systemd/commit/?id=91ca5bf0 Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On 21 January 2015 at 14:31, Simon McVittie simon.mcvit...@collabora.co.uk wrote: On 20/01/15 20:33, Martin Pitt wrote: Dimitri John Ledkov [2015-01-20 18:23 +]: With parallel test harness in automake (everyone should have it by now) Yay, thanks for pointing this out! That makes the whole thing indeed much friendlier. systemd currently has AM_INIT_AUTOMAKE([foreign 1.11 -Wall -Wno-portability silent-rules tar-pax no-dist-gzip dist-xz subdir-objects]) but Automake 1.11 and 1.12 use the old serial test harness by default. That doesn't understand the log compiler concept. You'll need to either depend on 1.13 (change the 1.11 to 1.13), or add the parallel-tests option. Although automake 1.13 is two years old, imho it's best to specify parallel-tests for now. No need to jump higher. -- Regards, Dimitri. Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On 20/01/15 20:33, Martin Pitt wrote: Dimitri John Ledkov [2015-01-20 18:23 +]: With parallel test harness in automake (everyone should have it by now) Yay, thanks for pointing this out! That makes the whole thing indeed much friendlier. systemd currently has AM_INIT_AUTOMAKE([foreign 1.11 -Wall -Wno-portability silent-rules tar-pax no-dist-gzip dist-xz subdir-objects]) but Automake 1.11 and 1.12 use the old serial test harness by default. That doesn't understand the log compiler concept. You'll need to either depend on 1.13 (change the 1.11 to 1.13), or add the parallel-tests option. S ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Zbigniew Jędrzejewski-Szmek [2015-01-20 16:48 +0100]: Maybe we could do this check in configure.ac/Makefile.am (add the test to the list conditinally)? Yes, that's a good idea. We already test for python presence and extract the version, so we shouldn't duplicate the tests here and have an extra wrapper. Unfortunately automake's TESTS only accepts single arguments, i. e. scripts without any arguments. But we need to call $(PYTHON) test/x.py thus we need an argument. An alternative would be to add a new TESTS_PYTHON = test/sysv-generator-test.py test/rule-syntax-check.py (and perhaps more in the future) and integrate that into Makefile.am. That would make Makefile.am more complex, but avoid the extra wrappers. WDYT? Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On Tue, Jan 20, 2015 at 04:19:24PM +0100, Martin Pitt wrote: Hello all, We've had numerous problems with the SysV generator in the past, and we just recently introduced another regression: init.d scripts which end in .sh are now totally broken. Thus I think it's high time to write some integration tests for that. The attached patch provides the necessary framework and an initial set of tests; e. g. test_multiple_provides() covers Michael's recent commit b7e71846. I can reproduce the .sh bug from above with a simple |def test_sh_suffix(self): |'''init.d script with .sh suffix''' | |self.add_sysv('foo.sh', {}, enable=True) |err, results = self.run_generator() |[... actual checks here, not written yet ...] which currently fails with | == | FAIL: test_sh_suffix (__main__.SysvGeneratorTest) | init.d script with .sh suffix | -- | Traceback (most recent call last): | File test/../test/sysv-generator-test.py, line 179, in test_sh_suffix | err, results = self.run_generator() | File test/../test/sysv-generator-test.py, line 58, in run_generator | self.assertFalse('Fail' in err, err) | AssertionError: True is not false : Looking for unit files in (higher priority first): | /etc/systemd/system | /run/systemd/system | /usr/local/lib/systemd/system | /lib/systemd/system | /usr/lib/systemd/system | Looking for SysV init scripts in: | /tmp/sysv-gen-test.7qlq6kg2/init.d | Looking for SysV rcN.d links in: | /tmp/sysv-gen-test.7qlq6kg2 | Failed to create unit file /tmp/sysv-gen-test.7qlq6kg2/output/foo.service: File exists Indeed it just creates a symlink pointing to itself and nothing else. I will look into that actual bug in a bit, and write a complete test along with it. But before I spend more work on the tests, I'd appreciate a quick review of it whether the general structure is ok for you. As this deals with temp dirs, cleaning them up, running external programs, parsing their output etc., I chose Python for this, as this stuff is just s much faster and convenient to write. We already have test/rule-syntax-check.py, so there's precedent :-) As automake's tests are rather limited and require a single command without arguments, but I want to make this obey configure's $(PYTHON) and skip the test properly if python 3 is not available, I created a simple shell wrapper around it. Obviously this is still lacking a lot of important cases; I'm happy to add them later on, I just wanted to get some initial generic feedback. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From 7d4f85e42ff5a7a05477e712dcb58ab99d02a87a Mon Sep 17 00:00:00 2001 From: Martin Pitt martin.p...@ubuntu.com Date: Tue, 20 Jan 2015 16:08:05 +0100 Subject: [PATCH] test: add initial integration test for systemd-sysv-generator This is still missing a lot of important scenarios and corner cases, but provides the groundwork and covers a recent bug (commit b7e718) --- Makefile.am | 9 ++- test/sysv-generator-test.py | 177 test/sysv-generator-test.sh | 33 + 3 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 test/sysv-generator-test.py create mode 100755 test/sysv-generator-test.sh diff --git a/Makefile.am b/Makefile.am index 788e634..f7ae578 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3767,7 +3767,9 @@ endif # -- TESTS += \ test/udev-test.pl \ - test/rules-test.sh + test/rules-test.sh \ + test/sysv-generator-test.sh \ + $(NULL) manual_tests += \ test-libudev \ @@ -3812,7 +3814,10 @@ EXTRA_DIST += \ test/sys.tar.xz \ test/udev-test.pl \ test/rules-test.sh \ - test/rule-syntax-check.py + test/rule-syntax-check.py \ + test/sysv-generator-test.sh \ + test/sysv-generator-test.py \ + $(NULL) # -- ata_id_SOURCES = \ diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py new file mode 100644 index 000..a3f80ca --- /dev/null +++ b/test/sysv-generator-test.py @@ -0,0 +1,177 @@ +# systemd-sysv-generator integration test +# +# (C) 2015 Canonical Ltd. +# Author: Martin Pitt martin.p...@ubuntu.com +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +# systemd is
[systemd-devel] [PATCH] initial sysv-generator test suite
Hello all, We've had numerous problems with the SysV generator in the past, and we just recently introduced another regression: init.d scripts which end in .sh are now totally broken. Thus I think it's high time to write some integration tests for that. The attached patch provides the necessary framework and an initial set of tests; e. g. test_multiple_provides() covers Michael's recent commit b7e71846. I can reproduce the .sh bug from above with a simple |def test_sh_suffix(self): |'''init.d script with .sh suffix''' | |self.add_sysv('foo.sh', {}, enable=True) |err, results = self.run_generator() |[... actual checks here, not written yet ...] which currently fails with | == | FAIL: test_sh_suffix (__main__.SysvGeneratorTest) | init.d script with .sh suffix | -- | Traceback (most recent call last): | File test/../test/sysv-generator-test.py, line 179, in test_sh_suffix | err, results = self.run_generator() | File test/../test/sysv-generator-test.py, line 58, in run_generator | self.assertFalse('Fail' in err, err) | AssertionError: True is not false : Looking for unit files in (higher priority first): | /etc/systemd/system | /run/systemd/system | /usr/local/lib/systemd/system | /lib/systemd/system | /usr/lib/systemd/system | Looking for SysV init scripts in: | /tmp/sysv-gen-test.7qlq6kg2/init.d | Looking for SysV rcN.d links in: | /tmp/sysv-gen-test.7qlq6kg2 | Failed to create unit file /tmp/sysv-gen-test.7qlq6kg2/output/foo.service: File exists Indeed it just creates a symlink pointing to itself and nothing else. I will look into that actual bug in a bit, and write a complete test along with it. But before I spend more work on the tests, I'd appreciate a quick review of it whether the general structure is ok for you. As this deals with temp dirs, cleaning them up, running external programs, parsing their output etc., I chose Python for this, as this stuff is just s much faster and convenient to write. We already have test/rule-syntax-check.py, so there's precedent :-) As automake's tests are rather limited and require a single command without arguments, but I want to make this obey configure's $(PYTHON) and skip the test properly if python 3 is not available, I created a simple shell wrapper around it. Obviously this is still lacking a lot of important cases; I'm happy to add them later on, I just wanted to get some initial generic feedback. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From 7d4f85e42ff5a7a05477e712dcb58ab99d02a87a Mon Sep 17 00:00:00 2001 From: Martin Pitt martin.p...@ubuntu.com Date: Tue, 20 Jan 2015 16:08:05 +0100 Subject: [PATCH] test: add initial integration test for systemd-sysv-generator This is still missing a lot of important scenarios and corner cases, but provides the groundwork and covers a recent bug (commit b7e718) --- Makefile.am | 9 ++- test/sysv-generator-test.py | 177 test/sysv-generator-test.sh | 33 + 3 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 test/sysv-generator-test.py create mode 100755 test/sysv-generator-test.sh diff --git a/Makefile.am b/Makefile.am index 788e634..f7ae578 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3767,7 +3767,9 @@ endif # -- TESTS += \ test/udev-test.pl \ - test/rules-test.sh + test/rules-test.sh \ + test/sysv-generator-test.sh \ + $(NULL) manual_tests += \ test-libudev \ @@ -3812,7 +3814,10 @@ EXTRA_DIST += \ test/sys.tar.xz \ test/udev-test.pl \ test/rules-test.sh \ - test/rule-syntax-check.py + test/rule-syntax-check.py \ + test/sysv-generator-test.sh \ + test/sysv-generator-test.py \ + $(NULL) # -- ata_id_SOURCES = \ diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py new file mode 100644 index 000..a3f80ca --- /dev/null +++ b/test/sysv-generator-test.py @@ -0,0 +1,177 @@ +# systemd-sysv-generator integration test +# +# (C) 2015 Canonical Ltd. +# Author: Martin Pitt martin.p...@ubuntu.com +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +# systemd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On 20 January 2015 at 16:49, Martin Pitt martin.p...@ubuntu.com wrote: Zbigniew Jędrzejewski-Szmek [2015-01-20 16:48 +0100]: Maybe we could do this check in configure.ac/Makefile.am (add the test to the list conditinally)? Yes, that's a good idea. We already test for python presence and extract the version, so we shouldn't duplicate the tests here and have an extra wrapper. Unfortunately automake's TESTS only accepts single arguments, i. e. scripts without any arguments. But we need to call $(PYTHON) test/x.py thus we need an argument. With parallel test harness in automake (everyone should have it by now) you can set custom runner of your test, based on extensions, e.g. from automake manual: TESTS = foo.pl bar.py baz TEST_EXTENSIONS = .pl .py PL_LOG_COMPILER = $(PERL) AM_PL_LOG_FLAGS = -w PY_LOG_COMPILER = $(PYTHON) AM_PY_LOG_FLAGS = -v LOG_COMPILER = ./wrapper-script AM_LOG_FLAGS = -d If above is not enough hint, i can poke automake-foo to support $(PYTHON) tests when $(PYTHON) is available. An alternative would be to add a new TESTS_PYTHON = test/sysv-generator-test.py test/rule-syntax-check.py (and perhaps more in the future) and integrate that into Makefile.am. That would make Makefile.am more complex, but avoid the extra wrappers. WDYT? Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Regards, Dimitri. Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On 01/20/2015 03:19 PM, Martin Pitt wrote: initial generic feedback We only provide backwards compatibility with initscript which are lsb compliance and I dont think .something ending on a script confirms to that standard hence that test should be unnecessary and that initscript be fixed upstream as in that .something ending removed ( or better yet that initscript be migrated ) JBG ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
On Tue, Jan 20, 2015 at 8:08 PM, Martin Pitt martin.p...@ubuntu.com wrote: Hey Jóhann, Jóhann B. Guðmundsson [2015-01-20 17:55 +]: We only provide backwards compatibility with initscript which are lsb compliance and I dont think .something ending on a script confirms to that standard hence that test should be unnecessary and that initscript be fixed upstream as in that .something ending removed ( or better yet that initscript be migrated ) But the generator already handles .sh extensions, as they do exist in the wild (and being compatible to them is the whole point of the generator -- for new things you'd write a proper unit right away). It's just missing that .sh extension handling in that particular place, and as it's (correctly) doing it in another you get that bug. Initscripts ending with .sh was indeed a case handled by the sysv-generator from the beginning. I think that fixing this specific issue makes sense and the patch (in the other email) looks pretty simple to me. Thanks for writing all these tests Martin. The bugs found in the generator are embarrassing and in hindsight I should have tested this more thoroughly on a debian system instead of just fedora. I look forward to all your tests of this. Let me know if I can help out with extending the test cases or anything. - Thomas ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Martin Pitt [2015-01-20 16:19 +0100]: Thus I think it's high time to write some integration tests for that. The attached patch provides the necessary framework and an initial set of tests; e. g. test_multiple_provides() covers Michael's recent commit b7e71846. Zbigniew and Thomas generally ack'ed this, so I pushed this as http://cgit.freedesktop.org/systemd/systemd/commit/?id=e28aa58 Improvements since the first version: - no shell wrapper any more - several new tests, including a reproducer for the bogus orderings bug in commit 1ed0c19 - assert_enabled() helper for making test cases smaller - now works with both python 2 and 3, so the $(PYTHON) is python3 condition got dropped Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Hey Dimitri, Dimitri John Ledkov [2015-01-20 18:23 +]: With parallel test harness in automake (everyone should have it by now) you can set custom runner of your test, based on extensions, e.g. from automake manual: TESTS = foo.pl bar.py baz TEST_EXTENSIONS = .pl .py PL_LOG_COMPILER = $(PERL) AM_PL_LOG_FLAGS = -w PY_LOG_COMPILER = $(PYTHON) Yay, thanks for pointing this out! That makes the whole thing indeed much friendlier. I pushed http://cgit.freedesktop.org/systemd/systemd/commit/?id=e8015e6e2 as preparatory work to move the remaining logic out of the rules-test.sh wrapper for rule-syntax-check.py, and then http://cgit.freedesktop.org/systemd/systemd/commit/?id=72521ab9fd to eliminiate it completely, and only run the *.py tests with HAVE_PYTHON. I'll rework the sysv-generator test suite patch tomorrow and add some more tests. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] initial sysv-generator test suite
Hey Jóhann, Jóhann B. Guðmundsson [2015-01-20 17:55 +]: We only provide backwards compatibility with initscript which are lsb compliance and I dont think .something ending on a script confirms to that standard hence that test should be unnecessary and that initscript be fixed upstream as in that .something ending removed ( or better yet that initscript be migrated ) But the generator already handles .sh extensions, as they do exist in the wild (and being compatible to them is the whole point of the generator -- for new things you'd write a proper unit right away). It's just missing that .sh extension handling in that particular place, and as it's (correctly) doing it in another you get that bug. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel