This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC/PATCH] Don't disable selftests in a non-development build


Thanks for the comments.

On Tuesday, August 14 2018, Pedro Alves wrote:

> On 08/14/2018 06:42 AM, Sergio Durigan Junior wrote:
>> This is an idea I mentioned a few days ago in our IRC channel, and
>> Simon said he'd be open to it, so I'm send this RFC/PATCH to see what
>> others think.
>> 
>> Due to the many racy testcases and random failures we see when running
>> the GDB testsuite, it is unfortunately not possible to perform a full
>> test when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.  However, we currently (for some
>> reason that is not clear by reading the archives, but see more below)
>> disable selftests on non-development builds.  Therefore, this patch
>> aims to leave them enabled all the time, for everyone (including the
>> end users).
>> 
>> I understand that disabling the selftests in a non-development build
>> brings a few advantages.  For example, we ship less code (even though
>> the final difference may be small enough that it doesn't really have
>> any real benefit).  
>
> At least for now.  It's plausible that the tests grow substantially
> over time.

I agree.  I think we could always revisit the decision if the unittests
become too big.

> Also remember that we include unit tests in gdbserver
> too.  From this angle, I'd think a configure switch would be more
> appropriate.

I thought about adding a configure switch to control whether
GDB_SELF_TEST is defined or not.  In the end, it seemed to me that
always enabling the tests made more sense.  But I can certainly add a
configure switch, both on GDB and gdbserver, of course.

>> Another thing to consider is that we disable
>> portions of the codebase that could have latent bugs, making GDB a bit
>> safer.  I don't really have an argument to compete with this;
>> nonetheless, I think the benefit of having "batteries included" when
>> it comes to testing is a huge thing.  With our selftests, the user
>> doesn't need to install any external
>> programs/libraries (dejagnu/tcl/etc.), and they're stable enough that
>> we know that they don't suffer from the racyness present in our
>> current testsuite.
>> 
>> Anyway, here's my take at the problem.  The patches are mostly
>> mechanical (getting rid of the "GDB_SELF_TEST" define, which becomes
>> useless now that selftests are always present); the ones that aren't
>> deal with the configure/Makefile machinery to always compile the
>> selftests source files.
>> 
>> I've tested this patch on BuildBot, without regressions.  I'm taking
>> the liberty to Cc Joel, who is our release manager and might have
>> opinions about this.
>
> As I mentioned in an internal discussion, I suspect that this must
> have been discussed while the original unit testing patch was added,
> since, well, we sprinkle #ifdef GDB_SELF_TEST throughout, so it
> wasn't done by chance.

I thought so too, and I looked in the archives and tried to search this
discussion, but nothing came up.  The first message I found was:

  https://sourceware.org/ml/gdb-patches/2016-04/msg00569.html

and it already proposed that the feature should be disabled in
non-development builds.  I couldn't find any discussions specific to
this, so I think people just accepted this was the better way to do it.

> One point I'd like to raise is that the fact that GDB's unit tests are built
> and bolted into GDB-the-binary is more of a technical consequence than
> a design goal.  It just so happens that it was much easier to build unit
> tests this way.  But this is not the only way.  In fact, for fairly isolated
> generic utilities, the corresponding unit tests are pretty self
> contained -- I'm talking about the src/gdb/unittests/ tests -- 
> I've considered creating a small unit tests driver binary for them, independent
> of GDB.  Imagine we get to a point where we want to
> share the code that is tested by those tests with another project like
> e.g., GCC (e.g., by moving the code to a new libiberty-like library or something
> like that).  It would be totally doable to build a separate small binary other
> than build/gdb/gdb whose only purpose would be to drive the unit tests (like,
> e.g., build/gdb/unittests-driver).  If we do that, then gdb's own built-in
> unit tests start disappearing...
>
> Last year, Yao was pondering/investigating using some off-the-shelf
> unit tests framework, like Google Test.  The result would be same.
>
> I would hope that going in the direction of having unit tests
> in released binaries would not prevent exploration or design
> changes in the unit testing space.

I appreciate the explanation, but it seems to me that this is another
problem.  If my proposal is accepted, there will be nothing preventing
us to go forward with this idea (which I think is a very good idea, of
course).

> Of course, if we had a separate unit tests driver, then there would
> be nothing preventing building it in release mode, since by design
> it would not affect gdb's binary.
>
> Another approach to addressing this issue here:
>
>> Due to the many racy testcases and random failures we see when running
>> the GDB testsuite, it is unfortunately not possible to perform a full
>> test when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.  However, we currently (for some
>> reason that is not clear by reading the archives, but see more below)
>> disable selftests on non-development builds.  Therefore, this patch
>> aims to leave them enabled all the time, for everyone (including the
>> end users).
>
> ... is to come up with some small set of stable testcases that
> are considered the "smoke tests" and add a mechanism to run them.
> Could be just a list of testcases in a file that is passed to
>   make check TESTS="list of basic tests here"
> or some make target like "make check-smoke", or something
> else even.

Sure, that works too.  This is somewhat related to your idea of marking
the racy tests in our testsuite (a la "setup_kfail"), which I think is
also good, and it's on my long-term TODO list.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]