From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120275 invoked by alias); 14 Aug 2018 18:08:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 120265 invoked by uid 89); 14 Aug 2018 18:08:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=raise, Another, angle X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Aug 2018 18:08:08 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2FF0B40704A2; Tue, 14 Aug 2018 18:08:07 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id A1A48112D163; Tue, 14 Aug 2018 18:08:04 +0000 (UTC) Subject: Re: [RFC/PATCH] Don't disable selftests in a non-development build To: Sergio Durigan Junior , GDB Patches References: <20180814054221.13061-1-sergiodj@redhat.com> Cc: Joel Brobecker From: Pedro Alves Message-ID: <367277ad-d735-0854-7aca-1df9a1927115@redhat.com> Date: Tue, 14 Aug 2018 18:08:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180814054221.13061-1-sergiodj@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-08/txt/msg00355.txt.bz2 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. Also remember that we include unit tests in gdbserver too. From this angle, I'd think a configure switch would be more appropriate. > 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. 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. 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. Thanks, Pedro Alves