.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%)
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.