[PATCH 00/12] fix many test failures in various build config scenarios
I was annoyed that meson build --auto-features=disabled -Dtests=enabled will fail in unit tests. Rather than just fix that scenario, I went down the rabbit hole and tested (almost) every single minimal build option. By that I mean I looked at meson_options.txt and got a list of every option that has 'auto' as its default. I then attempted a minimal build with only that enabled. I quickly found some options have mandatory pre-requisite options, so I re-did the tests with such deps present. This uncovered many scenarios where we didn't use the right conditions for tests. Surprisingly (at first, but not in retrospect) it also found many places where the tests failed to depend on the earlier build artifacts. My test process was: opts=$(grep auto meson_options.txt | sed -e 's/option(//' -e 's/,.*//' -e "s/'//g") for opt in $opts do deps=`grep --before 1 "option('$opt'" meson_options.txt | grep dep: | head -1 | sed -e 's/# //' -e 's/dep:/-D/g' -e 's/$/=enabled/' -e 's/ -D/=enabled -D/g'` echo "Try test $opt with $deps" rm -rf build meson setup build --auto-features=disabled -Dtests=enabled $deps -D$opt=enabled 1> opt-$opt.log 2>&1 meson test -C build --no-suite syntax-check --print-errorlogs 1>> opt-$opt.log 2>&1 ; done done Then to check results grep '^Fail:' opt-*.log | grep -v 0 This is slow, but not as slow as you might think, because each build is very minimal, so we're not actually building more than 300-400 files each time, rather than 1000's, and we're not running as many tests either. Might be interesting to get extensive test into CI. Strictly speaking we care about the combinatorial expansion of meson_options.txt, but that would be insanely slow and is likely to be overkill. Daniel P. Berrangé (12): scripts/rpcgen: skip tests if tirpc is not present tests: fix tests when test driver is disabled meson: record which other options are a pre-requisite docs: ensure HTML/images are built before running reference tests src: ensure augeas test file is generated before running test tests: build 'virsh' before running virsh-auth test tests: build driver modules before virdrivermoduletest test: conditionalize 'virsh-auth' on test driver tests: always build securityselinuxhelper if libselinux is present test: drop bogus check for YAJL from libxl test/mock tests: don't run mdevctl test if lacking YAJL src/node_device: don't overwrite error messages docs/images/meson.build | 5 ++- docs/logos/meson.build | 5 ++- docs/meson.build | 1 + meson_options.txt | 20 +++++++++ scripts/rpcgen/meson.build | 2 +- src/meson.build | 6 ++- src/node_device/node_device_driver.c | 2 - tests/fchosttest.c | 9 ++-- tests/libxlmock.c | 4 +- tests/libxlxml2domconfigtest.c | 4 +- tests/meson.build | 67 ++++++++++++++++++---------- 11 files changed, 87 insertions(+), 38 deletions(-) -- 2.43.0
This skips building tests which rely on tirpc when it is not present. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- scripts/rpcgen/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/rpcgen/meson.build b/scripts/rpcgen/meson.build index 7fb32c0f91..bce652c7ad 100644 --- a/scripts/rpcgen/meson.build +++ b/scripts/rpcgen/meson.build @@ -1,6 +1,6 @@ subdir('rpcgen') -if tests_enabled[0] +if tests_enabled[0] and xdr_dep.found() subdir('tests') if pytest_prog.found() and host_machine.system() != 'darwin' -- 2.43.0
Various tests try to open a connection to 'test:///default' and must be skipped when the test driver is disabled. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/fchosttest.c | 9 ++++++--- tests/meson.build | 37 ++++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index d050458a58..14f9975157 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -33,6 +33,7 @@ static char *fchost_prefix; #define TEST_FC_HOST_NUM 5 #define TEST_FC_HOST_NUM_NO_FAB 6 +#ifdef WITH_TEST /* virNodeDeviceCreateXML using "<parent>" to find the vport capable HBA */ static const char test7_xml[] = "<device>" @@ -85,7 +86,7 @@ static const char test11_xml[] = " <path>/dev/disk/by-path</path>" " </target>" "</pool>"; - +#endif /* WITH_TEST */ /* Test virIsVHBACapable */ static int @@ -222,7 +223,7 @@ test6(const void *data G_GNUC_UNUSED) } - +#ifdef WITH_TEST /* Test manageVHBAByNodeDevice * - Test both virNodeDeviceCreateXML and virNodeDeviceDestroy * - Create a node device vHBA allowing usage of various different @@ -313,7 +314,7 @@ manageVHBAByStoragePool(const void *data) virConnectClose(conn); return ret; } - +#endif static int mymain(void) @@ -334,6 +335,7 @@ mymain(void) ret = -1; if (virTestRun("virVHBAGetConfig-empty-fabric_wwn", test6, NULL) < 0) ret = -1; +#ifdef WITH_TEST if (virTestRun("manageVHBAByNodeDevice-by-parent", manageVHBAByNodeDevice, test7_xml) < 0) ret = -1; @@ -349,6 +351,7 @@ mymain(void) if (virTestRun("manageVHBAByStoragePool-by-parent", manageVHBAByStoragePool, test11_xml) < 0) ret = -1; +#endif VIR_FREE(fchost_prefix); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/tests/meson.build b/tests/meson.build index 7270840428..73b87be2a0 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -74,7 +74,6 @@ endif mock_libs = [ { 'name': 'domaincapsmock' }, - { 'name': 'shunload', 'sources': [ 'shunloadhelper.c' ] }, { 'name': 'vircgroupmock' }, { 'name': 'virdnsmasqmock' }, { 'name': 'virfilecachemock' }, @@ -106,6 +105,11 @@ if host_machine.system() != 'windows' ] endif +if conf.has('WITH_TEST') + mock_libs += [ + { 'name': 'shunload', 'sources': [ 'shunloadhelper.c' ] }, + ] +endif # build libraries used by tests @@ -258,15 +262,11 @@ tests += [ { 'name': 'domainconftest' }, { 'name': 'genericxml2xmltest' }, { 'name': 'interfacexml2xmltest' }, - { 'name': 'metadatatest' }, - { 'name': 'networkmetadatatest' }, { 'name': 'networkxml2xmlupdatetest' }, { 'name': 'nodedevxml2xmltest' }, { 'name': 'nwfilterxml2xmltest' }, - { 'name': 'objecteventtest' }, { 'name': 'seclabeltest' }, { 'name': 'secretxml2xmltest' }, - { 'name': 'shunloadtest', 'deps': [ thread_dep ] }, { 'name': 'sockettest' }, { 'name': 'storagevolxml2xmltest' }, { 'name': 'sysinfotest' }, @@ -298,7 +298,6 @@ tests += [ { 'name': 'virportallocatortest' }, { 'name': 'virrotatingfiletest' }, { 'name': 'virschematest' }, - { 'name': 'virshtest', 'depends': [ virsh_prog ] }, { 'name': 'virstringtest' }, { 'name': 'virsystemdtest' }, { 'name': 'virtimetest' }, @@ -333,6 +332,17 @@ if host_machine.system() == 'linux' endif endif +if conf.has('WITH_TEST') + tests += [ + { 'name': 'fdstreamtest' }, + { 'name': 'metadatatest' }, + { 'name': 'networkmetadatatest' }, + { 'name': 'objecteventtest' }, + { 'name': 'shunloadtest', 'deps': [ thread_dep ] }, + { 'name': 'virshtest', 'depends': [ virsh_prog ] }, + ] +endif + if conf.has('WITH_BHYVE') tests += [ { 'name': 'bhyveargv2xmltest', 'link_with': [ bhyve_driver_impl ] }, @@ -364,7 +374,6 @@ endif if conf.has('WITH_LIBVIRTD') tests += [ { 'name': 'eventtest', 'deps': [ thread_dep ] }, - { 'name': 'fdstreamtest' }, { 'name': 'virdriverconnvalidatetest' }, { 'name': 'virdrivermoduletest' }, ] @@ -619,12 +628,14 @@ test( suite: 'script', ) -# vsh based client self-test, which can be run directly from meson -test('virsh self-test', - virsh_prog, - args: [ '-q', '-c', 'test:///default', 'self-test' ], - suite: 'bin', -) +if conf.has('WITH_TEST') + # vsh based client self-test, which can be run directly from meson + test('virsh self-test', + virsh_prog, + args: [ '-q', '-c', 'test:///default', 'self-test' ], + suite: 'bin', + ) +endif if conf.has('WITH_REMOTE') test('virt-admin self-test', -- 2.43.0
Several meson options cannot be enabled, without first enabling another option. This adds a small comment prior to an optino to record its mandatory dependencies. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson_options.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/meson_options.txt b/meson_options.txt index ed91d97abf..9d729b3e1f 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -4,6 +4,7 @@ option('packager_version', type: 'string', value: '', description: 'Extra packag option('system', type: 'boolean', value: false, description: 'Set install paths to system ones') option('runstatedir', type: 'string', value: '', description: 'State directory for temporary sockets, pid files, etc') option('initconfdir', type: 'string', value: '', description: 'directory for init script configuration files') +# dep:tests option('expensive_tests', type: 'feature', value: 'auto', description: 'set the default for enabling expensive tests (long timeouts)') option('test_coverage', type: 'boolean', value: false, description: 'turn on code coverage instrumentation') option('git_werror', type: 'feature', value: 'auto', description: 'use -Werror if building from GIT') @@ -17,6 +18,7 @@ option('tests', type: 'feature', value: 'auto', description: 'whether to build a option('apparmor', type: 'feature', value: 'auto', description: 'apparmor support') option('attr', type: 'feature', value: 'auto', description: 'attr support') option('audit', type: 'feature', value: 'auto', description: 'audit support') +# dep:readline option('bash_completion', type: 'feature', value: 'auto', description: 'bash-completion support') option('bash_completion_dir', type: 'string', value: '', description: 'directory containing bash completion scripts') option('blkid', type: 'feature', value: 'auto', description: 'blkid support') @@ -40,7 +42,9 @@ option('sanlock', type: 'feature', value: 'auto', description: 'sanlock support' option('sasl', type: 'feature', value: 'auto', description: 'sasl support') option('selinux', type: 'feature', value: 'auto', description: 'selinux support') option('selinux_mount', type: 'string', value: '', description: 'set SELinux mount point') +# dep:pciaccess option('udev', type: 'feature', value: 'auto', description: 'udev support') +# dep:driver_remote option('wireshark_dissector', type: 'feature', value: 'auto', description: 'wireshark support') option('wireshark_plugindir', type: 'string', value: '', description: 'wireshark plugins directory for use when installing wireshark plugin') option('yajl', type: 'feature', value: 'auto', description: 'yajl support') @@ -48,17 +52,26 @@ option('yajl', type: 'feature', value: 'auto', description: 'yajl support') # build driver options option('driver_bhyve', type: 'feature', value: 'auto', description: 'bhyve driver') +# dep:curl option('driver_esx', type: 'feature', value: 'auto', description: 'esx driver') +# dep:openwsman option('driver_hyperv', type: 'feature', value: 'auto', description: 'Hyper-V driver') +# dep:pciaccess dep:udev dep:driver_remote dep:driver_libvirtd option('driver_interface', type: 'feature', value: 'auto', description: 'host interface driver') +# dep:driver_remote option('driver_libvirtd', type: 'feature', value: 'auto', description: 'libvirtd driver') +# dep:driver_remote dep:driver_libvirtd option('driver_libxl', type: 'feature', value: 'auto', description: 'libxenlight driver') +# dep:driver_remote dep:driver_libvirtd option('driver_lxc', type: 'feature', value: 'auto', description: 'Linux Container driver') +# dep:curl dep:yajl dep:driver_remote dep:driver_libvirtd option('driver_ch', type: 'feature', value: 'auto', description: 'Cloud-Hypervisor driver') option('ch_user', type: 'string', value: '', description: 'username to run Cloud-Hypervisor system instance as') option('ch_group', type: 'string', value: '', description: 'groupname to run Cloud-Hypervisor system instance as') +# dep:driver_remote dep:driver_libvirtd option('driver_network', type: 'feature', value: 'auto', description: 'virtual network driver') option('driver_openvz', type: 'feature', value: 'auto', description: 'OpenVZ driver') +# dep:yajl dep:driver_remote dep:driver_libvirtd option('driver_qemu', type: 'feature', value: 'auto', description: 'QEMU/KVM driver') option('qemu_user', type: 'string', value: '', description: 'username to run QEMU system instance as') option('qemu_group', type: 'string', value: '', description: 'groupname to run QEMU system instance as') @@ -74,7 +87,9 @@ option('driver_vmware', type: 'feature', value: 'auto', description: 'VMware dri option('driver_vz', type: 'feature', value: 'auto', description: 'Virtuozzo driver') option('secdriver_apparmor', type: 'feature', value: 'auto', description: 'use AppArmor security driver') +# dep:secdriver_apparmor option('apparmor_profiles', type: 'feature', value: 'auto', description: 'install apparmor profiles') +# dep:selinux option('secdriver_selinux', type: 'feature', value: 'auto', description: 'use SELinux security driver') @@ -97,16 +112,21 @@ option('storage_zfs', type: 'feature', value: 'auto', description: 'ZFS backend option('chrdev_lock_files', type: 'string', value: '', description: 'location for UUCP style lock files for character devices (leave empty for default paths on some platforms)') option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for static probing') option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') +# dep:firewalld option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate') option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install') option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.') option('login_shell', type: 'feature', value: 'auto', description: 'build virt-login-shell') +# dep:yajl dep:driver_network dep:libvirtd option('nss', type: 'feature', value: 'auto', description: 'enable Name Service Switch plugin for resolving guest IP addresses') +# dep:numactl option('numad', type: 'feature', value: 'auto', description: 'use numad to manage CPU placement dynamically') option('nbdkit', type: 'feature', value: 'auto', description: 'Build nbdkit storage backend') +# dep:nbdkit option('nbdkit_config_default', type: 'feature', value: 'auto', description: 'Whether to use nbdkit storage backend for network disks by default (configurable)') option('pm_utils', type: 'feature', value: 'auto', description: 'use pm-utils for power management') option('sysctl_config', type: 'feature', value: 'auto', description: 'Whether to install sysctl configs') +# dep:sysctl_config option('userfaultfd_sysctl', type: 'feature', value: 'auto', description: 'Whether to install sysctl config for enabling unprivileged userfaultfd') option('tls_priority', type: 'string', value: 'NORMAL', description: 'set the default TLS session priority string') -- 2.43.0
On Tue, May 07, 2024 at 14:58:58 +0100, Daniel P. Berrangé wrote:
Several meson options cannot be enabled, without first enabling another option. This adds a small comment prior to an optino to record its
s/optino/option/
mandatory dependencies.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson_options.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
The 'check-html-references' test will process the built HTML files, so they must exist before it is run, along with any images that they point to. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/images/meson.build | 5 +++-- docs/logos/meson.build | 5 +++-- docs/meson.build | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/images/meson.build b/docs/images/meson.build index b9ab30fc91..cccd351f05 100644 --- a/docs/images/meson.build +++ b/docs/images/meson.build @@ -18,10 +18,11 @@ foreach file : docs_image_files # This hack enables us to view the web pages # from within the uninstalled build tree if meson.version().version_compare('>=0.64.0') - fs.copyfile(file) + imgfile = fs.copyfile(file) else - configure_file(input: file, output: file, copy: true) + imgfile = configure_file(input: file, output: file, copy: true) endif + install_web_deps += [imgfile] install_web_files += '@0@:@1@'.format(meson.current_source_dir() / file, docs_html_dir / 'images') endforeach diff --git a/docs/logos/meson.build b/docs/logos/meson.build index c3f4c9f522..b4ea070a34 100644 --- a/docs/logos/meson.build +++ b/docs/logos/meson.build @@ -26,11 +26,12 @@ foreach file : docs_logo_files # This hack enables us to view the web pages # from within the uninstalled build tree if meson.version().version_compare('>=0.64.0') - fs.copyfile(file) + logofile = fs.copyfile(file) else - configure_file(input: file, output: file, copy: true) + logofile = configure_file(input: file, output: file, copy: true) endif + install_web_deps += [logofile] install_web_files += '@0@:@1@'.format(meson.current_source_dir() / file, docs_html_dir / 'logos') endforeach diff --git a/docs/meson.build b/docs/meson.build index 87d728213c..2dfe98ef62 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -360,6 +360,7 @@ if tests_enabled[0] '--webroot', meson.project_build_root() / 'docs' ], + depends: install_web_deps, env: runutf8, suite: 'script' ) -- 2.43.0
On Tue, May 07, 2024 at 02:58:59PM +0100, Daniel P. Berrangé wrote:
The 'check-html-references' test will process the built HTML files, so they must exist before it is run, along with any images that they point to.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/images/meson.build | 5 +++-- docs/logos/meson.build | 5 +++-- docs/meson.build | 1 + 3 files changed, 7 insertions(+), 4 deletions(-)
This seems to have a bit of a backcompat problems. Smoe versions of meson are complaining docs/meson.build:345:0: ERROR: run_target keyword argument 'depends' was of type array[CustomTarget | File] but should have been array[BuildTarget | CustomTarget]
diff --git a/docs/images/meson.build b/docs/images/meson.build index b9ab30fc91..cccd351f05 100644 --- a/docs/images/meson.build +++ b/docs/images/meson.build @@ -18,10 +18,11 @@ foreach file : docs_image_files # This hack enables us to view the web pages # from within the uninstalled build tree if meson.version().version_compare('>=0.64.0') - fs.copyfile(file) + imgfile = fs.copyfile(file) else - configure_file(input: file, output: file, copy: true) + imgfile = configure_file(input: file, output: file, copy: true)
I think this 'else' clause is the problem - doesn't like using the result of 'configiure_file' as a dependency
endif
+ install_web_deps += [imgfile] install_web_files += '@0@:@1@'.format(meson.current_source_dir() / file, docs_html_dir / 'images') endforeach diff --git a/docs/logos/meson.build b/docs/logos/meson.build index c3f4c9f522..b4ea070a34 100644 --- a/docs/logos/meson.build +++ b/docs/logos/meson.build @@ -26,11 +26,12 @@ foreach file : docs_logo_files # This hack enables us to view the web pages # from within the uninstalled build tree if meson.version().version_compare('>=0.64.0') - fs.copyfile(file) + logofile = fs.copyfile(file) else - configure_file(input: file, output: file, copy: true) + logofile = configure_file(input: file, output: file, copy: true) endif
+ install_web_deps += [logofile] install_web_files += '@0@:@1@'.format(meson.current_source_dir() / file, docs_html_dir / 'logos') endforeach
diff --git a/docs/meson.build b/docs/meson.build index 87d728213c..2dfe98ef62 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -360,6 +360,7 @@ if tests_enabled[0] '--webroot', meson.project_build_root() / 'docs' ], + depends: install_web_deps, env: runutf8, suite: 'script' ) -- 2.43.0
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 5/7/24 16:23, Daniel P. Berrangé wrote:
On Tue, May 07, 2024 at 02:58:59PM +0100, Daniel P. Berrangé wrote:
The 'check-html-references' test will process the built HTML files, so they must exist before it is run, along with any images that they point to.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/images/meson.build | 5 +++-- docs/logos/meson.build | 5 +++-- docs/meson.build | 1 + 3 files changed, 7 insertions(+), 4 deletions(-)
This seems to have a bit of a backcompat problems. Smoe versions of meson are complaining
docs/meson.build:345:0: ERROR: run_target keyword argument 'depends' was of type array[CustomTarget | File] but should have been array[BuildTarget | CustomTarget]
I believe that after we drop old build targets from our CI then meson can be bumped to something newer. Michal
On Tue, May 07, 2024 at 04:30:55PM +0200, Michal Prívozník wrote:
On 5/7/24 16:23, Daniel P. Berrangé wrote:
On Tue, May 07, 2024 at 02:58:59PM +0100, Daniel P. Berrangé wrote:
The 'check-html-references' test will process the built HTML files, so they must exist before it is run, along with any images that they point to.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/images/meson.build | 5 +++-- docs/logos/meson.build | 5 +++-- docs/meson.build | 1 + 3 files changed, 7 insertions(+), 4 deletions(-)
This seems to have a bit of a backcompat problems. Smoe versions of meson are complaining
docs/meson.build:345:0: ERROR: run_target keyword argument 'depends' was of type array[CustomTarget | File] but should have been array[BuildTarget | CustomTarget]
I believe that after we drop old build targets from our CI then meson can be bumped to something newer.
Unfortunately we can't bump it far enough. I hit failures on opensuse leap 15 (meson 0.61), ubuntu 22.04 (meson 0.61) and centos stream 9 (meson 0.63). There's probably some other way I can express this to be compatible. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
We fail to express an ordering between the custom target that generates the combined augeas test input file, and the meson test command. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/meson.build b/src/meson.build index a51f40fe16..17e6feafba 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1048,7 +1048,8 @@ if tests_enabled[0] '-I', data['builddir'], data['file'].full_path(), ], - suite: 'script' + suite: 'script', + depends: [data['file']] ) endforeach endif -- 2.43.0
The 'virsh-auth' test needs to be able to invoke the 'virsh' binary Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/meson.build | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 73b87be2a0..68fe00d0c1 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -720,17 +720,23 @@ if conf.has('WITH_LIBVIRTD') ) test_scripts += [ - 'virsh-auth', + { 'name': 'virsh-auth', 'depends': [virsh_prog] } ] if conf.has('WITH_SECDRIVER_APPARMOR') - test_scripts += 'virt-aa-helper-test' + test_scripts += { 'name': 'virt-aa-helper-test' } endif endif -foreach name : test_scripts - script = find_program(name) - test(name, script, env: tests_env, suite: 'script') +foreach data : test_scripts + script = find_program(data['name']) + test(data['name'], + script, + env: tests_env, + depends: [ + data.get('depends', []), + ], + suite: 'script') endforeach testenv = runutf8 -- 2.43.0
On Tue, May 07, 2024 at 14:59:01 +0100, Daniel P. Berrangé wrote:
The 'virsh-auth' test needs to be able to invoke the 'virsh' binary
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/meson.build | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tests/meson.build b/tests/meson.build index 73b87be2a0..68fe00d0c1 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -720,17 +720,23 @@ if conf.has('WITH_LIBVIRTD') )
test_scripts += [ - 'virsh-auth', + { 'name': 'virsh-auth', 'depends': [virsh_prog] }
In few places I've checked we use '[ DEPNAME ]' instead of the compressed variant.
The virdrivermoduletest will attempt to dlopen() each driver module, so they must be build before the test can run. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/meson.build | 3 +++ tests/meson.build | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/meson.build b/src/meson.build index 17e6feafba..fead0de998 100644 --- a/src/meson.build +++ b/src/meson.build @@ -578,6 +578,8 @@ endif # build libvirt shared modules +virt_module_deps = [] + foreach module : virt_modules mod = shared_module( module['name'], @@ -607,6 +609,7 @@ foreach module : virt_modules install_rpath: libvirt_rpath, ) set_variable('@0@_module'.format(module['name'].underscorify()), mod) + virt_module_deps += [mod] endforeach diff --git a/tests/meson.build b/tests/meson.build index 68fe00d0c1..76c05b9fed 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -375,7 +375,7 @@ if conf.has('WITH_LIBVIRTD') tests += [ { 'name': 'eventtest', 'deps': [ thread_dep ] }, { 'name': 'virdriverconnvalidatetest' }, - { 'name': 'virdrivermoduletest' }, + { 'name': 'virdrivermoduletest', 'depends': virt_module_deps }, ] endif -- 2.43.0
On Tue, May 07, 2024 at 14:59:02 +0100, Daniel P. Berrangé wrote:
The virdrivermoduletest will attempt to dlopen() each driver module, so they must be build before the test can run.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/meson.build | 3 +++ tests/meson.build | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/meson.build b/src/meson.build index 17e6feafba..fead0de998 100644 --- a/src/meson.build +++ b/src/meson.build @@ -578,6 +578,8 @@ endif
# build libvirt shared modules
+virt_module_deps = [] + foreach module : virt_modules mod = shared_module( module['name'], @@ -607,6 +609,7 @@ foreach module : virt_modules install_rpath: libvirt_rpath, ) set_variable('@0@_module'.format(module['name'].underscorify()), mod) + virt_module_deps += [mod]
Same spacing comment as in previous patch
On Tue, May 07, 2024 at 14:59:02 +0100, Daniel P. Berrangé wrote:
The virdrivermoduletest will attempt to dlopen() each driver module, so they must be build before the test can run.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/meson.build | 3 +++ tests/meson.build | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/meson.build b/src/meson.build index 17e6feafba..fead0de998 100644 --- a/src/meson.build +++ b/src/meson.build @@ -578,6 +578,8 @@ endif
# build libvirt shared modules
+virt_module_deps = [] + foreach module : virt_modules mod = shared_module( module['name'], @@ -607,6 +609,7 @@ foreach module : virt_modules install_rpath: libvirt_rpath, ) set_variable('@0@_module'.format(module['name'].underscorify()), mod) + virt_module_deps += [mod]
Does this even require the array/list brackets?
The 'virsh-auth' test is mistakenly conditionalized on the libvirtd daemon build, however, it just uses the 'test:///default' driver URI, so does not require a daemon. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/meson.build | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 76c05b9fed..bb244d6c61 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -711,6 +711,12 @@ endforeach # list of test scripts to run test_scripts = [] +if conf.has('WITH_TEST') + test_scripts += [ + { 'name': 'virsh-auth', 'depends': [virsh_prog] } + ] +endif + if conf.has('WITH_LIBVIRTD') test('libvirtd fail with missing config', libvirtd_prog, @@ -719,10 +725,6 @@ if conf.has('WITH_LIBVIRTD') suite: 'bin', ) - test_scripts += [ - { 'name': 'virsh-auth', 'depends': [virsh_prog] } - ] - if conf.has('WITH_SECDRIVER_APPARMOR') test_scripts += { 'name': 'virt-aa-helper-test' } endif -- 2.43.0
The securityselinuxhelper build is conditionalized on the SELinux security driver feature. It is also needed, however, by viridentitytest whenever libselinux is present. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/meson.build b/tests/meson.build index bb244d6c61..027567d25c 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -508,7 +508,9 @@ if conf.has('WITH_SECDRIVER_SELINUX') ] endif endif +endif +if conf.has('WITH_SELINUX') mock_libs += [ { 'name': 'securityselinuxhelper' }, ] -- 2.43.0
The libxlmock.c conditionalizes on WITH_YAJL, but this mock is used from other tests which only conditionalize on WITH_LIBXL. The libxl code does not have any dependancy on YAJL, so the bogus condition can be removed from the mock and also from libxlxml2domconfigtest.c Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/libxlmock.c | 4 ++-- tests/libxlxml2domconfigtest.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/libxlmock.c b/tests/libxlmock.c index f564a0ef72..8b697702b5 100644 --- a/tests/libxlmock.c +++ b/tests/libxlmock.c @@ -20,7 +20,7 @@ #include <config.h> -#if defined(WITH_LIBXL) && defined(WITH_YAJL) +#if defined(WITH_LIBXL) # include "virmock.h" # include <sys/stat.h> # include <unistd.h> @@ -168,4 +168,4 @@ libxlDomainGetEmulatorType(const virDomainDef *def G_GNUC_UNUSED) return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; } -#endif /* WITH_LIBXL && WITH_YAJL */ +#endif /* WITH_LIBXL */ diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 6418341564..21c4e7d149 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -28,7 +28,7 @@ #include "testutils.h" -#if defined(WITH_LIBXL) && defined(WITH_YAJL) +#if defined(WITH_LIBXL) # include "internal.h" # include "libxl/libxl_conf.h" @@ -224,4 +224,4 @@ int main(void) return EXIT_AM_SKIP; } -#endif /* WITH_LIBXL && WITH_YAJL */ +#endif /* WITH_LIBXL */ -- 2.43.0
The mdev code requires YAJL in order to convert from node dev XML to mdev's config format. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/meson.build b/tests/meson.build index 027567d25c..0d83dd445a 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -402,7 +402,7 @@ if conf.has('WITH_NETWORK') ] endif -if conf.has('WITH_NODE_DEVICES') +if conf.has('WITH_NODE_DEVICES') and conf.has('WITH_YAJL') tests += [ { 'name': 'nodedevmdevctltest', 'link_with': [ node_device_driver_impl ] }, ] -- 2.43.0
The nodedev code unhelpfully reports couldn't convert node device def to mdevctl JSON which hides the actual error message No JSON parser implementation is available Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/node_device/node_device_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index d99b48138e..700776c90c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -774,8 +774,6 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def, } if (nodeDeviceDefToMdevctlConfig(def, &inbuf, true) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("couldn't convert node device def to mdevctl JSON")); return NULL; } -- 2.43.0
On Tue, May 07, 2024 at 14:58:55 +0100, Daniel P. Berrangé wrote:
I was annoyed that
meson build --auto-features=disabled -Dtests=enabled
will fail in unit tests. Rather than just fix that scenario, I went down the rabbit hole and tested (almost) every single minimal build option.
By that I mean I looked at meson_options.txt and got a list of every option that has 'auto' as its default. I then attempted a minimal build with only that enabled. I quickly found some options have mandatory pre-requisite options, so I re-did the tests with such deps present.
This uncovered many scenarios where we didn't use the right conditions for tests. Surprisingly (at first, but not in retrospect) it also found many places where the tests failed to depend on the earlier build artifacts.
My test process was:
opts=$(grep auto meson_options.txt | sed -e 's/option(//' -e 's/,.*//' -e "s/'//g")
for opt in $opts do deps=`grep --before 1 "option('$opt'" meson_options.txt | grep dep: | head -1 | sed -e 's/# //' -e 's/dep:/-D/g' -e 's/$/=enabled/' -e 's/ -D/=enabled -D/g'` echo "Try test $opt with $deps" rm -rf build meson setup build --auto-features=disabled -Dtests=enabled $deps -D$opt=enabled 1> opt-$opt.log 2>&1 meson test -C build --no-suite syntax-check --print-errorlogs 1>> opt-$opt.log 2>&1 ; done done
Then to check results
grep '^Fail:' opt-*.log | grep -v 0
This is slow, but not as slow as you might think, because each build is very minimal, so we're not actually building more than 300-400 files each time, rather than 1000's, and we're not running as many tests either.
Might be interesting to get extensive test into CI.
Strictly speaking we care about the combinatorial expansion of meson_options.txt, but that would be insanely slow and is likely to be overkill.
Whoah, that's rather cool. But as you note I don't think this is anything for CI and even if, this'd be something to run once a month perhaps. Otherwise it's almost as useful as mining cryptocurrencies With the few things I've pointed out inline: Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (3)
-
Daniel P. Berrangé -
Michal Prívozník -
Peter Krempa