public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 14/18] xfstests: remove -x functionality
@ 2013-03-14 13:06 Philip White
  2013-03-15  8:47 ` Dave Chinner
  2013-03-15 14:13 ` Eric Sandeen
  0 siblings, 2 replies; 3+ messages in thread
From: Philip White @ 2013-03-14 13:06 UTC (permalink / raw)
  To: xfs

From: Phil White <pwhite@sgi.com>


This is a rebasing & resubmit of a dchinner patch.  His comments on the
original:
-----------------
Now that there are no tests in the top level directory, the exlude
group functionality no longer works (-x group) as it cannot find
tests to build an initial list of tests. Rework the the exclude
group implementation to operate correctly on the new test locations.

Also, swtich from sed to grep for exclusion because sed has problems
distnguishing the '/' in path names from regex control...
-----------------
This one is mildly different from the original as it modifies infrastructure
which remains in common.  In Dave's original patch, that had all been moved
into check.

Signed-off-by: Phil White <pwhite@sgi.com>
---
 common |   43 +++++++++++++++++++++++++++++++------------
 1 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/common b/common
index 1c6a08c..4732a66 100644
--- a/common
+++ b/common
@@ -56,6 +56,19 @@ get_group_list()
 	echo $grpl
 }
 
+# find all tests, excluding files that are test metadata such as group files.
+# This assumes that tests are defined purely by alphanumeric filenames with no
+# ".xyz" extensions in the name.
+get_all_tests()
+{
+	touch $tmp.list
+	for d in $SRC_GROUPS $FSTYP; do
+		ls $SRC_DIR/$d/* | \
+			grep -v "\..*" | \
+			grep -v group >> $tmp.list 2>/dev/null
+	done
+}
+
 _wallclock()
 {
 	date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
@@ -69,7 +82,7 @@ _timestamp()
 
 # start the initialisation work now
 _setenvironment
-rm -f $tmp.list $tmp.tmp $tmp.sed $here/$iam.out
+rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out
 
 # Autodetect fs type based on what's on $TEST_DEV
 if [ "$HOSTOS" == "Linux" ]
@@ -109,27 +122,33 @@ while [ $# -gt 0 ]; do
 		;;
 
 	-x)	xgroup=$2 ; shift ;
-		[ ! -s $tmp.list ] && ls $SUPPORTED_TESTS >$tmp.list 2>/dev/null
+
+		# 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
+
 		group_list=$(get_group_list $xgroup)
 		if [ -z "$group_list" ]; then
 			echo "Group \"$xgroup\" is empty or not defined?"
 			exit 1
 		fi
 
-		rm -f $tmp.sed
-		numsed=0
+		rm -f $tmp.grep
+		numgrep=0
 		for t in $group_list
 		do
-			if [ $numsed -gt 100 ]; then
-				sed -f $tmp.sed <$tmp.list >$tmp.tmp
+			if [ $numgrep -gt 100 ]; then
+				grep -v -f $tmp.grep <$tmp.list >$tmp.tmp
 				mv $tmp.tmp $tmp.list
-				numsed=0
-				rm -f $tmp.sed
+				numgrep=0
+				rm -f $tmp.grep
 			fi
-			echo "/^$t\$/d" >>$tmp.sed
-			numsed=`expr $numsed + 1`
+			echo "/^$t\$/d" >>$tmp.grep
+			numgrep=`expr $numgrep + 1`
 		done
-		sed -f $tmp.sed <$tmp.list >$tmp.tmp
+		grep -v -f $tmp.grep <$tmp.list >$tmp.tmp
 		mv $tmp.tmp $tmp.list
 		;;
 
@@ -199,7 +218,7 @@ fi
 
 # sort the list of tests into numeric order
 list=`sort -n $tmp.list`
-rm -f $tmp.list $tmp.tmp $tmp.sed
+rm -f $tmp.list $tmp.tmp $tmp.grep
 
 if $randomize
 then
-- 
1.5.3.8

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 14/18] xfstests: remove -x functionality
  2013-03-14 13:06 [PATCH 14/18] xfstests: remove -x functionality Philip White
@ 2013-03-15  8:47 ` Dave Chinner
  2013-03-15 14:13 ` Eric Sandeen
  1 sibling, 0 replies; 3+ messages in thread
From: Dave Chinner @ 2013-03-15  8:47 UTC (permalink / raw)
  To: Philip White; +Cc: xfs

On Thu, Mar 14, 2013 at 06:06:11AM -0700, Philip White wrote:
> From: Phil White <pwhite@sgi.com>
> 
> 
> This is a rebasing & resubmit of a dchinner patch.  His comments on the
> original:
> -----------------
> Now that there are no tests in the top level directory, the exlude
> group functionality no longer works (-x group) as it cannot find
> tests to build an initial list of tests. Rework the the exclude
> group implementation to operate correctly on the new test locations.
> 
> Also, swtich from sed to grep for exclusion because sed has problems
> distnguishing the '/' in path names from regex control...
> -----------------
> This one is mildly different from the original as it modifies infrastructure
> which remains in common.  In Dave's original patch, that had all been moved
> into check.

Why change the patch title? It still does what the original patch
did, so the new patch title is incorrect....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 14/18] xfstests: remove -x functionality
  2013-03-14 13:06 [PATCH 14/18] xfstests: remove -x functionality Philip White
  2013-03-15  8:47 ` Dave Chinner
@ 2013-03-15 14:13 ` Eric Sandeen
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Sandeen @ 2013-03-15 14:13 UTC (permalink / raw)
  To: Philip White; +Cc: xfs

On 3/14/13 8:06 AM, Philip White wrote:
> From: Phil White <pwhite@sgi.com>
> 
> 
> This is a rebasing & resubmit of a dchinner patch.  His comments on the
> original:
> -----------------
> Now that there are no tests in the top level directory, the exlude
> group functionality no longer works (-x group) as it cannot find
> tests to build an initial list of tests. Rework the the exclude
> group implementation to operate correctly on the new test locations.
> 
> Also, swtich from sed to grep for exclusion because sed has problems
> distnguishing the '/' in path names from regex control...
> -----------------
> This one is mildly different from the original as it modifies infrastructure
> which remains in common.  In Dave's original patch, that had all been moved
> into check.

And the rationale for the change from the original is what?  Changelogs are better with reasoning, than with just descriptions of change.

Just a style note - it's not great form to take someone else's large body of work, tweak it, then claim authorship (From:) and sign-off (Signed-off-by:).  If it were me, I might be a little tweaked.  It's probably an oversight, and maybe Dave doesn't even care, but it's my pet peeve.

Since Dave has rebased & resent the original patchset he wrote, it might be better, going forward, for you guys to try to reach agreement on any necessary changes to his body of work, and get that committed.  Unless we want to carry on w/ dueling patchsets, which doesn't sound optimal to me.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-03-15 14:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-14 13:06 [PATCH 14/18] xfstests: remove -x functionality Philip White
2013-03-15  8:47 ` Dave Chinner
2013-03-15 14:13 ` Eric Sandeen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox