| Message ID | 1456492150-5299-1-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 26 February 2016 at 13:09, Daniel P. Berrange <berrange@redhat.com> wrote: > It is easy to miss that a change broke compilation of a unit > test unless you always remember to run 'make check-unit' for > every patch. Hook build of unit test programs into the 'all' > target so that they are built by default. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > tests/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > Note sure if this is considered to be desirable in general > or not. We did this in libvirt a while back because it > stopped people submitting patches which broke build of > the unit tests. On the downside it means more code is > built by default. Perhaps it could be on by default but > with a --disable-tests to let people optionally disble > it ? Without this, if people want to check unit test > they'd curently do 'make && make check-unit' which takes > longer because that actually runs the unit tests as well > as bulding them - there's no target just to build them. I think we should be encouraging people to run the tests, not just build them (and indeed to run 'make check' rather than just 'make check-unit'). Conversely, if you're not going to run the test suite there's not much point building it. > NB, tests/qemu-iotests/socket_scm_helper is already set > to build by default, so we're a bit inconsistent about > rules for building test suite artifacts currently. I think that's a bug which we should fix. It should only be built if you try to run the test or tests that use it. thanks -- PMM
diff --git a/tests/Makefile b/tests/Makefile index cb72eb4..7fadd9e 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -638,6 +638,8 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \ done,) +all: $(check-unit-y) + .PHONY: $(patsubst %, check-%, $(check-unit-y)) $(patsubst %, check-%, $(check-unit-y)): check-%: % $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
It is easy to miss that a change broke compilation of a unit test unless you always remember to run 'make check-unit' for every patch. Hook build of unit test programs into the 'all' target so that they are built by default. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/Makefile | 2 ++ 1 file changed, 2 insertions(+) Note sure if this is considered to be desirable in general or not. We did this in libvirt a while back because it stopped people submitting patches which broke build of the unit tests. On the downside it means more code is built by default. Perhaps it could be on by default but with a --disable-tests to let people optionally disble it ? Without this, if people want to check unit test they'd curently do 'make && make check-unit' which takes longer because that actually runs the unit tests as well as bulding them - there's no target just to build them. NB, tests/qemu-iotests/socket_scm_helper is already set to build by default, so we're a bit inconsistent about rules for building test suite artifacts currently.