From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mYDB4-0007xQ-Ks for mharc-grub-devel@gnu.org; Wed, 06 Oct 2021 16:06:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:55442) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mYDB2-0007tS-Ry for grub-devel@gnu.org; Wed, 06 Oct 2021 16:06:08 -0400 Received: from mail-qt1-x836.google.com ([2607:f8b0:4864:20::836]:41491) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mYDAz-00020Y-2S for grub-devel@gnu.org; Wed, 06 Oct 2021 16:06:08 -0400 Received: by mail-qt1-x836.google.com with SMTP id t2so3922765qtx.8 for ; Wed, 06 Oct 2021 13:06:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=Dn+pIdvaPMbIiZgqoE1HBv0YN1uXjip4OsCSsvpfJV0=; b=U18P/hs/w6n2IRU+Rgp9LGHEVRxu8Nvr12ogArlptXcY8pViqEkiIVmFGRi7081w9g c82VSPMwbanJzOQJ05A7M72/qNpiEoMGRDxjwrHoDCmLtpjbBXNDvJLElT5LXQ1GMzup mBDvyEgEhvbgXr4zdRKckzIxg9DHyMo8iD+0MbSgiPrlhHaF6nm81Vxbs63iwuUAjrFG ziL+E143vZxDARvdnAdV0GPbiThLODLrmgacriV2F6ppdARMUQymNQLIkDTwS+x6R0GS zcinyYdEbBoXGsdnGTEnwXZH26NZL6Cfri4J0+62WfxOCGVyp0ANtHpIjwYlkZ807zEX fgzg== 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:cc:subject:message-id:in-reply-to :references:reply-to:mime-version:content-transfer-encoding; bh=Dn+pIdvaPMbIiZgqoE1HBv0YN1uXjip4OsCSsvpfJV0=; b=ui2iYr2gHr8RsmL7M3GziI4sP8V5lthuWVz1ahMTgQq3oT+MkyF2565GorCnXVSoJP NhPbB10pWzv2ar+OPnLrRsPuyqGcMHod7PKqqrAEUAsuZUI/Vn7lsKh9YbdE7LOKaIm7 v0UrnWyF7Gv/t5fPtdYX9zcEo91m5wOeAfTBOcAn7sWW2cJYKPt+pvGCRueJ7iazzUI8 6yJNYry3evHL3d+4UlEV419jyII5Ww/tsJ4nGt1x4PQic1l5i0npwbMdxD3Uk/8mu3qB LYyxUkIO7lKz0VlDBla/gVeL4HhUYsdds2YdjkLRWy/Vd0Bg1LIMIDw78eKSYELqzzbU 8Y/g== X-Gm-Message-State: AOAM532eW5XUAaFBL+IHR8KToPXWC8Di9z3F4WPk9p47JToy+x6xuq3r OeWSogL/Z9IPNujTL0wN6vAqXmf1y32Q2w== X-Google-Smtp-Source: ABdhPJwiclGcRs+8J51FkEQi8+uD9AA3HDffimNwckL9w5BDDrJfv3zm/+hXHMJ5eOoNZieh1k/bxg== X-Received: by 2002:ac8:4111:: with SMTP id q17mr130262qtl.407.1633550763478; Wed, 06 Oct 2021 13:06:03 -0700 (PDT) Received: from crass-HP-ZBook-15-G2 ([37.218.244.251]) by smtp.gmail.com with ESMTPSA id p15sm13811893qti.70.2021.10.06.13.06.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Oct 2021 13:06:03 -0700 (PDT) Date: Wed, 6 Oct 2021 15:05:57 -0500 From: Glenn Washburn To: Daniel Kiper Cc: grub-devel@gnu.org Subject: Re: [PATCH v2 4/8] tests: Fail immediately when grub-shell fails and do not occlude the error code Message-ID: <20211006150557.61ece94a@crass-HP-ZBook-15-G2> In-Reply-To: <20211006135724.eauduywkvwfuuyxq@tomti.i.net-space.pl> References: <562d74a49a2beb7c68e1a9e4cf96fb9c9873defc.1629874373.git.development@efficientek.com> <20211006135724.eauduywkvwfuuyxq@tomti.i.net-space.pl> Reply-To: development@efficientek.com X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::836; envelope-from=development@efficientek.com; helo=mail-qt1-x836.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Oct 2021 20:06:09 -0000 On Wed, 6 Oct 2021 15:57:24 +0200 Daniel Kiper wrote: > On Wed, Aug 25, 2021 at 02:03:58AM -0500, Glenn Washburn wrote: > > If grub-shell fails, that means that whatever was being tested was not > > actually tested. So fail immediately. Sometimes grub-shell is not the last > > command in a pipeline of several commands, and in this case the failed error > > code can be hidden by a later failing command or hidden when 'set -e' is not > > set and there are subsequent successful commands. When the test script fails > > because of a failure in grub-shell, then test script should exit with the > > failed exit code of grub-shell. > > > > Signed-off-by: Glenn Washburn > > --- > > tests/ahci_test.in | 6 +++++- > > tests/cdboot_test.in | 3 ++- > > tests/core_compress_test.in | 6 ++++-- > > tests/ehci_test.in | 6 +++++- > > tests/fddboot_test.in | 3 ++- > > tests/grub_cmd_date.in | 3 ++- > > tests/grub_cmd_test.in | 1 + > > tests/grub_script_blockarg.in | 2 +- > > tests/grub_script_expansion.in | 3 ++- > > tests/gzcompress_test.in | 3 ++- > > tests/hddboot_test.in | 3 ++- > > tests/lzocompress_test.in | 3 ++- > > tests/netboot_test.in | 3 ++- > > tests/ohci_test.in | 6 +++++- > > tests/partmap_test.in | 4 ++-- > > tests/pata_test.in | 3 ++- > > tests/test_sha512sum.in | 1 + > > tests/uhci_test.in | 6 +++++- > > tests/xzcompress_test.in | 3 ++- > > 19 files changed, 49 insertions(+), 19 deletions(-) > > > > diff --git a/tests/ahci_test.in b/tests/ahci_test.in > > index d844fe680..30dc9d31a 100644 > > --- a/tests/ahci_test.in > > +++ b/tests/ahci_test.in > > @@ -41,7 +41,11 @@ echo "hello" > "$outfile" > > > > tar cf "$imgfile" "$outfile" > > > > -if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci -device ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then > > +echo "nativedisk; source '(ahci0)/$outfile';" | > > + "${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none > > + -device ahci,id=ahci > > + -device ide-hd,drive=disk,bus=ahci.0" >$outfile > > +if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then > > This change is in line with the commit message... > > > rm "$imgfile" > > rm "$outfile" > > exit 1 > > diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in > > index 75acdfedb..7229f79fb 100644 > > --- a/tests/cdboot_test.in > > +++ b/tests/cdboot_test.in > > @@ -34,6 +34,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in > > exit 0;; > > esac > > > > -if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ]; then > > +v=`echo hello | "${grubshell}" --boot=cd` > > +if [ "$v" != "Hello World" ]; then > > ... but this one is not. The grub-shell call is last one here. Hmmm... > Am I missing something? This is the case where the error code is hidden, so 'set -e' doesn't fail as it should. Note how this will not cause the script to error: $ bash -e -c 'if [ "$(echo XXX | ( cat; false ))" == "XXX" ]; then echo `echo $$`; fi' But this will, which is what we want. $ bash -e -c 'v=`echo XXX | ( cat; false )`; if [ "$v" == "XXX" ]; then echo `echo $$`; fi' So yes, this case, where error code is occluded with 'set -e', is not described in the commit message. Should I update the commit message? > > > exit 1 > > fi > > diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in > > index 9d216ebcf..90dd00607 100644 > > --- a/tests/core_compress_test.in > > +++ b/tests/core_compress_test.in > > @@ -27,10 +27,12 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in > > esac > > > > > > -if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)" != "Hello World" ]; then > > +v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz` > > +if [ "$v" != "Hello World" ]; then > > Ditto... And a few similar things below... > > Daniel Glenn