Message ID | 20220714215325.GA18923@ldh-imac.local |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F2FEE3858C20 for <patchwork@sourceware.org>; Thu, 14 Jul 2022 21:53:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F2FEE3858C20 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1657835639; bh=E/iN3BZNT1aNyuQsuL9+ussYtFnVTQAkqG4PGqmuyNo=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=AZTgO8CdheritCFvAJ8lZdb85WupmcbnN1kQmWlXh1kFXP7DCW0JhupSVCV5TQmYv i/bpbD9mebe6o30YJHfbL1yL8IikonYppfV4Qx4pFbsauOsHofV6BegnuKq5OJLQrz bC8X6NiiaCRIG1MdZf7CnBuNfPV2aUFzFYrsoC5s= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id 7EBA63858D37 for <gcc-patches@gcc.gnu.org>; Thu, 14 Jul 2022 21:53:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7EBA63858D37 Received: by mail-qk1-x72e.google.com with SMTP id b25so2305558qkl.1 for <gcc-patches@gcc.gnu.org>; Thu, 14 Jul 2022 14:53:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition; bh=E/iN3BZNT1aNyuQsuL9+ussYtFnVTQAkqG4PGqmuyNo=; b=7+YqUeh4Y/DAfFrKl50lxp4VWtwrZEGIw73RZLMw1eIO6S8+bTNoAvDGZql0N5FYXt v+HcQV+Kntl2PsmhKXq1uiFpeKSGvtun9dIP3+j99Ru2KZoRHHobZpbGrzHvj1vW9AAx +UdEM6hyO1CloWI/1duEN4Eb7nqQEFVtRjhrEw2wU0spLhQUV2GOKWFXbJneNNxJCj86 GTEyoIpGFY+4VrJHpleDaiDHgAjaGxgRpgzC+Z06HLcFXb/URq4wMIF7Gf9UTnmiUQAH Bvgs1ZgCoU1sThsfAn+80COnT3FoKfiR/E23SSUicicZkFrCjfdGFY6TTLYO9x9lVfHR TVjg== X-Gm-Message-State: AJIora/RvEZYL6qNV60vHZgage/W3/SLVFMqXE0gS8UD7/GBpdYFNVwG ebNqDxAW0Hw84ZY44bu7I1fsSRz2Pqc= X-Google-Smtp-Source: AGRyM1tXWeZl9apXxZ5MddgxrENvrpGZH9fKmUFBOY48CPwn2koYtTt8lz7uoUU+9rw5ZB8mcWnxCA== X-Received: by 2002:a05:620a:44c9:b0:6b2:1f49:f0f5 with SMTP id y9-20020a05620a44c900b006b21f49f0f5mr7486961qkp.327.1657835608492; Thu, 14 Jul 2022 14:53:28 -0700 (PDT) Received: from ldh-imac.local (96-67-140-173-static.hfc.comcastbusiness.net. [96.67.140.173]) by smtp.gmail.com with ESMTPSA id k10-20020ac8474a000000b00304bc2acc25sm2384383qtp.6.2022.07.14.14.53.27 for <gcc-patches@gcc.gnu.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jul 2022 14:53:27 -0700 (PDT) Date: Thu, 14 Jul 2022 17:53:25 -0400 To: gcc-patches@gcc.gnu.org Subject: [PATCH] libphobos: Fix instability in the parallelized testsuite Message-ID: <20220714215325.GA18923@ldh-imac.local> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="jRHKVT23PllUwdXP" Content-Disposition: inline X-Spam-Status: No, score=-3039.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Lewis Hyatt via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Lewis Hyatt <lhyatt@gmail.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
libphobos: Fix instability in the parallelized testsuite
|
|
Commit Message
Lewis Hyatt
July 14, 2022, 9:53 p.m. UTC
Hello- I get a different number of test results from libphobos.unittest/unittest.exp, depending on server load. I believe it's because this testsuite doesn't check runtest_file_p: $ make -j 1 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' # of expected passes 10 $ make -j 2 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' # of expected passes 10 # of expected passes 10 $ make -j 4 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' # of expected passes 10 # of expected passes 10 # of expected passes 10 # of expected passes 10 When running in parallel along with other tests, even at a fixed argument for -j, the number of tests that actually execute will depend on how many of the parallel sub-makes happened to start prior to the first one finishing, hence it changes from run to run. The attached patch fixes it for me, if it looks OK? Thanks, this would remove some noise from before/after test comparisons. -Lewis libphobos: Fix instability in the parallelized testsuite libphobos.unittest/unittest.exp calls bare dg-test rather than dg-runtest, and so it should call runtest_file_p to determine whether to run each test or not. Without that call, the tests run too many times in parallel mode (they will run as many times, as the argument to make -j). libphobos/ChangeLog: * testsuite/libphobos.unittest/unittest.exp: Call runtest_file_p prior to running each test.
Comments
Excerpts from Lewis Hyatt via Gcc-patches's message of Juli 14, 2022 11:53 pm: > Hello- > > I get a different number of test results from libphobos.unittest/unittest.exp, > depending on server load. I believe it's because this testsuite doesn't check > runtest_file_p: > > $ make -j 1 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' > # of expected passes 10 > > $ make -j 2 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' > # of expected passes 10 > # of expected passes 10 > > $ make -j 4 RUNTESTFLAGS='unittest.exp' check-target-libphobos | grep '^#' > # of expected passes 10 > # of expected passes 10 > # of expected passes 10 > # of expected passes 10 > > When running in parallel along with other tests, even at a fixed argument > for -j, the number of tests that actually execute will depend on how many of the > parallel sub-makes happened to start prior to the first one finishing, hence > it changes from run to run. > > The attached patch fixes it for me, if it looks OK? Thanks, this would remove > some noise from before/after test comparisons. > > -Lewis > libphobos: Fix instability in the parallelized testsuite > > libphobos.unittest/unittest.exp calls bare dg-test rather than dg-runtest, and > so it should call runtest_file_p to determine whether to run each test or > not. Without that call, the tests run too many times in parallel mode (they will > run as many times, as the argument to make -j). Hi Lewis, Thanks! Good spot. I think it should be calling dg-runtest however, same as what libphobos.cycles/cycles.exp is doing. Could also fix the test name so each one is unique, just to hit two birds in one - something like the following would suffice (haven't had time to check). Kind Regards, Iain. --- --- a/libphobos/testsuite/libphobos.unittest/unittest.exp +++ b/libphobos/testsuite/libphobos.unittest/unittest.exp @@ -42,8 +42,10 @@ foreach unit_test $unit_test_list { set expected_fail [lindex $unit_test 1] foreach test $tests { - set shouldfail $expected_fail - dg-test $test "" $test_flags + set libphobos_test_name "[dg-trim-dirname $srcdir $test] $test_flags" + set shouldfail $expected_fail + dg-runtest $test "" $test_flags + set libphobos_test_name "" } set shouldfail 0
> Hi Lewis, > > Thanks! Good spot. I think it should be calling dg-runtest however, > same as what libphobos.cycles/cycles.exp is doing. Could also fix the > test name so each one is unique, just to hit two birds in one - > something like the following would suffice (haven't had time to check). > > Kind Regards, > Iain. > > --- > > --- a/libphobos/testsuite/libphobos.unittest/unittest.exp > +++ b/libphobos/testsuite/libphobos.unittest/unittest.exp > @@ -42,8 +42,10 @@ foreach unit_test $unit_test_list { > set expected_fail [lindex $unit_test 1] > > foreach test $tests { > - set shouldfail $expected_fail > - dg-test $test "" $test_flags > + set libphobos_test_name "[dg-trim-dirname $srcdir $test] $test_flags" > + set shouldfail $expected_fail > + dg-runtest $test "" $test_flags > + set libphobos_test_name "" > } > > set shouldfail 0 > Thanks for the followup. I tested and can confirm your version works fine: PASS: libphobos.unittest/customhandler.d -fversion=FailNoPrintout (test for excess errors) PASS: libphobos.unittest/customhandler.d -fversion=FailNoPrintout execution test PASS: libphobos.unittest/customhandler.d -fversion=FailedTests (test for excess errors) PASS: libphobos.unittest/customhandler.d -fversion=FailedTests execution test PASS: libphobos.unittest/customhandler.d -fversion=GoodTests (test for excess errors) PASS: libphobos.unittest/customhandler.d -fversion=GoodTests execution test PASS: libphobos.unittest/customhandler.d -fversion=NoTests (test for excess errors) PASS: libphobos.unittest/customhandler.d -fversion=NoTests execution test PASS: libphobos.unittest/customhandler.d -fversion=PassNoPrintout (test for excess errors) PASS: libphobos.unittest/customhandler.d -fversion=PassNoPrintout execution test Let me know if you want me to do anything from there please? By the way, there are a few other tests that cause some minor glitches with comparing results: libphobos.sum:PASS: libphobos.shared/link.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared lib.so -shared-libphobos (test for excess errors) libphobos.sum:PASS: libphobos.shared/link.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared lib.so -shared-libphobos execution test libphobos.sum:PASS: libphobos.shared/link_linkdep.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared liblinkdep.so lib.so -shared-libphobos (test for excess errors) libphobos.sum:PASS: libphobos.shared/link_linkdep.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared liblinkdep.so lib.so -shared-libphobos execution test libphobos.sum:PASS: libphobos.shared/link_loaddep.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared libloaddep.so -shared-libphobos (test for excess errors) libphobos.sum:PASS: libphobos.shared/link_loaddep.d -I/home/lewis/gccdev/base/src/libphobos/testsuite/libphobos.shared libloaddep.so -shared-libphobos execution test The problem here is that the absolute path to the test dir ends up in the results summary, since it appears in the options string that is part of the test name. It's not so hard to work around when doing the comparisons, but it seems to be the only case where this happens in the whole testsuite, other than one other similar case from libgo. Is there a standard way to handle it I take it? Thanks... -Lewis
diff --git a/libphobos/testsuite/libphobos.unittest/unittest.exp b/libphobos/testsuite/libphobos.unittest/unittest.exp index 2a019caca8c..175decdc333 100644 --- a/libphobos/testsuite/libphobos.unittest/unittest.exp +++ b/libphobos/testsuite/libphobos.unittest/unittest.exp @@ -42,6 +42,9 @@ foreach unit_test $unit_test_list { set expected_fail [lindex $unit_test 1] foreach test $tests { + if {![runtest_file_p $runtests $test]} { + continue + } set shouldfail $expected_fail dg-test $test "" $test_flags }