[libvirt] [PATCH] tests: Move tools under tests/tools/
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 +- Makefile.am | 2 +- cfg.mk | 4 +- configure.ac | 1 + tests/Makefile.am | 22 +----- tests/qemucapabilitiestest.c | 4 +- tests/testutils.c | 2 +- tests/{ => tools}/.valgrind.supp | 0 tests/tools/Makefile.am | 85 +++++++++++++++++++++ tests/{ => tools}/check-file-access.pl | 0 tests/{ => tools}/file_access_whitelist.txt | 0 tests/{ => tools}/group-qemu-caps.pl | 0 tests/{ => tools}/oomtrace.pl | 0 tests/{ => tools}/qemucapsprobe.c | 0 tests/{ => tools}/qemucapsprobemock.c | 0 tests/{ => tools}/test-wrap-argv.pl | 2 +- 16 files changed, 98 insertions(+), 26 deletions(-) rename tests/{ => tools}/.valgrind.supp (100%) create mode 100644 tests/tools/Makefile.am rename tests/{ => tools}/check-file-access.pl (100%) rename tests/{ => tools}/file_access_whitelist.txt (100%) rename tests/{ => tools}/group-qemu-caps.pl (100%) rename tests/{ => tools}/oomtrace.pl (100%) rename tests/{ => tools}/qemucapsprobe.c (100%) rename tests/{ => tools}/qemucapsprobemock.c (100%) rename tests/{ => tools}/test-wrap-argv.pl (98%) diff --git a/.gitignore b/.gitignore index 16eb4a3e2e..c231d394f3 100644 --- a/.gitignore +++ b/.gitignore @@ -170,7 +170,7 @@ /tests/*.trs /tests/*test /tests/commandhelper -/tests/qemucapsprobe +/tests/tools/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test !/tests/virt-admin-self-test diff --git a/Makefile.am b/Makefile.am index eba5916352..875c0fa997 100644 --- a/Makefile.am +++ b/Makefile.am @@ -17,7 +17,7 @@ ## <http://www.gnu.org/licenses/>. SUBDIRS = . gnulib/lib include/libvirt src tools docs gnulib/tests \ - tests po examples + tests tests/tools po examples XZ_OPT ?= -v -T0 export XZ_OPT diff --git a/cfg.mk b/cfg.mk index b785089910..5e055023ee 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1169,10 +1169,10 @@ header-ifdef: test-wrap-argv: $(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | xargs \ - $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check + $(PERL) $(top_srcdir)/tests/tools/test-wrap-argv.pl --check group-qemu-caps: - $(AM_V_GEN)$(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check $(top_srcdir)/ + $(AM_V_GEN)$(PERL) $(top_srcdir)/tests/tools/group-qemu-caps.pl --check $(top_srcdir)/ # sc_po_check can fail if generated files are not built first sc_po_check: \ diff --git a/configure.ac b/configure.ac index fabec815db..893d0db17a 100644 --- a/configure.ac +++ b/configure.ac @@ -925,6 +925,7 @@ AC_CONFIG_FILES([\ include/libvirt/libvirt-common.h \ examples/Makefile \ tests/Makefile \ + tests/tools/Makefile \ tools/Makefile]) AC_OUTPUT diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..0f5a5c231e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -75,7 +75,6 @@ MOCKLIBS_LIBS = \ ../src/libvirt.la EXTRA_DIST = \ - .valgrind.supp \ bhyvexml2argvdata \ bhyveargv2xmldata \ bhyvexml2xmloutdata \ @@ -107,7 +106,6 @@ EXTRA_DIST = \ nwfilterxml2firewalldata \ nwfilterxml2xmlin \ nwfilterxml2xmlout \ - oomtrace.pl \ qemuagentdata \ qemuargv2xmldata \ qemublocktestdata \ @@ -285,12 +283,10 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemusecuritytest \ qemufirmwaretest \ $(NULL) -test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ libqemutestdriver.la \ qemuxml2argvmock.la \ qemucaps2xmlmock.la \ - qemucapsprobemock.la \ qemucpumock.la \ $(NULL) endif WITH_QEMU @@ -442,15 +438,15 @@ EXTRA_DIST += $(test_scripts) if WITH_LINUX check-access: file-access-clean VIR_TEST_FILE_ACCESS=1 $(MAKE) $(AM_MAKEFLAGS) check - $(PERL) check-file-access.pl | sort -u + $(PERL) tools/check-file-access.pl | sort -u file-access-clean: > test_file_access.txt endif WITH_LINUX EXTRA_DIST += \ - check-file-access.pl \ - file_access_whitelist.txt + tools/check-file-access.pl \ + tools/file_access_whitelist.txt if WITH_TESTS noinst_PROGRAMS = $(test_programs) $(test_helpers) @@ -478,7 +474,7 @@ TESTS_ENVIRONMENT = \ VALGRIND = valgrind --quiet --leak-check=full --trace-children=yes \ --trace-children-skip="*/tools/virsh","*/tests/commandhelper" \ - --suppressions=$(abs_srcdir)/.valgrind.supp + --suppressions=$(abs_srcdir)/tools/.valgrind.supp valgrind: $(MAKE) check VG="$(LIBTOOL) --mode=execute $(VALGRIND)" @@ -603,16 +599,6 @@ qemucapabilitiestest_SOURCES = \ qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ $(qemu_LDADDS) $(LDADDS) -qemucapsprobe_SOURCES = \ - qemucapsprobe.c -qemucapsprobe_LDADD = \ - libqemutestdriver.la $(LDADDS) - -qemucapsprobemock_la_SOURCES = \ - qemucapsprobemock.c -qemucapsprobemock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) -qemucapsprobemock_la_LIBADD = $(MOCKLIBS_LIBS) - qemucommandutiltest_SOURCES = \ qemucommandutiltest.c \ testutils.c testutils.h \ diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index ac9ab6bfce..48363326f4 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -225,11 +225,11 @@ mymain(void) return EXIT_FAILURE; /* - * Run "tests/qemucapsprobe /path/to/qemu/binary >foo.replies" + * Run "tests/tools/qemucapsprobe /path/to/qemu/binary >foo.replies" * to generate updated or new *.replies data files. * * If you manually edit replies files you can run - * "tests/qemucapsfixreplies foo.replies" to fix the replies ids. + * "tests/tools/qemucapsfixreplies foo.replies" to fix the replies ids. * * Once a replies file has been generated and tweaked if necessary, * you can drop it into tests/qemucapabilitiesdata/ (with a sensible diff --git a/tests/testutils.c b/tests/testutils.c index 245b1832f6..080a1ccda2 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -533,7 +533,7 @@ virTestRewrapFile(const char *filename) return -1; } - if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0) + if (virAsprintf(&script, "%s/tools/test-wrap-argv.pl", abs_srcdir) < 0) goto cleanup; cmd = virCommandNewArgList(perl, script, "--in-place", filename, NULL); diff --git a/tests/.valgrind.supp b/tests/tools/.valgrind.supp similarity index 100% rename from tests/.valgrind.supp rename to tests/tools/.valgrind.supp diff --git a/tests/tools/Makefile.am b/tests/tools/Makefile.am new file mode 100644 index 0000000000..8a34b4a84f --- /dev/null +++ b/tests/tools/Makefile.am @@ -0,0 +1,85 @@ +# vim: filetype=automake + +AM_CPPFLAGS = \ + -I$(top_srcdir)/tests/ \ + -I$(top_builddir) -I$(top_srcdir) \ + -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \ + -I$(top_builddir)/include -I$(top_srcdir)/include \ + -I$(top_builddir)/src -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/util \ + -I$(top_srcdir)/src/conf \ + $(NULL) + +WARN_CFLAGS += $(RELAXED_FRAME_LIMIT_CFLAGS) + +AM_CFLAGS = \ + -Dabs_builddir="\"$(abs_builddir)\"" \ + -Dabs_top_builddir="\"$(abs_top_builddir)\"" \ + -Dabs_srcdir="\"$(abs_srcdir)\"" \ + -Dabs_top_srcdir="\"$(abs_top_srcdir)\"" \ + $(LIBXML_CFLAGS) \ + $(LIBNL_CFLAGS) \ + $(GNUTLS_CFLAGS) \ + $(SASL_CFLAGS) \ + $(SELINUX_CFLAGS) \ + $(APPARMOR_CFLAGS) \ + $(YAJL_CFLAGS) \ + $(XDR_CFLAGS) \ + $(WARN_CFLAGS) + +AM_LDFLAGS = \ + -export-dynamic + +MOCKLIBS_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation \ + $(MINGW_EXTRA_LDFLAGS) + +GNULIB_LIBS = \ + ../../gnulib/lib/libgnu.la + +MOCKLIBS_LIBS = \ + $(GNULIB_LIBS) \ + ../../src/libvirt.la + +PROBES_O = +if WITH_DTRACE_PROBES +PROBES_O += ../../src/libvirt_probes.lo +endif WITH_DTRACE_PROBES + +LDADDS = \ + $(NO_INDIRECT_LDFLAGS) \ + $(PROBES_O) \ + $(GNULIB_LIBS) \ + ../../src/libvirt.la + +test_helpers = +test_libraries = + +if WITH_QEMU +test_helpers += qemucapsprobe +test_libraries += qemucapsprobemock.la + +qemucapsprobe_SOURCES = \ + qemucapsprobe.c \ + ../testutils.h +qemucapsprobe_LDADD = \ + ../libqemutestdriver.la $(LDADDS) + +qemucapsprobemock_la_SOURCES = \ + qemucapsprobemock.c +qemucapsprobemock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +qemucapsprobemock_la_LIBADD = $(MOCKLIBS_LIBS) +endif WITH_QEMU + + +if WITH_TESTS +noinst_PROGRAMS = $(test_helpers) +noinst_LTLIBRARIES = $(test_libraries) +else ! WITH_TESTS +check_PROGRAMS = $(test_helpers) +check_LTLIBRARIES = $(test_libraries) +endif ! WITH_TESTS + +EXTRA_DIST = \ + .valgrind.supp \ + oomtrace.pl diff --git a/tests/check-file-access.pl b/tests/tools/check-file-access.pl similarity index 100% rename from tests/check-file-access.pl rename to tests/tools/check-file-access.pl diff --git a/tests/file_access_whitelist.txt b/tests/tools/file_access_whitelist.txt similarity index 100% rename from tests/file_access_whitelist.txt rename to tests/tools/file_access_whitelist.txt diff --git a/tests/group-qemu-caps.pl b/tests/tools/group-qemu-caps.pl similarity index 100% rename from tests/group-qemu-caps.pl rename to tests/tools/group-qemu-caps.pl diff --git a/tests/oomtrace.pl b/tests/tools/oomtrace.pl similarity index 100% rename from tests/oomtrace.pl rename to tests/tools/oomtrace.pl diff --git a/tests/qemucapsprobe.c b/tests/tools/qemucapsprobe.c similarity index 100% rename from tests/qemucapsprobe.c rename to tests/tools/qemucapsprobe.c diff --git a/tests/qemucapsprobemock.c b/tests/tools/qemucapsprobemock.c similarity index 100% rename from tests/qemucapsprobemock.c rename to tests/tools/qemucapsprobemock.c diff --git a/tests/test-wrap-argv.pl b/tests/tools/test-wrap-argv.pl similarity index 98% rename from tests/test-wrap-argv.pl rename to tests/tools/test-wrap-argv.pl index 7867e9d719..4a28ee9d46 100755 --- a/tests/test-wrap-argv.pl +++ b/tests/tools/test-wrap-argv.pl @@ -94,7 +94,7 @@ sub rewrap { close DIFF; print STDERR "Incorrect line wrapping in $file\n"; - print STDERR "Use test-wrap-argv.pl to wrap test data files\n"; + print STDERR "Use tests/tools/test-wrap-argv.pl to wrap test data files\n"; return -1; } } else { -- 2.21.0
On Mon, 2019-05-06 at 10:01 +0200, Michal Privoznik wrote: [...]
+++ b/configure.ac @@ -925,6 +925,7 @@ AC_CONFIG_FILES([\ include/libvirt/libvirt-common.h \ examples/Makefile \ tests/Makefile \ + tests/tools/Makefile \
Are we sure we want to have a tests/tools/Makefile.am rather than a tests/tools/Makefile.inc.am that we include from tests/Makefile.am here? We seem to be going in the former direction, though I'm not actually sure what the trade-offs are. CC'ing Dan who introduced the Makefile.inc.am files in the first place. [...]
+++ b/tests/Makefile.am EXTRA_DIST += \ - check-file-access.pl \ - file_access_whitelist.txt + tools/check-file-access.pl \ + tools/file_access_whitelist.txt
These should go into the new Makefile(.inc).am, just like you've done with .valgrind.supp and oomtrace.pl. Everything else looks sane from a very cursory looks, but I'd like to put it through its paces ('make distcheck', 'make rpm', ...) before ACKing it. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, May 06, 2019 at 01:58:49PM +0200, Andrea Bolognani wrote:
On Mon, 2019-05-06 at 10:01 +0200, Michal Privoznik wrote: [...]
+++ b/configure.ac @@ -925,6 +925,7 @@ AC_CONFIG_FILES([\ include/libvirt/libvirt-common.h \ examples/Makefile \ tests/Makefile \ + tests/tools/Makefile \
Are we sure we want to have a tests/tools/Makefile.am rather than a tests/tools/Makefile.inc.am that we include from tests/Makefile.am
including a makefile one level up helps paralellism, especialy if there's not much done in it. Jano
here? We seem to be going in the former direction, though I'm not actually sure what the trade-offs are. CC'ing Dan who introduced the Makefile.inc.am files in the first place.
[...]
+++ b/tests/Makefile.am EXTRA_DIST += \ - check-file-access.pl \ - file_access_whitelist.txt + tools/check-file-access.pl \ + tools/file_access_whitelist.txt
These should go into the new Makefile(.inc).am, just like you've done with .valgrind.supp and oomtrace.pl.
Everything else looks sane from a very cursory looks, but I'd like to put it through its paces ('make distcheck', 'make rpm', ...) before ACKing it.
-- Andrea Bolognani / Red Hat / Virtualization
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 5/6/19 1:58 PM, Andrea Bolognani wrote:
On Mon, 2019-05-06 at 10:01 +0200, Michal Privoznik wrote: [...]
+++ b/configure.ac @@ -925,6 +925,7 @@ AC_CONFIG_FILES([\ include/libvirt/libvirt-common.h \ examples/Makefile \ tests/Makefile \ + tests/tools/Makefile \
Are we sure we want to have a tests/tools/Makefile.am rather than a tests/tools/Makefile.inc.am that we include from tests/Makefile.am here? We seem to be going in the former direction, though I'm not actually sure what the trade-offs are. CC'ing Dan who introduced the Makefile.inc.am files in the first place.
I don't think that will work. The problem is that Makefile.inc.am would still be included from tests/Makefile.am and thus all the relative paths would be evaluated in relation to that. That is the reason why for instance src/qemu/Makefile.inc.am lists source files as qemu/qemu_*.c and all the libraries/binaries it builds are placed right under src/. What I'd like to have is for all these tools to live under separate directory - even the compiled binaries. And this is the point where my automake knowledge was weak enough so the only solution I was able to come up with was a separate Makefile. But I'm open to learn something new.
[...]
+++ b/tests/Makefile.am EXTRA_DIST += \ - check-file-access.pl \ - file_access_whitelist.txt + tools/check-file-access.pl \ + tools/file_access_whitelist.txt
These should go into the new Makefile(.inc).am, just like you've done with .valgrind.supp and oomtrace.pl.
Shoot. I wanted to move these. Consider fixed. Michal
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 +- Makefile.am | 2 +- cfg.mk | 4 +- configure.ac | 1 + tests/Makefile.am | 22 +----- tests/qemucapabilitiestest.c | 4 +- tests/testutils.c | 2 +- tests/{ => tools}/.valgrind.supp | 0 tests/tools/Makefile.am | 85 +++++++++++++++++++++ tests/{ => tools}/check-file-access.pl | 0 tests/{ => tools}/file_access_whitelist.txt | 0 tests/{ => tools}/group-qemu-caps.pl | 0 tests/{ => tools}/oomtrace.pl | 0 tests/{ => tools}/qemucapsprobe.c | 0 tests/{ => tools}/qemucapsprobemock.c | 0 tests/{ => tools}/test-wrap-argv.pl | 2 +- 16 files changed, 98 insertions(+), 26 deletions(-) rename tests/{ => tools}/.valgrind.supp (100%) create mode 100644 tests/tools/Makefile.am rename tests/{ => tools}/check-file-access.pl (100%) rename tests/{ => tools}/file_access_whitelist.txt (100%) rename tests/{ => tools}/group-qemu-caps.pl (100%) rename tests/{ => tools}/oomtrace.pl (100%) rename tests/{ => tools}/qemucapsprobe.c (100%) rename tests/{ => tools}/qemucapsprobemock.c (100%) rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to see the goal be to remove the top level "tests/" directory entirely. Instead move the tests under the src/ sub-directory that corresponds to the code being tested. eg we should have src/util/virhashtest.c alongside src/util/virhash.c So instead of one giant test dir we have everything distributed. That way if you are running "make" in the src/ directory, you don't need to change dir to do tests. It will also let us split up the ever growing set of makefile rules for tests. 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/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 +- Makefile.am | 2 +- cfg.mk | 4 +- configure.ac | 1 + tests/Makefile.am | 22 +----- tests/qemucapabilitiestest.c | 4 +- tests/testutils.c | 2 +- tests/{ => tools}/.valgrind.supp | 0 tests/tools/Makefile.am | 85 +++++++++++++++++++++ tests/{ => tools}/check-file-access.pl | 0 tests/{ => tools}/file_access_whitelist.txt | 0 tests/{ => tools}/group-qemu-caps.pl | 0 tests/{ => tools}/oomtrace.pl | 0 tests/{ => tools}/qemucapsprobe.c | 0 tests/{ => tools}/qemucapsprobemock.c | 0 tests/{ => tools}/test-wrap-argv.pl | 2 +- 16 files changed, 98 insertions(+), 26 deletions(-) rename tests/{ => tools}/.valgrind.supp (100%) create mode 100644 tests/tools/Makefile.am rename tests/{ => tools}/check-file-access.pl (100%) rename tests/{ => tools}/file_access_whitelist.txt (100%) rename tests/{ => tools}/group-qemu-caps.pl (100%) rename tests/{ => tools}/oomtrace.pl (100%) rename tests/{ => tools}/qemucapsprobe.c (100%) rename tests/{ => tools}/qemucapsprobemock.c (100%) rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds to the code being tested. eg we should have src/util/virhashtest.c alongside src/util/virhash.c So instead of one giant test dir we have everything distributed.
That way if you are running "make" in the src/ directory, you don't need to change dir to do tests. It will also let us split up the ever growing set of makefile rules for tests.
How is that? Won't we have to have the rules to build tests in src/Makefile.am? Personally, I find having code along side with the tests even more disaranged than what we have now. For some unit tests we can have strict naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests qemuxml2*test this won't work. BTW: I've found a way to have Makefile.inc.am and have it working so I'll post a v2 shortly. Michal
On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote:
On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 +- Makefile.am | 2 +- cfg.mk | 4 +- configure.ac | 1 + tests/Makefile.am | 22 +----- tests/qemucapabilitiestest.c | 4 +- tests/testutils.c | 2 +- tests/{ => tools}/.valgrind.supp | 0 tests/tools/Makefile.am | 85 +++++++++++++++++++++ tests/{ => tools}/check-file-access.pl | 0 tests/{ => tools}/file_access_whitelist.txt | 0 tests/{ => tools}/group-qemu-caps.pl | 0 tests/{ => tools}/oomtrace.pl | 0 tests/{ => tools}/qemucapsprobe.c | 0 tests/{ => tools}/qemucapsprobemock.c | 0 tests/{ => tools}/test-wrap-argv.pl | 2 +- 16 files changed, 98 insertions(+), 26 deletions(-) rename tests/{ => tools}/.valgrind.supp (100%) create mode 100644 tests/tools/Makefile.am rename tests/{ => tools}/check-file-access.pl (100%) rename tests/{ => tools}/file_access_whitelist.txt (100%) rename tests/{ => tools}/group-qemu-caps.pl (100%) rename tests/{ => tools}/oomtrace.pl (100%) rename tests/{ => tools}/qemucapsprobe.c (100%) rename tests/{ => tools}/qemucapsprobemock.c (100%) rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds to the code being tested. eg we should have src/util/virhashtest.c alongside src/util/virhash.c So instead of one giant test dir we have everything distributed.
That way if you are running "make" in the src/ directory, you don't need to change dir to do tests. It will also let us split up the ever growing set of makefile rules for tests.
How is that? Won't we have to have the rules to build tests in src/Makefile.am?
Most of the rules will be in the many src/*/Makefile.in.am files
Personally, I find having code along side with the tests even more disaranged than what we have now. For some unit tests we can have strict naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests qemuxml2*test this won't work.
We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c easily enough 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/19 1:15 PM, Daniel P. Berrangé wrote:
On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote:
On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 +- Makefile.am | 2 +- cfg.mk | 4 +- configure.ac | 1 + tests/Makefile.am | 22 +----- tests/qemucapabilitiestest.c | 4 +- tests/testutils.c | 2 +- tests/{ => tools}/.valgrind.supp | 0 tests/tools/Makefile.am | 85 +++++++++++++++++++++ tests/{ => tools}/check-file-access.pl | 0 tests/{ => tools}/file_access_whitelist.txt | 0 tests/{ => tools}/group-qemu-caps.pl | 0 tests/{ => tools}/oomtrace.pl | 0 tests/{ => tools}/qemucapsprobe.c | 0 tests/{ => tools}/qemucapsprobemock.c | 0 tests/{ => tools}/test-wrap-argv.pl | 2 +- 16 files changed, 98 insertions(+), 26 deletions(-) rename tests/{ => tools}/.valgrind.supp (100%) create mode 100644 tests/tools/Makefile.am rename tests/{ => tools}/check-file-access.pl (100%) rename tests/{ => tools}/file_access_whitelist.txt (100%) rename tests/{ => tools}/group-qemu-caps.pl (100%) rename tests/{ => tools}/oomtrace.pl (100%) rename tests/{ => tools}/qemucapsprobe.c (100%) rename tests/{ => tools}/qemucapsprobemock.c (100%) rename tests/{ => tools}/test-wrap-argv.pl (98%)
If we're going to re-arrange our tests, then I would really like to see the goal be to remove the top level "tests/" directory entirely.
Instead move the tests under the src/ sub-directory that corresponds to the code being tested. eg we should have src/util/virhashtest.c alongside src/util/virhash.c So instead of one giant test dir we have everything distributed.
That way if you are running "make" in the src/ directory, you don't need to change dir to do tests.
I don't know what your workflow is, but I have a terminal open at toplevel libvirt.git and then run vim over individual files, e.g. libvirt.git $ vim src/qemu/qemu_driver.c libvirt.git $ vim tools/virsh.c This way I can have tags in libvirt.git and can access them from whatever file I'm in. Running binaries is a bit more verbose: libvirt.git $ ./src/libvirtd --listen For tests I have a separate terminal open which entered tests: libvirt.git/tests $ So the benefin of having virhashtest next to virhash.c highly depends on one's workflow.
It will also let us split up the ever growing set of makefile rules for tests.
How is that? Won't we have to have the rules to build tests in src/Makefile.am?
Most of the rules will be in the many src/*/Makefile.in.am files
Yeah, while the overall number of rules won't change, they'll be spread accross multiple Makefiles.
Personally, I find having code along side with the tests even more disaranged than what we have now. For some unit tests we can have strict naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests qemuxml2*test this won't work.
We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c easily enough
Okay, let me send a v2 and see what others think. Michal
On Tue, May 07, 2019 at 12:15:54 +0100, Daniel Berrange wrote:
On Tue, May 07, 2019 at 12:49:18PM +0200, Michal Privoznik wrote:
On 5/7/19 11:13 AM, Daniel P. Berrangé wrote:
On Mon, May 06, 2019 at 10:01:37AM +0200, Michal Privoznik wrote:
There are some scripts/binaries that are not tests themselves but rather fulfill support purpose. Separate them from the rest of the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
[...]
Personally, I find having code along side with the tests even more disaranged than what we have now. For some unit tests we can have strict naming scheme: virhash.c -> virhashtest.c; but for some more advanced tests qemuxml2*test this won't work.
We don't need to rename everything - that could be src/qemu/qemuxml2argvtest.c easily enough
Ewwww that is super gross. I strongly prefer to keep the tests stashed into a directory (thus qemu/tests/qemuxml2argvtest.c) to avoid having a gazillion files in the main directory for the driver.
participants (5)
-
Andrea Bolognani -
Daniel P. Berrangé -
Ján Tomko -
Michal Privoznik -
Peter Krempa