Re: [PATCH] Only build tests when unit tests are enabled.
On Wed, 2011-03-23 at 09:14 +1000, Peter Hutterer wrote: On Tue, Mar 22, 2011 at 11:26:24AM -0400, Gaetan Nadon wrote: On Tue, 2011-03-22 at 11:56 +1000, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- I'll squash this in with the other patch, no need to have two separate ones. test/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Now that I have cleared some of my misunderstandings, there is little benefits in using XORG_ENABLE_UNIT_TESTS for such a simple case, unless you foresee a more complex situation in the short term. But nothing wrong in using it either. It helps establishing a pattern. Using the check_PROGRAMS pattern would most likely ease maintenance and reduce the risk of errors as it would work like most/all other modules. If there is a need to build the test program in the regular build, it could be moved to the eventcomm dir. The test dir would simply make use of it. There is no obligation for programs to be built in the test dir. answering the other email in this one too: check_PROGRAMS are the list of binaries built on 'make check'. TESTS are the list of binaries executed on 'make check' this is what we currently use in the server. the problem with it is that if the checks take a while, people are less inclined to run make check. the result of this is that tests may have build errors until someone who actually runs make check notices. the reason why i chose noinst_PROGRAMS here is so that the tests built when running 'make', but _executed_ when running 'make check'. this avoids the aforementioned build errors in the tests. so yes, noinst_PROGRAMS was quite intentional here, and I'm planning to send out a similar patch for the server. as for the XORG_ENABLE_UNIT_TESTS, I don't mind having it there, it has no maintainance requirement and if someone absolutely wants to disable the unit tests, so be it. this makes more sense in a long-term approach if tests use libraries that may not be available (such as glib) or if the tests require a certain setup that may not be available (e.g. running X server). Cheers, Peter diff --git a/test/Makefile.am b/test/Makefile.am index 16502ee..0b45a2d 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,3 +1,4 @@ +if ENABLE_UNIT_TESTS AM_CPPFLAGS = -I$(top_srcdir)/src AM_CFLAGS = $(XORG_CFLAGS) $(CWARNFLAGS) fake_syms = fake-symbols.c fake-symbols.h @@ -10,6 +11,5 @@ eventcomm_test_SOURCES = eventcomm-test.c\ $(fake_syms) endif -if ENABLE_UNIT_TESTS TESTS = $(noinst_PROGRAMS) endif Reviewed-by: Gaetan Nadon mems...@videotron.ca signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Only build tests when unit tests are enabled.
On Mon, Mar 21, 2011 at 6:56 PM, Peter Hutterer peter.hutte...@who-t.net wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- I'll squash this in with the other patch, no need to have two separate ones. test/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/test/Makefile.am b/test/Makefile.am index 16502ee..0b45a2d 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,3 +1,4 @@ +if ENABLE_UNIT_TESTS AM_CPPFLAGS = -I$(top_srcdir)/src AM_CFLAGS = $(XORG_CFLAGS) $(CWARNFLAGS) fake_syms = fake-symbols.c fake-symbols.h @@ -10,6 +11,5 @@ eventcomm_test_SOURCES = eventcomm-test.c\ $(fake_syms) endif -if ENABLE_UNIT_TESTS TESTS = $(noinst_PROGRAMS) endif Not that it matters too much, but you just need to wrap the *_PROGRAMS declaration to get automake to not output the toplevel rules. I do notice here the TESTS = $(noinst_PROGRAMS) line. If check_PROGRAMS is used instead, then the programs are only built on make check. Not sure that's the intention or not. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Only build tests when unit tests are enabled.
On Tue, 2011-03-22 at 05:39 -0700, Dan Nicholson wrote: Not that it matters too much, but you just need to wrap the *_PROGRAMS declaration to get automake to not output the toplevel rules. I do notice here the TESTS = $(noinst_PROGRAMS) line. If check_PROGRAMS is used instead, then the programs are only built on make check. Not sure that's the intention or not. I had not paid too much attention too that, I used libxkbcommon as a model. So it is the check_PROGRAMS that delays building until make check. I think Peter wants it to be build all the time. Not sure why. If it is to ensure the test programs compiles, this could be addressed in the build where it can invoke 'make clean install check'. signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Only build tests when unit tests are enabled.
On Tue, Mar 22, 2011 at 11:26:24AM -0400, Gaetan Nadon wrote: On Tue, 2011-03-22 at 11:56 +1000, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- I'll squash this in with the other patch, no need to have two separate ones. test/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Now that I have cleared some of my misunderstandings, there is little benefits in using XORG_ENABLE_UNIT_TESTS for such a simple case, unless you foresee a more complex situation in the short term. But nothing wrong in using it either. It helps establishing a pattern. Using the check_PROGRAMS pattern would most likely ease maintenance and reduce the risk of errors as it would work like most/all other modules. If there is a need to build the test program in the regular build, it could be moved to the eventcomm dir. The test dir would simply make use of it. There is no obligation for programs to be built in the test dir. answering the other email in this one too: check_PROGRAMS are the list of binaries built on 'make check'. TESTS are the list of binaries executed on 'make check' this is what we currently use in the server. the problem with it is that if the checks take a while, people are less inclined to run make check. the result of this is that tests may have build errors until someone who actually runs make check notices. the reason why i chose noinst_PROGRAMS here is so that the tests built when running 'make', but _executed_ when running 'make check'. this avoids the aforementioned build errors in the tests. so yes, noinst_PROGRAMS was quite intentional here, and I'm planning to send out a similar patch for the server. as for the XORG_ENABLE_UNIT_TESTS, I don't mind having it there, it has no maintainance requirement and if someone absolutely wants to disable the unit tests, so be it. this makes more sense in a long-term approach if tests use libraries that may not be available (such as glib) or if the tests require a certain setup that may not be available (e.g. running X server). Cheers, Peter diff --git a/test/Makefile.am b/test/Makefile.am index 16502ee..0b45a2d 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,3 +1,4 @@ +if ENABLE_UNIT_TESTS AM_CPPFLAGS = -I$(top_srcdir)/src AM_CFLAGS = $(XORG_CFLAGS) $(CWARNFLAGS) fake_syms = fake-symbols.c fake-symbols.h @@ -10,6 +11,5 @@ eventcomm_test_SOURCES = eventcomm-test.c\ $(fake_syms) endif -if ENABLE_UNIT_TESTS TESTS = $(noinst_PROGRAMS) endif ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
