Usability problems with '--enable-tests' (configure.ac) #3708
Comments
I think, tests/README should be added to the EXTRA_DIST in tests/Makefile.am, otherwise it will not be in distributed tarboll. |
|
Replying to andrew_b:
|
Branch: 3708_tests_usability |
Awesome, thank you! Love it... |
Hmmm, Travis doesn't like it:
https://travis-ci.org/MidnightCommander/mc/builds/172262523
It seems that make dist is botched, need to see why; that's why I'm making so much fuss about CI: problem caught early before causing any damage. |
Replying to zaytsev:
So it turns out that makefiles can't be created conditionally, because they always have to be there to tell make how to create the distribution.
This means that --disable-tests would have triggered the problem too.
I'm attaching v2 of the patch. It removes the conditional from configure.ac.
(I haven't yet had the time to test this patch; it's quite tortuous to run 'make whatever' on my system (which is why I refrained from trying out 'make dist' earlier.)) |
|
We've talked about that in the past and the obvious solution was rejected, but here are a few more (hopefully acceptable) suggestions:
|
Ok, I've verified that 'make dist' works. Which is not surprising now as the patch no longer contains fancy stuff: it contributes just a "Unit tests: X" summary line and a README. Well, it's better than the current situation.
(@yury: Thanks. I'll have a look at ccache soon (heard of it before but I guess I was sceptical). Then tcc. (I've already tried memory filesystem and clang, but they weren't much an improvement.)) |
Yes, for a project like mc, tmpfs + clang won't bring much, but even with large C++ projects, clang is not that much faster than gcc these days. I would expect tmpfs + tcc to really rock though, if it works at all.
ccache is very nice as long as you don't often change stuff that ripples all over the code base, such that effectively you'll have to recompile everything anyways, and thus it will be of little help; it all depends on your workflow. In the case of mc, the config.h / version.h thing will, unfortunately, effectively kill the caching upon new commits, but since my compile times are very low, I never cared enough to disentangle this stuff and put version strings in a separate object file. |
|
Branch: 3708_tests_usability
Rebased the new patch against latest master, Travis build seems to pass now. |
Folks,
At some moment in the past two months I got a jolt of Psychic Insight. It lasted for only a second but as it was flowing through me it occured to me that the intention of the original author of mc-tests.m4 was for '--enable-tests' to effectively be the default.
This makes a thing or two seem logical now.
So, seen in this new light, the patch needs work. I'll do this sometime soon. |
Replying to mooffie:
|
I'm not quite sure this was Slava's intention, otherwise he would have added action-if-given / action-if-not-given as he did to many other similar options, but I would definitively approve of a plan to enable them by default if check is found. Whatever makes tests easier to build and run is a good thing (tm) |
Ok, here's the revised patch. Tests are now enabled by default. No reason not to. |
|
Branch: 3708_tests_usability
Third attempt ;-) |
|
Merged to master: [67b3d64]
Sorry, Andrew, I totally forgot about this, thanks for the reminder. |
|
Important
This issue was migrated from Trac:
mooffie(@mooffie)To enable tests, one has to pass --enable-tests to configure. But there are a few usability problems in this mechanism:
When the programmer doesn't pass --enable-tests:
In this case, the output on screen and the generated files falsely give the impression tests were enabled:
(1) The message "checking for CHECK..." appears.
(2) The makefiles under the 'tests' tree are generated, even though configure.ac has a conditional that's supposed to prevent this (i.e., if test x$enable_tests != xno).
When the programmer does pass --enable-tests:
(3) She doesn't have an easy way to know if tests were indeed successfully enabled. That's because error messages are very hard to spot, and because the test makefiles, because of (2), are created unconditionally and therefore can't serve as indicator.
Additionally:
(4) Instructions on how to enable the tests should appear where they're expected.
Are these usability problems grave?
Yes! People are #comment:49 wasting time (#3449) trying to figure out these issues by themselves.
The attached patch fixes these problems:
Note
Original attachments:
mooffie(@mooffie) onNov 1, 2016 at 8:24 UTCmooffie(@mooffie) onNov 1, 2016 at 15:26 UTCmooffie(@mooffie) onDec 27, 2016 at 0:44 UTCThe text was updated successfully, but these errors were encountered: