xfstests: Run all tests when nothing is specified
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
@@ -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
@@ -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
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
@@ -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
PATCH v2 3/3 xfstests: Add support for sections in config file
In reply to Lukas CzernerThis 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
@@ -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
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
Signed-off-by Lukas Czerner <[email protected]>--- 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
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 fileQuoted 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
Signed-off-by Lukas Czerner <[email protected]>--- 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; thencommon/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
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
Signed-off-by Lukas Czerner <[email protected]>--- 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