Skip to contentSkip to conversation

xfstests: Run all tests when nothing is specified

PATCH In review Lukas Czerner 2 participants 6 messages Jun 28, 2013 – Jul 1, 2013

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/3 xfstests: Run all tests when nothing is specified

Currently when no tests or test groups is specified xfstests will silently test nothing. Interestingly enough when test groups to exclude is specified the rest of the tests will be run.

This commit changes that to run all possible tests (for a given file system) when no specific tests has been specified. This matches the old xfstests behaviour.

---
 check | 6 +++---
 group | 5 -----
 2 files changed, 3 insertions(+), 8 deletions(-)
 delete mode 100644 group
check +3 -3
index ff8fbcf..71b179b 100755
@@ -115,7 +115,7 @@ get_all_tests()
 	for d in $SRC_GROUPS $FSTYP; do
 		ls $SRC_DIR/$d/* | \
 			grep -v "\..*" | \
-			grep -v group >> $tmp.list 2>/dev/null
+			grep -v "group\|Makefile" >> $tmp.list 2>/dev/null
 	done
 }
 
@@ -263,8 +263,8 @@ elif $have_test_arg; then
 	# had test numbers, but none in group file ... do nothing
 	touch $tmp.list
 else
-	# no test numbers, do everything from group file
-	sed -n -e '/^[0-9][0-9][0-9]*/s/[ 	].*//p' <group >$tmp.list
+	# no test numbers, do everything
+	get_all_tests
 fi
 
 # sort the list of tests into numeric order
group deleted +0 -5
deleted file mode 100644
index 4e01f0c..0000000
@@ -1,5 +0,0 @@
-# QA groups control file
-# Defines test groups and nominal group owners
-# - do not start group names with a digit
-# - comment line before each group is "new" description
-#
Signature · 6 lines
1.8.2.1

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#1 lore raw

PATCH v2 2/3 xfstests: Refactor code for obtaining test list

Put the code for obtaining the list of test into one place which makes things more readable. It will also allow us to re-init the list in the future if we need it.

---
 check | 97 ++++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 52 insertions(+), 45 deletions(-)
check +52 -45
index 71b179b..1fded37 100755
@@ -154,45 +154,70 @@ _timestamp()
     echo -n " [$now]"
 }
 
-# Process command arguments first.
-while [ $# -gt 0 ]; do
-	case "$1" in
-	-\? | -h | --help) usage ;;
-
-	-udf)	FSTYP=udf ;;
-	-xfs)	FSTYP=xfs ;;
-	-nfs)	FSTYP=nfs ;;
+_prepare_test_list()
+{
+	# Tests specified on the command line
+	if [ -s $tmp.arglist ]; then
+		cat $tmp.arglist > $tmp.list
+	else
+		touch $tmp.list
+	fi
 
-	-g)	group=$2 ; shift ;
-		group_list=$(get_group_list $group)
-		if [ -z "$group_list" ]; then
+	# Specified groups to include
+	for group in $GROUP_LIST; do
+		list=$(get_group_list $group)
+		if [ -z "$list" ]; then
 		    echo "Group \"$group\" is empty or not defined?"
 		    exit 1
 		fi
 
-		[ ! -s $tmp.list ] && touch $tmp.list
-		for t in $group_list; do
+		for t in $list; do
 			grep -s "^$t\$" $tmp.list >/dev/null || \
 							echo "$t" >>$tmp.list
 		done
+	done
 
-		;;
-
-	-x)	xgroup=$2 ; shift ;
-
-		# Note: behaviour is dependent on command line ordering of
-		# -g and -x parameters. If there are no preceding -g commands,
-		# this works on all tests, otherwise just the tests specified by
-		# the early -g inclusions.
-		[ ! -s $tmp.list ] && get_all_tests
+	if [ ! $have_test_arg ] && [ -z "$GROUP_LIST" ]; then
+		# no test numbers, do everything
+		get_all_tests
+	fi
 
