From 7c18b187deafded4d3c6919fdb8640261d33e631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 30 Nov 2019 04:35:18 +0100 Subject: [PATCH 1/6] tests: add include and exclude lists for extra tests loader 'extra' tests run is getting ridiculously long. Allow splitting it into several jobs. Since this appears as just one class from the test loader perspective, implement it as environment variables: - QUBES_TEST_EXTRA_INCLUDE - load just selected tests - QUBES_TEST_EXTRA_EXCLUDE - skip selected tests (to select "the rest" tests) --- qubes/tests/extra.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/qubes/tests/extra.py b/qubes/tests/extra.py index fe61e0bd..a1e0f03e 100644 --- a/qubes/tests/extra.py +++ b/qubes/tests/extra.py @@ -19,6 +19,7 @@ # import asyncio +import os import subprocess import sys @@ -195,7 +196,18 @@ class ExtraTestCase(qubes.tests.SystemTestCase): def load_tests(loader, tests, pattern): + include_list = None + if 'QUBES_TEST_EXTRA_INCLUDE' in os.environ: + include_list = os.environ['QUBES_TEST_EXTRA_INCLUDE'].split() + exclude_list = [] + if 'QUBES_TEST_EXTRA_EXCLUDE' in os.environ: + exclude_list = os.environ['QUBES_TEST_EXTRA_EXCLUDE'].split() + for entry in pkg_resources.iter_entry_points('qubes.tests.extra'): + if include_list is not None and entry.name not in include_list: + continue + if entry.name in exclude_list: + continue try: for test_case in entry.load()(): tests.addTests(loader.loadTestsFromNames([ @@ -210,6 +222,10 @@ def load_tests(loader, tests, pattern): for entry in pkg_resources.iter_entry_points( 'qubes.tests.extra.for_template'): + if include_list is not None and entry.name not in include_list: + continue + if entry.name in exclude_list: + continue try: for test_case in entry.load()(): tests.addTests(loader.loadTestsFromNames( From 13f35da24a4c546d26a00d0ab3d8012bed6523a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 30 Nov 2019 04:38:10 +0100 Subject: [PATCH 2/6] doc/tests: extend qubes-specific quirks in tests - per-VM (template) tests - extra tests loader --- doc/qubes-tests.rst | 105 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/doc/qubes-tests.rst b/doc/qubes-tests.rst index a5dd06ec..1cea7095 100644 --- a/doc/qubes-tests.rst +++ b/doc/qubes-tests.rst @@ -33,7 +33,11 @@ variables. :py:class:`qubes.tests.QubesTestCase` classes should be named named ``test_xxx_test_name``, where ``xxx`` is three-digit number. You may introduce some structure of your choice in this number. -FIXME: where are placed integration tests? +Integration tests for Qubes core features are stored in :file:`tests/integ/` +directory. Additional tests may be loaded from other packages (see extra test +loader below). Those tests are run only on real Qubes system and are not suitable +for running in VM or in Travis. Test classes of this category inherit from +:py:class:`qubes.tests.SystemTestCase`. Writing tests ------------- @@ -133,6 +137,105 @@ even entire class) to be skipped outside dom0. Use it freely:: # all tests in this class are skipped pass +VM tests +^^^^^^^^ + +Some integration tests verifies not only dom0 part of the system, but also VM +part. In those cases, it makes sense to iterate them for different templates. +Additionally, list of the templates can be dynamic (different templates +installed, only some considered for testing etc). +This can be achieved by creating a mixin class with the actual tests (a class +inheriting just from :py:class:`object`, instead of +:py:class:`qubes.tests.SystemTestCase` or :py:class:`unittest.TestCase`) and +then create actual test classes dynamically using +:py:func:`qubes.tests.create_testcases_for_templates`. +Test classes created this way will have :py:attr:`template` set to the template +name under test and also this template will be set as the default template +during the test execution. +The function takes a test class name prefix (template name will be appended to +it after '_' separator), a classes to inherit from (in most cases the just +created mixin and :py:class:`qubes.tests.SystemTestCase`) and a current module +object (use `sys.modules[__name__]`). The function will return created test +classes but also add them to the appropriate module (pointed by the *module* +parameter). This should be done in two cases: + +* :py:func:`load_tests` function - when test loader request list of tests +* on module import time, using a wrapper + :py:func:`qubes.tests.maybe_create_testcases_on_import` (will call the + function only if explicit list of templates is given, to avoid loading + :file:`qubes.xml` when just importing the module) + +An example boilerplate looks like this:: + + def create_testcases_for_templates(): + return qubes.tests.create_testcases_for_templates('TC_00_AppVM', + TC_00_AppVMMixin, qubes.tests.SystemTestCase, + module=sys.modules[__name__]) + + def load_tests(loader, tests, pattern): + tests.addTests(loader.loadTestsFromNames( + create_testcases_for_templates())) + return tests + + qubes.tests.maybe_create_testcases_on_import(create_testcases_for_templates) + +This will by default create tests for all the templates installed in the system. +Additionally, it is possible to control this process using environment +variables: + +* `QUBES_TEST_TEMPLATES` - space separated list of templates to test +* `QUBES_TEST_LOAD_ALL` - create tests for all the templates (by inspecting + the :file:`qubes.xml` file), even at module import time + +This is dynamic test creation is intentionally made compatible with Nose2 test +runner and its load_tests protocol implementation. + +Extra tests +^^^^^^^^^^^ + +Most tests live in this package, but it is also possible to store tests in other +packages while still using infrastructure provided here and include them in the +common test run. Loading extra tests is implemented in +:py:mod:`qubes.tests.extra`. To write test to be loaded this way, you need to +create test class(es) as usual. You can also use helper class +:py:class:`qubes.tests.extra.ExtraTestCase` (instead of +:py:class:`qubes.tests.SystemTestCase`) which provide few convenient functions +and hide usage of asyncio for simple cases (like `vm.start()`, `vm.run()`). + +The next step is to register the test class(es). You need to do this by defining +entry point for your package. There are two groups: + +* `qubes.tests.extra` - for general tests (called once) +* `qubes.tests.extra.for_template` - for per-VM tests (called for each template + under test) + +As a name in the group, choose something unique, preferably package name. An +object reference should point at the function that returns a list of test +classes. + +Example :file:`setup.py`:: + + from setuptools import setup + + setup( + name='splitgpg', + version='1.0', + packages=['splitgpg'], + entry_points={ + 'qubes.tests.extra.for_template': + 'splitgpg = splitgpg.tests:list_tests', + } + ) + +The test loading process can be additionally controlled with environment +variables: + +* `QUBES_TEST_EXTRA_INCLUDE` - space separated list of tests to include (named + by a name in an entry point, `splitgpg` in the above example); if defined, only + those extra tests will be loaded + +* `QUBES_TEST_EXTRA_EXCLUDE` - space separated list of tests to exclude + Module contents --------------- From 5d77cf22980b607dae601850a0ea1af5e3326195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 30 Nov 2019 05:20:08 +0100 Subject: [PATCH 3/6] Avoid resetting clocksync service of just enabled clockvm When setting global clockvm property, 'clocksync' service is automatically added to the new value and then removed from the old one. But if the new value is the same as the old one, the service gets removed from the just set new value. Check for this case explicitly. Fixes QubesOS/qubes-issues#4939 --- qubes/app.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qubes/app.py b/qubes/app.py index 0177148f..7e1576b7 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -1473,6 +1473,8 @@ class Qubes(qubes.PropertyHolder): @qubes.events.handler('property-set:clockvm') def on_property_set_clockvm(self, event, name, newvalue, oldvalue=None): # pylint: disable=unused-argument,no-self-use + if oldvalue == newvalue: + return if oldvalue and oldvalue.features.get('service.clocksync', False): del oldvalue.features['service.clocksync'] From 10f99e5c4abb7dd00134c5506b8626c7399debbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 3 Dec 2019 06:27:37 +0100 Subject: [PATCH 4/6] api/admin: implement *.property.GetAll methods Allow getting all the VM properties with one call. This greatly improve performance of an applications retrieving many/all of them (qvm-ls, qubes manager etc) QubesOS/qubes-issues#5415 Fixes QubesOS/qubes-issues#3293 --- Makefile | 2 ++ qubes/api/admin.py | 42 ++++++++++++++++++++++++++++++++++++---- qubes/tests/api_admin.py | 32 ++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index e5538477..d871570e 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,7 @@ ADMIN_API_METHODS_SIMPLE = \ admin.pool.volume.Set.rw \ admin.pool.volume.Snapshot \ admin.property.Get \ + admin.property.GetAll \ admin.property.GetDefault \ admin.property.Help \ admin.property.HelpRst \ @@ -87,6 +88,7 @@ ADMIN_API_METHODS_SIMPLE = \ admin.vm.firewall.SetPolicy \ admin.vm.firewall.Reload \ admin.vm.property.Get \ + admin.vm.property.GetAll \ admin.vm.property.GetDefault \ admin.vm.property.Help \ admin.vm.property.HelpRst \ diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 4432b00f..678cb98a 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -169,7 +169,11 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): self.fire_event_for_permission() - property_def = dest.property_get_def(self.arg) + return self._serialize_property(dest, self.arg) + + @staticmethod + def _serialize_property(dest, prop): + property_def = dest.property_get_def(prop) # explicit list to be sure that it matches protocol spec if isinstance(property_def, qubes.vm.VMProperty): property_type = 'vm' @@ -177,21 +181,51 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): property_type = 'int' elif property_def.type is bool: property_type = 'bool' - elif self.arg == 'label': + elif prop == 'label': property_type = 'label' else: property_type = 'str' try: - value = getattr(dest, self.arg) + value = getattr(dest, str(prop)) except AttributeError: return 'default=True type={} '.format(property_type) else: return 'default={} type={} {}'.format( - str(dest.property_is_default(self.arg)), + str(dest.property_is_default(prop)), property_type, str(value) if value is not None else '') + @qubes.api.method('admin.vm.property.GetAll', no_payload=True, + scope='local', read=True) + @asyncio.coroutine + def vm_property_get_all(self): + """Get values of all VM properties""" + return self._property_get_all(self.dest) + + @qubes.api.method('admin.property.GetAll', no_payload=True, + scope='global', read=True) + @asyncio.coroutine + def property_get_all(self): + """Get value all global properties""" + self.enforce(self.dest.name == 'dom0') + return self._property_get_all(self.app) + + def _property_get_all(self, dest): + self.enforce(not self.arg) + + properties = dest.property_list() + + properties = self.fire_event_for_filter(properties) + + return ''.join( + '{} {}\n'.format(str(prop), + self._serialize_property(dest, prop). + replace('\\', '\\\\').replace('\n', '\\n')) + for prop in sorted(properties)) + + + @qubes.api.method('admin.vm.property.GetDefault', no_payload=True, scope='local', read=True) @asyncio.coroutine diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index da964636..a35a1b64 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -180,6 +180,38 @@ class TC_00_VMs(AdminAPITestCase): b'provides_network') self.assertEqual(value, 'type=bool False') + def test_027_vm_property_get_all(self): + # any string property, test \n encoding + self.vm.kernelopts = 'opt1\nopt2\nopt3\\opt4' + with unittest.mock.patch.object(self.vm, 'property_list') as list_mock: + list_mock.return_value = [ + self.vm.property_get_def('name'), + self.vm.property_get_def('default_user'), + self.vm.property_get_def('netvm'), + self.vm.property_get_def('klass'), + self.vm.property_get_def('debug'), + self.vm.property_get_def('label'), + self.vm.property_get_def('kernelopts'), + self.vm.property_get_def('qrexec_timeout'), + self.vm.property_get_def('qid'), + self.vm.property_get_def('updateable'), + ] + value = self.call_mgmt_func(b'admin.vm.property.GetAll', b'test-vm1') + self.maxDiff = None + expected = '''debug default=True type=bool False +default_user default=True type=str user +klass default=True type=str AppVM +label default=False type=label red +name default=False type=str test-vm1 +qid default=False type=int 2 +qrexec_timeout default=True type=int 60 +updateable default=True type=bool False +kernelopts default=False type=str opt1\\nopt2\\nopt3\\\\opt4 +netvm default=True type=vm +''' + self.assertEqual(value, expected) + + def test_030_vm_property_set_vm(self): netvm = self.app.add_new_vm('AppVM', label='red', name='test-net', template='test-template', provides_network=True) From ffa1a40e6e9d524b8fe59a88b79275fbb9656674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 3 Dec 2019 13:24:58 +0100 Subject: [PATCH 5/6] tests/network: improve error reporting Include stdout/stderr of failed command during netvm setup. --- qubes/tests/integ/network.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/qubes/tests/integ/network.py b/qubes/tests/integ/network.py index 63298a69..9f8ba06d 100644 --- a/qubes/tests/integ/network.py +++ b/qubes/tests/integ/network.py @@ -93,8 +93,12 @@ class VmNetworkingMixin(object): :type self: qubes.tests.SystemTestCase | VMNetworkingMixin ''' def run_netvm_cmd(cmd): - if self.run_cmd(self.testnetvm, cmd) != 0: - self.fail("Command '%s' failed" % cmd) + try: + self.loop.run_until_complete( + self.testnetvm.run_for_stdio(cmd, user='root')) + except subprocess.CalledProcessError as e: + self.fail("Command '%s' failed: %s%s" % + (cmd, e.stdout.decode(), e.stderr.decode())) if not self.testnetvm.is_running(): self.loop.run_until_complete(self.testnetvm.start()) @@ -783,8 +787,12 @@ class VmIPv6NetworkingMixin(VmNetworkingMixin): super(VmIPv6NetworkingMixin, self).configure_netvm() def run_netvm_cmd(cmd): - if self.run_cmd(self.testnetvm, cmd) != 0: - self.fail("Command '%s' failed" % cmd) + try: + self.loop.run_until_complete( + self.testnetvm.run_for_stdio(cmd, user='root')) + except subprocess.CalledProcessError as e: + self.fail("Command '%s' failed: %s%s" % + (cmd, e.stdout.decode(), e.stderr.decode())) run_netvm_cmd("ip addr add {}/128 dev test0".format(self.test_ip6)) run_netvm_cmd( From 8965e9a8e4e3bd26665a07720eb091ffed486404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 3 Dec 2019 13:43:19 +0100 Subject: [PATCH 6/6] tests/network: let xl devd bring the interfaces up xl devd doesn't use startup notify, so when the service is started (according to systemd) it may still be initializing interfaces. Add a little sleep for that. --- qubes/tests/integ/network.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qubes/tests/integ/network.py b/qubes/tests/integ/network.py index 9f8ba06d..5759e2df 100644 --- a/qubes/tests/integ/network.py +++ b/qubes/tests/integ/network.py @@ -389,6 +389,9 @@ class VmNetworkingMixin(object): if self.run_cmd(self.testnetvm, cmd) != 0: self.fail("Command '%s' failed" % cmd) + # let it initialize the interface(s) + time.sleep(1) + self.assertEqual(self.run_cmd(self.testvm1, self.ping_ip), 0) def test_110_dynamic_attach(self):