GNOME Bugzilla – Bug 538226
Build tests only on make check
Last modified: 2009-01-19 12:06:13 UTC
Hi, It would be nice if gtkmm avoided building his tests by default, it’s just seconds, or minutes, but this is useless for end users since they are not installed. reference: https://bugs.gentoo.org/show_bug.cgi?sourceid=Mozilla-search&id=225957
I can't see any possible advantage to not building the tests. They test that basic compilation and linking with the API is working. That's useful.
Created attachment 114380 [details] [review] patch to only build tests on "make check" + libtool 2.2 support Disclaimer: I didn't have gtk+ 2.13 so this patch is untested (yet). With this patch, tests are only built on "make check". The actual tests could also be modified to return an error code and then make check could run those tests. (See http://www.gnu.org/software/automake/manual/automake.html#Tests ) The patch to configure.in is to add support for libtool 2.2 which no longer adds the implicit check for a C++ compiler. It's backwards compatible with libtool 1.5, I strongly suggest adding at least that part even if you think the rest of the patch is useless :) Thanks
gtkmm's configure.in already has a call to AC_PROG_CXX, so this additional call is wrong. I'm not sure if tests should only be built on make check.
(In reply to comment #3) > gtkmm's configure.in already has a call to AC_PROG_CXX, so this additional call > is wrong. Yes, but it's done too late. AC_PROG_LIBTOOL used to call AC_PROG_CXX directly but that "feature" has been removed in 2.2 Basically if you want to call AC_LANG_PUSH(C++) (the first one is on line 132), AC_PROG_CXX needs to be called before. Then of course, the original call to AC_PROG_CXX on line 244 (which I didn't spot) should be removed. > I'm not sure if tests should only be built on make check. I'm not sure either :) Would a configure switch be OK with you? The default could then to still build tests as they are now.
Created attachment 115814 [details] [review] patch for libtool 2.2 support Here's a patch for libtool 2.2 support
This seems to be because the PageList has the operator== and operator != as separate functions rather than member functions: /** @relates Gtk::Notebook_Helpers::PageIterator */ inline bool operator==(const PageIterator& lhs, const PageIterator& rhs) { return lhs.equal(rhs); } /** @relates Gtk::Notebook_Helpers::PageIterator */ inline bool operator!=(const PageIterator& lhs, const PageIterator& rhs) { return !lhs.equal(rhs); } I forget why that's generally considered a good thing. I suppose we could make the member operator functions separate functions in glibmm/containers.h and make the code expect that. Thoughts?
Sorry, wrong bug.
(In reply to comment #5) > Created an attachment (id=115814) [edit] > patch for libtool 2.2 support > > Here's a patch for libtool 2.2 support Committed. Many thanks. Please patch the ChangeLog in future, and please don't put unrelated patches in existing bug reports.
> > I'm not sure if tests should only be built on make check. > > I'm not sure either :) Well someone needs to find out. > Would a configure switch be OK with you? No, it would be best to do the right thing and avoid unnecessary complication.
(In reply to comment #8) > Committed. Many thanks. Please patch the ChangeLog in future, and please don't > put unrelated patches in existing bug reports. Thanks and it's noted, I'll open another bug next time this happens. (In reply to comment #9) > > I'm not sure either :) > > Well someone needs to find out. > > > Would a configure switch be OK with you? > > No, it would be best to do the right thing and avoid unnecessary complication. Well, it's pretty much your call as a maintainer. Automake allows you to hook things to "make check" : 1) Build some extra programs that won't be built with the standard "make all" 2) Actually run some tests and report success/failure (kind of like unit testing) Both options can be done without doing the other. But if you go with #1, users will have to run "make check" to build those tests, which might not be what you want. The other option (which a lot of apps actually do) is to conditionally build tests using a simple AM_CONDITIONAL. By default, tests might get built like they are today. But it still allows knowledgeable users to turn off building tests with a simple configure switch. The reason why we ask for this is that Gentoo's package manager has support for package tests, and if a user reports a failure, we will ask him to rebuild the package with the tests. So a configure switch would allow for faster build time for most users but still allow tests to be activated. Gtkmm developers will probably still build the tests anyway. Sample patch to be posted in a minute.
Created attachment 115820 [details] [review] patch to add a configure switch to disable tests build Here's a patch for the other option (with a patched ChangeLog this time) FYI, glib/gio does uses the automake support since they have a test suite that can run in batch mode to report success/failure. gtksourceview on the other hand does exactly like this patch because the test programs are more like simple demos that have to be ran manually to see that the behavior is correct.
Again, I do not want a configure option for this. That is silly. Regarding, glib/gio and gtksourceview, yes they use make check because they want to check the result of their tests. But we don't do that (yet) and this is about building, not running.
glib/gio uses "make check" to build and run tests. gtksourceview does not, they have a configure switch like in attachment #115820 [details]. I think there's a small misunderstanding about the first patch I posted. "make check" does 2 things : 1) *builds* tests programs that are marked with "check_PROGRAMS" 2) *runs* tests programs/scripts that are marked with "TESTS" The original patch only does step #1, so "make check" will only just build tests. In the future, you can convert your tests to behave like the glib/gio ones and use the TESTS variable of Makefile.am, and "make check" will run them. Sorry if my first explanations were not concise enough. I hope these are better. Thanks
Created attachment 115826 [details] [review] patch to only build tests on "make check" Same patch as attachment 114380 [details] [review] without the libtool stuff and with a proper ChangeLog entry
I have no problem with make check doing something new, but I don't want to disable building of the tests during the normal build. A failure is more likely to be caught if they are always built. Please reopen this if you really need this change for some reason.