-		group_list=$(get_group_list $xgroup)
-		if [ -z "$group_list" ]; then
+	# Specified groups to exclude
+	for xgroup in $XGROUP_LIST; do
+		list=$(get_group_list $xgroup)
+		if [ -z "$list" ]; then
 		    echo "Group \"$xgroup\" is empty or not defined?"
 		    exit 1
 		fi
 
-		trim_test_list $group_list
+		trim_test_list $list
+	done
+
+	# sort the list of tests into numeric order
+	list=`sort -n $tmp.list | uniq`
+	rm -f $tmp.list $tmp.tmp $tmp.grep
+
+	if $randomize
+	then
+	    list=`echo $list | awk -f randomize.awk`
+	fi
+}
+
+# Process command arguments first.
+while [ $# -gt 0 ]; do
+	case "$1" in
+	-\? | -h | --help) usage ;;
+
+	-udf)	FSTYP=udf ;;
+	-xfs)	FSTYP=xfs ;;
+	-nfs)	FSTYP=nfs ;;
+
+	-g)	group=$2 ; shift ;
+		GROUP_LIST="$GROUP_LIST $group"
+		;;
+
+	-x)	xgroup=$2 ; shift ;
+		XGROUP_LIST="$XGROUP_LIST $xgroup"
 		;;
 
 	-X)	xfile=$2; shift ;
@@ -244,7 +269,7 @@ if $have_test_arg; then
 
 			if egrep "^$test_name" $group_file >/dev/null ; then
 				# in group file ... OK
-				echo $SRC_DIR/$1 >>$tmp.list
+				echo $SRC_DIR/$1 >>$tmp.arglist
 			else
 				# oops
 				echo "$1 - unknown test, ignored"
@@ -256,25 +281,7 @@ if $have_test_arg; then
 	done
 fi
 
-if [ -s $tmp.list ]; then
-    # found some valid test numbers ... this is good
-    :
-elif $have_test_arg; then
-	# had test numbers, but none in group file ... do nothing
-	touch $tmp.list
-else
-	# no test numbers, do everything
-	get_all_tests
-fi
-
-# sort the list of tests into numeric order
-list=`sort -n $tmp.list`
-rm -f $tmp.list $tmp.tmp $tmp.grep
-
-if $randomize
-then
-    list=`echo $list | awk -f randomize.awk`
-fi
+_prepare_test_list
 
 # we need common/rc
 if ! . ./common/rc
Signature · 6 lines
1.8.2.1

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#2 lore raw

PATCH v2 3/3 xfstests: Add support for sections in config file

In reply to Lukas Czerner

This patch add support for sections in the config file. Each section can contain configuration options in the format

OPTION=value

when one section is processed xfstests will proceed to next section until all secitons are processed, or an error occur.

The name of the section can consist of alphanumeric characters + '_', nothing else is allowed. Name of the section is also used to create results subdirectory for each section. After all the sections are processed summary of all runs is printed out.

If the config file does not contain sections, or we're not using config file at all, nothing is changed and xfstests will work the same way as it used to.

This is very useful for testing file system with different options. Here is an example of the config file with sections:

[ext4_4k_block_size]
TEST_DEV=/dev/sda
TEST_DIR=/mnt/test
SCRATCH_DEV=/dev/sdb
SCRATCH_MNT=/mnt/test1
MKFS_OPTIONS="-q -F -b4096"
FSTYP=ext4

[ext4_1k_block_size]
MKFS_OPTIONS="-q -F -b1024"

[ext4_nojournal]
MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"

[ext4_discard_ssd]
MKFS_OPTIONS="-q -F -b4096"
TEST_DEV=/dev/sdc
SCRATCH_DEV=/dev/sdd
MOUNT_OPTIONS="-o discard"

Note that once the variable is set it remains set across the sections, so you do not have to specify all the options in all sections. However one have to make sure that unwanted options are not set from previous sections.

---
 check         | 371 +++++++++++++++++++++++++++++++++-------------------------
 common/config | 126 ++++++++++++--------
 common/rc     |  63 +++++-----
 3 files changed, 326 insertions(+), 234 deletions(-)
check +213 -158
index 1fded37..e8519a5 100755
@@ -22,9 +22,11 @@
 tmp=/tmp/$$
 status=0
 needwrap=true
