diff mbox series

fstests: make check actually honor expunged list

Message ID 732fc62557b3dfdba820b83cd9643de314b6976e.1689968898.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series fstests: make check actually honor expunged list | expand

Commit Message

Josef Bacik July 21, 2023, 7:48 p.m. UTC
The patch 60054d51 ("check: fix excluded tests are only expunged in the
first iteration") messed up the logic, if _function will be true if it
gets 1, so the _expunge_test return values are inverted.  Also it
appears bash swallows the output in this calling convention, so you
don't get the 'expunged' output.  I noticed this when my CI system
stopped honoring my exclude list, this makes everything work properly
again.

Fixes: 60054d51 ("check: fix excluded tests are only expunged in the first iteration")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Josef Bacik July 21, 2023, 8:04 p.m. UTC | #1
On Fri, Jul 21, 2023 at 03:48:24PM -0400, Josef Bacik wrote:
> The patch 60054d51 ("check: fix excluded tests are only expunged in the
> first iteration") messed up the logic, if _function will be true if it
> gets 1, so the _expunge_test return values are inverted.  Also it
> appears bash swallows the output in this calling convention, so you
> don't get the 'expunged' output.  I noticed this when my CI system
> stopped honoring my exclude list, this makes everything work properly
> again.
> 
> Fixes: 60054d51 ("check: fix excluded tests are only expunged in the first iteration")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

So turns out I'm an idiot, this was just working in my simple case where I tried
to run a test that I knew was in the list, but of course it falls apart with
normal stuff.

Turns out the real issue is that we now expect the expunge list to be in the
format of

whatever/1
whatever/2
whatever/3

instead of

whatever/1 whatever/2 whatever/3

maybe we always expected the other way and it just happened to work my way
because we were using grep.

So ignore this patch, but we should probably document the format of the expunge
list files.  Thanks,

Josef
diff mbox series

Patch

diff --git a/check b/check
index c8593252..b731af74 100755
--- a/check
+++ b/check
@@ -570,11 +570,10 @@  _expunge_test()
 
 	for f in "${exclude_tests[@]}"; do
 		if [ "${TEST_ID}" == "$f" ]; then
-			echo "       [expunged]"
-			return 0
+			return 1
 		fi
 	done
-	return 1
+	return 0
 }
 
 # retain files which would be overwritten in subsequent reruns of the same test
@@ -870,6 +869,7 @@  function run_section()
 
 		if $showme; then
 			if _expunge_test $seqnum; then
+				echo "       [expunged]"
 				tc_status="expunge"
 			else
 				echo
@@ -896,6 +896,7 @@  function run_section()
 
 		# check if we really should run it
 		if _expunge_test $seqnum; then
+			echo "       [expunged]"
 			tc_status="expunge"
 			_stash_test_status "$seqnum" "$tc_status"
 			continue