From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14424 invoked by alias); 24 Aug 2012 16:53:11 -0000 Received: (qmail 14412 invoked by uid 22791); 24 Aug 2012 16:53:06 -0000 X-SWARE-Spam-Status: No, hits=-7.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Aug 2012 16:52:49 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7OGqm4s023785 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 24 Aug 2012 12:52:48 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q7OGqkps004562; Fri, 24 Aug 2012 12:52:47 -0400 Message-ID: <5037B15E.1020809@redhat.com> Date: Fri, 24 Aug 2012 16:53:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 MIME-Version: 1.0 To: Jan Kratochvil CC: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] Remove pass in skip_unwinder_tests References: <877gt1zbr5.fsf@fleche.redhat.com> <1345715389-20955-1-git-send-email-yao@codesourcery.com> <1345715389-20955-2-git-send-email-yao@codesourcery.com> <20120824133738.GB5219@host2.jankratochvil.net> <5037A087.1090703@redhat.com> <20120824161854.GA10953@host2.jankratochvil.net> In-Reply-To: <20120824161854.GA10953@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-08/txt/msg00744.txt.bz2 On 08/24/2012 05:18 PM, Jan Kratochvil wrote: > On Fri, 24 Aug 2012 17:40:55 +0200, Pedro Alves wrote: >> Nothing actually FAILed here. We have lots of precedent for "supports-foo" or >> "try this" style functions that issue no FAIL. > > There are cases which one can be sure they never can fail. But otherwise > I find it as a testsuitea bug. > > >> It is expected that >> some systems won't have the unwinder hooks. In the absurd, issuing a FAIL for >> these cases would be like issuing FAILs when tests are skipped because >> a [istarget "foobar-*-*"] returns false. > > If the system does not have unwinder hook it will XFAIL. XFAIL is not even > displayed on screen during interactive run. That's not what an XFAIL is for. XFAIL is when you do "print 2+2", you expect "4" to come out, but you know that on some broken systems instead "5" comes out, so you XFAIL on those systems, as in, to fix that _wrong result_, you need to fix something else, not GDB, but there _is_ something broken that should be fixed. The fact is that the check for the unwinder hook actually _succeeded_. It just happened that the result of the successful test was "no unwinder hook". I don't believe this support check should XFAIL on not-top-of-tree-glibc, just like they shouldn't trigger an XFAIL on Windows or Solaris (if it ever runs there). Rather, this is why DejaGNU has UNSUPPORTED -- it also leaves a line in the .sum file, and is also not displayed on screen during interactive run (IIUYC). If testing for the presence of the unwinder hooks fails (as in, we get some unexpected output, like an internal error), then we should be issuing in addition an UNRESOLVED instead of UNSUPPORTED (we don't do that presently): "Declares a test to have an unresolved outcome. unresolved writes in the log file a message beginning with `UNRESOLVED', appending the argument string. This usually means the test did not execute as expected, and a human being must go over results to determine if it passed or failed (and to improve the test case). " > If it even FAILs it is a GDB testsuite problem one should fix. > > In summary I find better: > > -PASS: gdb.java/jnpe.exp: check for unwinder hook > +FAIL: gdb.java/jnpe.exp: check for unwinder hook > > or: > > -XFAIL: gdb.java/jnpe.exp: check for unwinder hook > +FAIL: gdb.java/jnpe.exp: check for unwinder hook > > I find worse to get in diffs just: > > +FAIL: gdb.java/jnpe.exp: check for unwinder hook In isolation, it's mildly worse, because you can't tell whether it's a regression, or a new failure. But if you get that new FAIL, you'll actually see: FAIL: gdb.java/jnpe.exp: check for unwinder hook UNSUPPORTED: gdb.java/jnpe.exp: jnpe.exp could not find _Unwind_DebugHook" and several following -PASS: ... -PASS: ... -PASS: ... lines missing, corresponding to the tests that were skipped. Those FAIL+UNSUPPORTED lines look pretty clearly related to me. And IMO, ideally, we should see: FAIL: gdb.java/jnpe.exp: check for unwinder hook UNRESOLVED: gdb.java/jnpe.exp: looking for unwinder hooks failed > > Sure the testsuite has much more serious problems than this one, but when we > already discuss it it would be nice to get some consensus and write it to: > http://sourceware.org/gdb/wiki/GDBTestcaseCookbook > > > Thanks, > Jan > -- Pedro Alves