+needsum=true
 n_try=0
 try=""
 n_bad=0
+sum_bad=0
 bad=""
 notrun=""
 interrupt=true
@@ -66,7 +68,6 @@ export FSTYP
 SUPPORTED_TESTS="[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]"
 SRC_GROUPS="generic shared"
 export SRC_DIR="tests"
-export RESULT_BASE=${RESULT_BASE:="$here/results"}
 
 usage()
 {
@@ -296,7 +297,12 @@ then
     exit 1
 fi
 
-# Ok, time to start running...
+_wipe_counters()
+{
+	n_try="0"
+	n_bad="0"
+	unset try notrun bad
+}
 
 _wrapup()
 {
@@ -325,16 +331,20 @@ END	{ if (NR > 0) {
 	date >>$check.log
 	echo $list | fmt | sed -e 's/^/    /' -e "s;$SRC_DIR/;;g" >>$check.log
 	$interrupt && echo "Interrupted!" >>$check.log
-        
+
+	echo "SECTION       -- $section" >>$tmp.summary
+	echo "=========================" >>$tmp.summary
         if [ ! -z "$n_try" -a $n_try != 0 ]
 	then
 	    echo "Ran:$try"
+	    echo "Ran:$try" >>$tmp.summary
 	fi
 
 	if [ ! -z "$notrun" ]
 	then
 	    echo "Not run:$notrun"
 	    echo "Not run:$notrun" >>$check.log
+	    echo "Not run:$notrun" >>$tmp.summary
 	fi
 
         if [ ! -z "$n_bad" -a $n_bad != 0 ]
@@ -343,200 +353,245 @@ END	{ if (NR > 0) {
 	    echo "Failed $n_bad of $n_try tests"
 	    echo "Failures:$bad" | fmt >>$check.log
 	    echo "Failed $n_bad of $n_try tests" >>$check.log
+	    echo "Failures:$bad" >>$tmp.summary
+	    echo "Failed $n_bad of $n_try tests" >>$tmp.summary
 	else
 	    echo "Passed all $n_try tests"
 	    echo "Passed all $n_try tests" >>$check.log
+	    echo "Passed all $n_try tests" >>$tmp.summary
 	fi
+	echo "" >>$tmp.summary
 	needwrap=false
     fi
 
+    sum_bad=`expr $sum_bad + $n_bad`
+    _wipe_counters
     rm -f /tmp/*.rawout /tmp/*.out /tmp/*.err /tmp/*.time
-    rm -f $tmp.*
+    if ! $OPTIONS_HAVE_SECTIONS; then
+        rm -f $tmp.*
+    fi
 }
 
-trap "_wrapup; exit \$status" 0 1 2 3 15
-
-mkdir -p $RESULT_BASE
-if [ ! -d $RESULT_BASE ]; then
-	echo "failed to create results directory $RESULTS_BASE"
-	exit 1;
-fi
+_summary()
+{
+	_wrapup
+	if $showme; then
+		:
+	elif $needsum; then
+		count=`wc -L $tmp.summary | cut -f1 -d" "`
+		cat $tmp.summary
+		needsum=false
+	fi
+	rm -f $tmp.*
+}
 
-seq="check"
-check="$RESULT_BASE/check"
+# Ok, time to start running...
 
-# don't leave old full output behind on a clean run
-rm -f $check.full
+if $OPTIONS_HAVE_SECTIONS; then
+	trap "_summary; exit \$status" 0 1 2 3 15
+else
+	trap "_wrapup; exit \$status" 0 1 2 3 15
+fi
 
-[ -f $check.time ] || touch $check.time
+for section in $HOST_OPTIONS_SECTIONS; do
 
-# print out our test configuration
-echo "FSTYP         -- `_full_fstyp_details`"
-echo "PLATFORM      -- `_full_platform_details`"
-if [ ! -z "$SCRATCH_DEV" ]; then
-  echo "MKFS_OPTIONS  -- `_scratch_mkfs_options`"
-  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
-fi
-echo
-
-
-if [ ! -z "$SCRATCH_DEV" ]; then
-  umount $SCRATCH_DEV 2>/dev/null
-  # call the overridden mkfs - make sure the FS is built
-  # the same as we'll create it later.
-
-  if ! _scratch_mkfs $flag >$tmp.err 2>&1
-  then
-      echo "our local _scratch_mkfs routine ..."
-      cat $tmp.err
-      echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
-      exit 1
-  fi
-
-  # call the overridden mount - make sure the FS mounts with
-  # the same options that we'll mount with later.
-  if ! _scratch_mount >$tmp.err 2>&1
-  then
-      echo "our local mount routine ..."
-      cat $tmp.err
-      echo "check: failed to mount \$SCRATCH_DEV using specified options"
-      exit 1
-  fi
-fi
+	if $OPTIONS_HAVE_SECTIONS; then
+		export RESULT_BASE="$here/results/$section"
+	else
+		export RESULT_BASE="$here/results/"
+	fi
 
-seqres="$check"
-_check_test_fs
+	get_next_config $section
+	init_rc
 
-for seq in $list
-do
-    err=false
+	mkdir -p $RESULT_BASE
+	if [ ! -d $RESULT_BASE ]; then
+		echo "failed to create results directory $RESULTS_BASE"
+		exit 1;
+	fi
 
-    # the filename for the test and the name output are different.
-    # we don't include the tests/ directory in the name output.
-    seqnum=`echo $seq | sed -e "s;$SRC_DIR/;;"`
+	seq="check"
+	check="$RESULT_BASE/check"
 
-    # Similarly, the result directory needs to replace the tests/
-    # part of the test location.
-    group=`dirname $seq`
-    export RESULT_DIR=`echo $group | sed -e "s;$SRC_DIR;$RESULT_BASE;"`
-    mkdir -p $RESULT_DIR
-    seqres="$RESULT_BASE/$seqnum"
+	# don't leave old full output behind on a clean run
+	rm -f $check.full
 
-    echo -n "$seqnum"
+	[ -f $check.time ] || touch $check.time
 
-    if $showme
-    then
+	# print out our test configuration
+	if $OPTIONS_HAVE_SECTIONS; then
+		echo "SECTION       -- $section"
+	fi
+	echo "FSTYP         -- `_full_fstyp_details`"
+	echo "PLATFORM      -- `_full_platform_details`"
+	if [ ! -z "$SCRATCH_DEV" ]; then
+	  echo "MKFS_OPTIONS  -- `_scratch_mkfs_options`"
+	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
+	fi
 	echo
-	continue
-    elif [ ! -f $seq ]
-    then
-	echo " - no such test?"
-    else
-	# really going to try and run this one
-	#
-	rm -f $seqres.out.bad
-
-	# check if we really should run it
-	if [ -s $tmp.xlist ]; then
-		if grep $seqnum $tmp.xlist > /dev/null 2>&1 ; then
-			echo "       [expunged]"
-			continue
-		fi
+	needwrap=true
+
+
+	if [ ! -z "$SCRATCH_DEV" ]; then
+	  umount $SCRATCH_DEV 2>/dev/null
+	  # call the overridden mkfs - make sure the FS is built
+	  # the same as we'll create it later.
+
+	  if ! _scratch_mkfs $flag >$tmp.err 2>&1
+	  then
+	      echo "our local _scratch_mkfs routine ..."
+	      cat $tmp.err
+	      echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
+	      exit 1
+	  fi
+
+	  # call the overridden mount - make sure the FS mounts with
+	  # the same options that we'll mount with later.
+	  if ! _scratch_mount >$tmp.err 2>&1
+	  then
+	      echo "our local mount routine ..."
+	      cat $tmp.err
+	      echo "check: failed to mount \$SCRATCH_DEV using specified options"
+	      exit 1
+	  fi
 	fi
 
-	# slashes now in names, sed barfs on them so use grep
-	lasttime=`grep -w ^$seqnum $check.time | awk '// {print $2}'`
-	if [ "X$lasttime" != X ]; then
-		echo -n " ${lasttime}s ..."
-	else
-		echo -n "	"	# prettier output with timestamps.
-	fi
-	rm -f core $seqres.notrun
+	seqres="$check"
+	_check_test_fs
 
-	start=`_wallclock`
-	$timestamp && echo -n "	["`date "+%T"`"]"
-	[ ! -x $seq ] && chmod u+x $seq # ensure we can run it
-	$LOGGER_PROG "run xfstest $seqnum"
-	./$seq >$tmp.rawout 2>&1
-	sts=$?
-	$timestamp && _timestamp
-	stop=`_wallclock`
+	for seq in $list
+	do
+	    err=false
 
-	_fix_malloc <$tmp.rawout >$tmp.out
-	rm -f $tmp.rawout
+	    # the filename for the test and the name output are different.
+	    # we don't include the tests/ directory in the name output.
+	    seqnum=`echo $seq | sed -e "s;$SRC_DIR/;;"`
 
-	if [ -f core ]
-	then
-	    echo -n " [dumped core]"
-	    mv core $RESULT_BASE/$seqnum.core
-	    err=true
-	fi
+	    # Similarly, the result directory needs to replace the tests/
+	    # part of the test location.
+	    group=`dirname $seq`
+	    export RESULT_DIR=`echo $group | sed -e "s;$SRC_DIR;$RESULT_BASE;"`
+	    mkdir -p $RESULT_DIR
+	    seqres="$RESULT_BASE/$seqnum"
 
-	if [ -f $seqres.notrun ]
-	then
-	    $timestamp || echo -n " [not run] "
-	    $timestamp && echo " [not run]" && echo -n "	$seqnum -- "
-	    cat $seqres.notrun
-	    notrun="$notrun $seqnum"
-	else
-	    if [ $sts -ne 0 ]
+	    echo -n "$seqnum"
+
+	    if $showme
 	    then
-		echo -n " [failed, exit status $sts]"
-		err=true
-	    fi
-	    if [ ! -f $seq.out ]
+		echo
+		continue
+	    elif [ ! -f $seq ]
 	    then
-		echo " - no qualified output"
-		err=true
+		echo " - no such test?"
 	    else
-		if diff $seq.out $tmp.out >/dev/null 2>&1
+		# really going to try and run this one
+		#
+		rm -f $seqres.out.bad
+
+		# check if we really should run it
+		if [ -s $tmp.xlist ]; then
+			if grep $seqnum $tmp.xlist > /dev/null 2>&1 ; then
+				echo "       [expunged]"
+				continue
+			fi
+		fi
+
+		# slashes now in names, sed barfs on them so use grep
+		lasttime=`grep -w ^$seqnum $check.time | awk '// {print $2}'`
+		if [ "X$lasttime" != X ]; then
+			echo -n " ${lasttime}s ..."
+		else
+			echo -n "	"	# prettier output with timestamps.
+		fi
+		rm -f core $seqres.notrun
+
+		start=`_wallclock`
+		$timestamp && echo -n "	["`date "+%T"`"]"
+		[ ! -x $seq ] && chmod u+x $seq # ensure we can run it
+		$LOGGER_PROG "run xfstest $seqnum"
+		./$seq >$tmp.rawout 2>&1
+		sts=$?
+		$timestamp && _timestamp
+		stop=`_wallclock`
+
+		_fix_malloc <$tmp.rawout >$tmp.out
+		rm -f $tmp.rawout
+
+		if [ -f core ]
+		then
+		    echo -n " [dumped core]"
+		    mv core $RESULT_BASE/$seqnum.core
+		    err=true
+		fi
+
+		if [ -f $seqres.notrun ]
 		then
-		    if $err
+		    $timestamp || echo -n " [not run] "
+		    $timestamp && echo " [not run]" && echo -n "	$seqnum -- "
+		    cat $seqres.notrun
+		    notrun="$notrun $seqnum"
+		else
+		    if [ $sts -ne 0 ]
 		    then
-			:
-		    else
-			echo "$seqnum `expr $stop - $start`" >>$tmp.time
-			echo -n " `expr $stop - $start`s"
+			echo -n " [failed, exit status $sts]"
+			err=true
 		    fi
-		    echo ""
-		else
-		    echo " - output mismatch (see $seqres.out.bad)"
-		    mv $tmp.out $seqres.out.bad
-		    $diff $seq.out $seqres.out.bad | {
-		        if test "$DIFF_LENGTH" -le 0; then
-				cat
+		    if [ ! -f $seq.out ]
+		    then
+			echo " - no qualified output"
+			err=true
+		    else
+			if diff $seq.out $tmp.out >/dev/null 2>&1
+			then
+			    if $err
+			    then
+				:
+			    else
+				echo "$seqnum `expr $stop - $start`" >>$tmp.time
+				echo -n " `expr $stop - $start`s"
+			    fi
+			    echo ""
 			else
-				head -n "$DIFF_LENGTH"
-			fi; } | \
-			sed -e 's/^\(.\)/    \1/'
-		    echo "     ..."
-		    echo "     (Run '$diff $seq.out $seqres.out.bad' to see the" \
-			 "entire diff)"
-		    err=true
+			    echo " - output mismatch (see $seqres.out.bad)"
+			    mv $tmp.out $seqres.out.bad
+			    $diff $seq.out $seqres.out.bad | {
+				if test "$DIFF_LENGTH" -le 0; then
+					cat
+				else
+					head -n "$DIFF_LENGTH"
+				fi; } | \
+				sed -e 's/^\(.\)/    \1/'
+			    echo "     ..."
+			    echo "     (Run '$diff $seq.out $seqres.out.bad' to see the" \
+				 "entire diff)"
+			    err=true
+			fi
+		    fi
 		fi
-	    fi
-	fi
 
-    fi
+	    fi
 
-    # come here for each test, except when $showme is true
-    #
-    if $err
-    then
-	bad="$bad $seqnum"
-	n_bad=`expr $n_bad + 1`
-	quick=false
-    fi
-    if [ ! -f $seqres.notrun ]
-    then
-	try="$try $seqnum"
-	n_try=`expr $n_try + 1`
-        _check_test_fs
-    fi
+	    # come here for each test, except when $showme is true
+	    #
+	    if $err
+	    then
+		bad="$bad $seqnum"
+		n_bad=`expr $n_bad + 1`
+		quick=false
+	    fi
+	    if [ ! -f $seqres.notrun ]
+	    then
+		try="$try $seqnum"
+		n_try=`expr $n_try + 1`
+		_check_test_fs
+	    fi
 
-    seq="after_$seqnum"
+	    seq="after_$seqnum"
+	done
+	_wrapup
+        echo
 done
 
 interrupt=false
-status=`expr $n_bad`
+status=`expr $sum_bad`
 exit
common/config +80 -46

142 lines not on this page · raw message

common/rc +33 -30

79 lines not on this page · raw message

Signature · 6 lines
1.8.2.1

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#3 lore raw

Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

Quoted 40 lines · Lukas Czerner

This patch add support for sections in the config file. Each section can contain configuration options in the format

OPTION=value

when one section is processed xfstests will proceed to next section until all secitons are processed, or an error occur.

The name of the section can consist of alphanumeric characters + '_', nothing else is allowed. Name of the section is also used to create results subdirectory for each section. After all the sections are processed summary of all runs is printed out.

If the config file does not contain sections, or we're not using config file at all, nothing is changed and xfstests will work the same way as it used to.

This is very useful for testing file system with different options. Here is an example of the config file with sections:

[ext4_4k_block_size]
TEST_DEV=/dev/sda
TEST_DIR=/mnt/test
SCRATCH_DEV=/dev/sdb
SCRATCH_MNT=/mnt/test1
MKFS_OPTIONS="-q -F -b4096"
FSTYP=ext4

[ext4_1k_block_size]
MKFS_OPTIONS="-q -F -b1024"

[ext4_nojournal]
MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"

[ext4_discard_ssd]
MKFS_OPTIONS="-q -F -b4096"
TEST_DEV=/dev/sdc
SCRATCH_DEV=/dev/sdd
MOUNT_OPTIONS="-o discard"

Quoted 4 lines

Note that once the variable is set it remains set across the sections, so you do not have to specify all the options in all sections. However one have to make sure that unwanted options are not set from previous sections.

I like the idea, but a lot of this needs to be in the (missing) patch 0 description for the series. YOu also need to describe how these get invoked, what the output looks like, etc because right now I can't work it out from this...

Quoted 6 lines
---
 check         | 371 +++++++++++++++++++++++++++++++++-------------------------
 common/config | 126 ++++++++++++--------
 common/rc     |  63 +++++-----
 3 files changed, 326 insertions(+), 234 deletions(-)

This patch probably needs to be broken up, too. A substantial part of it is indentation changes, which probably should be split into 2 parts - factor code into function, then wrap loop around function. The change to the summary information should be done as a separate patch, too. I suspect many of the common/config changes coul dbe split up, too.

The changing of the $RESULT_BASE should probably also bein a separate patch, because this is something that we'll need to discuss as it changes the structure of the output....

Oh, and why make a distinction between no sections and $OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is just the same as having 1 section....

common/config known_hosts() · line 257

Quoted 13 lines
common/config +13 -0
+parse_config_section() {
+	SECTION=$1
+	if ! $OPTIONS_HAVE_SECTIONS; then
+		return 0
+	fi
+	eval `sed -e 's/[[:space:]]*\=[[:space:]]*/=/g' \
+		-e 's/#.*$//' \
+		-e 's/[[:space:]]*$//' \
+		-e 's/^[[:space:]]*//' \
+		-e "s/^\(.*\)=\([^\"']*\)$/export \1=\"\2\"/" \
+		< $HOST_OPTIONS \
+		| sed -n -e "/^\[$SECTION\]/,/^\s*\[/{/^[^;].*\=.*/p;}"`
+}

And line noise like this needs comments explaining the format and what it is doing. ;)

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#4 lore raw

Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

Quoted 52 lines · Dave Chinner

Date: Mon, 1 Jul 2013 11:49:44 +1000
From: Dave Chinner <[email protected]>
To: Lukas Czerner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

Quoted 40 lines · Lukas Czerner

This patch add support for sections in the config file. Each section can contain configuration options in the format

OPTION=value

when one section is processed xfstests will proceed to next section until all secitons are processed, or an error occur.

The name of the section can consist of alphanumeric characters + '_', nothing else is allowed. Name of the section is also used to create results subdirectory for each section. After all the sections are processed summary of all runs is printed out.

If the config file does not contain sections, or we're not using config file at all, nothing is changed and xfstests will work the same way as it used to.

This is very useful for testing file system with different options. Here is an example of the config file with sections:

[ext4_4k_block_size]
TEST_DEV=/dev/sda
TEST_DIR=/mnt/test
SCRATCH_DEV=/dev/sdb
SCRATCH_MNT=/mnt/test1
MKFS_OPTIONS="-q -F -b4096"
FSTYP=ext4

[ext4_1k_block_size]
MKFS_OPTIONS="-q -F -b1024"

[ext4_nojournal]
MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"

[ext4_discard_ssd]
MKFS_OPTIONS="-q -F -b4096"
TEST_DEV=/dev/sdc
SCRATCH_DEV=/dev/sdd
MOUNT_OPTIONS="-o discard"

Quoted 4 lines

Note that once the variable is set it remains set across the sections, so you do not have to specify all the options in all sections. However one have to make sure that unwanted options are not set from previous sections.

Quoted 4 lines

I like the idea, but a lot of this needs to be in the (missing) patch 0 description for the series. YOu also need to describe how these get invoked, what the output looks like, etc because right now I can't work it out from this...

Fair enough, I should probably describe it in the README as well.

Quoted 8 lines
Quoted 7 lines
---
 check         | 371 +++++++++++++++++++++++++++++++++-------------------------
 common/config | 126 ++++++++++++--------
 common/rc     |  63 +++++-----
 3 files changed, 326 insertions(+), 234 deletions(-)
Quoted 6 lines

This patch probably needs to be broken up, too. A substantial part of it is indentation changes, which probably should be split into 2 parts - factor code into function, then wrap loop around function. The change to the summary information should be done as a separate patch, too. I suspect many of the common/config changes coul dbe split up, too.

Ok, I'll see how can I split it up. Btw, indentation in xfstests is a mess, because sometimes we're using spaces and sometimes tabs. Is there any preference ? (I would definitely prefer tabs)

Quoted 4 lines

The changing of the $RESULT_BASE should probably also bein a separate patch, because this is something that we'll need to discuss as it changes the structure of the output....

I am not sure it should be separate from this patch because the new structure will only be used if the new config format (with sections) is used.

Quoted 4 lines

Oh, and why make a distinction between no sections and $OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is just the same as having 1 section....

Yes, but it's much "nicer" to check boolean option than checking what is the name of the first section. Really this is just a workaround, because I did not want to change result structure and output if one is not using the new config format. It could be done so that when there are no sections we'll always use "default" section, not sure what would people prefer.

Quoted 17 lines
Quoted 4 lines
common/config +3 -0
+parse_config_section() {
+	SECTION=$1
+	if ! $OPTIONS_HAVE_SECTIONS; then

common/config known_hosts() · line 257

Quoted 10 lines
common/config +10 -0
+		return 0
+	fi
+	eval `sed -e 's/[[:space:]]*\=[[:space:]]*/=/g' \
+		-e 's/#.*$//' \
+		-e 's/[[:space:]]*$//' \
+		-e 's/^[[:space:]]*//' \
+		-e "s/^\(.*\)=\([^\"']*\)$/export \1=\"\2\"/" \
+		< $HOST_OPTIONS \
+		| sed -n -e "/^\[$SECTION\]/,/^\s*\[/{/^[^;].*\=.*/p;}"`
+}

And line noise like this needs comments explaining the format and what it is doing. ;)

Sure I'll add some comments.

Thanks!
-Lukas

Quoted 5 lines

Cheers,

Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#5 lore raw

Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

Quoted 15 lines · Lukáš Czerner
Quoted 7 lines · Dave Chinner
Quoted 6 lines
---
 check         | 371 +++++++++++++++++++++++++++++++++-------------------------
 common/config | 126 ++++++++++++--------
 common/rc     |  63 +++++-----
 3 files changed, 326 insertions(+), 234 deletions(-)
Quoted 6 lines

This patch probably needs to be broken up, too. A substantial part of it is indentation changes, which probably should be split into 2 parts - factor code into function, then wrap loop around function. The change to the summary information should be done as a separate patch, too. I suspect many of the common/config changes coul dbe split up, too.

Quoted 3 lines

Ok, I'll see how can I split it up. Btw, indentation in xfstests is a mess, because sometimes we're using spaces and sometimes tabs. Is there any preference ? (I would definitely prefer tabs)

Tabs.

Quoted 7 lines
Quoted 3 lines

The changing of the $RESULT_BASE should probably also bein a separate patch, because this is something that we'll need to discuss as it changes the structure of the output....

I am not sure it should be separate from this patch because the new structure will only be used if the new config format (with sections) is used.

Which is a bit confusing, especially as a separate result directory might be desired for the output of each section - say I want to keep all the "config A" results together, but spearate to all the "config B" results. Placing them all under the same $RESULT_BASE isn't ideal at that point....

IOWs, I suspect that $RESULT_BASE should be able to be defined in the section config so that you can redirect results that way.

Quoted 8 lines
Quoted 3 lines

Oh, and why make a distinction between no sections and $OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is just the same as having 1 section....

Yes, but it's much "nicer" to check boolean option than checking what is the name of the first section. Really this is just a workaround, because I did not want to change result structure and output if one is not using the new config format.

That's another reason why I think that we should be able to have a per-section RESULTS_BASE - that way the code doesn't care how RESULTS_BASE is defined or where it points to - the output of xfstests is *always* the same. That is, after all, why $RESULTS_BASE was introduced in the first place.

Quoted 3 lines

It could be done
so that when there are no sections we'll always use "default" section, not sure what would people prefer.

Exactly why it needs discussion ;)

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs