devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [0/10] Assorted cleanups (esp for fdt{get,put} tests)
@ 2012-02-03  5:11 David Gibson
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:11 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

As noted earlier, there were a few things I wasn't totally happy with
in the recently applied fdt{get,put} patches, mostly in the test code.
I had a look at cleaning up those problems today, which kind of
expanded into a whole bunch of little cleanups for tests and a few
other things.

So, here they all are.  Jon, please apply.

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

* [PATCH 01/10] Update .gitignore for tests
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 02/10] Add quilt files to .gitignore David Gibson
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

We've add some test (generated) binaries that aren't currently listed in
.gitignore, in addition more scripts now generate various tmp.* files
during operation.  This adds them all to .gitignore.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/.gitignore |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tests/.gitignore b/tests/.gitignore
index 9e062c3..0b71bcf 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,7 +1,9 @@
 *.dtb
 *.dtb.test.dts
 *.dts.test.s
+tmp.*
 /add_subnode_with_nops
+/appendprop[12]
 /asm_tree_dump
 /boot-cpuid
 /char_literal
@@ -46,6 +48,7 @@
 /supernode_atdepth_offset
 /sw_tree1
 /truncated_property
+/utilfdt_test
 /value-labels
 /dtb_reverse
 /dtbs_equal_unordered
-- 
1.7.8.3

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

* [PATCH 02/10] Add quilt files to .gitignore
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
  2012-02-03  5:12   ` [PATCH 01/10] Update .gitignore for tests David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 03/10] Trivial style fixup David Gibson
                     ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

For the benefit of quilt users (such as myself, sometimes) have git
ignore the quilt control and patches files.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 .gitignore |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index 5074980..7cabc49 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,3 +12,5 @@ lex.yy.c
 /version_gen.h
 /fdtget
 /fdtput
+/patches
+/.pc
-- 
1.7.8.3

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

* [PATCH 03/10] Trivial style fixup
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
  2012-02-03  5:12   ` [PATCH 01/10] Update .gitignore for tests David Gibson
  2012-02-03  5:12   ` [PATCH 02/10] Add quilt files to .gitignore David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 04/10] Remove unused variable from test scripts David Gibson
                     ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Having braces on an if branch but not the else branch, or vice
versa is ugly and can trick you when reading the code.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 fdtget.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fdtget.c b/fdtget.c
index 48ab615..2c384b6 100644
--- a/fdtget.c
+++ b/fdtget.c
@@ -77,9 +77,9 @@ static int show_data(struct display_info *disp, const char *data, int len)
 		return 0;
 	}
 	size = disp->size;
-	if (size == -1)
+	if (size == -1) {
 		size = (len % 4) == 0 ? 4 : 1;
-	else if (len % size) {
+	} else if (len % size) {
 		fprintf(stderr, "Property length must be a multiple of "
 				"selected data size\n");
 		return -1;
-- 
1.7.8.3

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

* [PATCH 04/10] Remove unused variable from test scripts
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (2 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 03/10] Trivial style fixup David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 05/10] Use 'trap' builtin to clean up temporaries in " David Gibson
                     ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Several of the test scripts remove $TMPFILE, without ever having set
the TMPFILE variable. This patch fixes it.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/dtc-checkfails.sh |    2 +-
 tests/fdtget-runtest.sh |    2 +-
 tests/fdtput-runtest.sh |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/dtc-checkfails.sh b/tests/dtc-checkfails.sh
index c58694f..e7aa25e 100755
--- a/tests/dtc-checkfails.sh
+++ b/tests/dtc-checkfails.sh
@@ -12,7 +12,7 @@ done
 
 LOG="tmp.log.$$"
 
-rm -f $TMPFILE $LOG
+rm -f $LOG
 
 verbose_run_log "$LOG" $VALGRIND "$DTC" -o /dev/null "$@"
 ret="$?"
diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
index f38184f..c17c8f9 100755
--- a/tests/fdtget-runtest.sh
+++ b/tests/fdtget-runtest.sh
@@ -5,7 +5,7 @@
 LOG="tmp.log.$$"
 EXPECT="tmp.expect.$$"
 
-rm -f $TMPFILE $LOG
+rm -f $LOG
 
 expect="$1"
 echo "$expect" >$EXPECT
diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
index ea51569..8e4cd95 100644
--- a/tests/fdtput-runtest.sh
+++ b/tests/fdtput-runtest.sh
@@ -11,7 +11,7 @@
 LOG="tmp.log.$$"
 EXPECT="tmp.expect.$$"
 
-rm -f $TMPFILE $LOG
+rm -f $LOG
 
 expect="$1"
 echo "$expect" >$EXPECT
-- 
1.7.8.3

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

* [PATCH 05/10] Use 'trap' builtin to clean up temporaries in test scripts
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (3 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 04/10] Remove unused variable from test scripts David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 06/10] Remove bashism from run_tests.sh David Gibson
                     ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Some of the test scripts create temporary files, which we remove at the
end.  Except that we usually forgot to remove them on some exit paths. To
avoid this problem in future, this modifies the scripts to use the shell's
trap 0 functionality to automatically remove the temporaries on any exit.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/dtc-checkfails.sh |    6 ++----
 tests/fdtget-runtest.sh |   10 ++++------
 tests/fdtput-runtest.sh |   10 ++++------
 3 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/tests/dtc-checkfails.sh b/tests/dtc-checkfails.sh
index e7aa25e..87992a0 100755
--- a/tests/dtc-checkfails.sh
+++ b/tests/dtc-checkfails.sh
@@ -10,9 +10,9 @@ for x; do
     CHECKS="$CHECKS $x"
 done
 
-LOG="tmp.log.$$"
-
+LOG=tmp.log.$$
 rm -f $LOG
+trap "rm -f $LOG" 0
 
 verbose_run_log "$LOG" $VALGRIND "$DTC" -o /dev/null "$@"
 ret="$?"
@@ -28,6 +28,4 @@ for c in $CHECKS; do
     fi
 done
 
-rm -f $LOG
-
 PASS
diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
index c17c8f9..44c3529 100755
--- a/tests/fdtget-runtest.sh
+++ b/tests/fdtget-runtest.sh
@@ -2,10 +2,10 @@
 
 . ./tests.sh
 
-LOG="tmp.log.$$"
-EXPECT="tmp.expect.$$"
-
-rm -f $LOG
+LOG=tmp.log.$$
+EXPECT=tmp.expect.$$
+rm -f $LOG $EXPECT
+trap "rm -f $LOG $EXPECT" 0
 
 expect="$1"
 echo "$expect" >$EXPECT
@@ -26,8 +26,6 @@ fi
 diff $EXPECT $LOG
 ret="$?"
 
-rm -f $LOG $EXPECT
-
 if [ "$ret" -eq 0 ]; then
 	PASS
 else
diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
index 8e4cd95..c4b2135 100644
--- a/tests/fdtput-runtest.sh
+++ b/tests/fdtput-runtest.sh
@@ -8,10 +8,10 @@
 
 . ./tests.sh
 
-LOG="tmp.log.$$"
-EXPECT="tmp.expect.$$"
-
-rm -f $LOG
+LOG=tmp.log.$$
+EXPECT=tmp.expect.$$
+rm -f $LOG $EXPECT
+trap "rm -f $LOG $EXPECT" 0
 
 expect="$1"
 echo "$expect" >$EXPECT
@@ -46,8 +46,6 @@ fi
 diff $EXPECT $LOG
 ret="$?"
 
-rm -f $LOG $EXPECT
-
 if [ "$ret" -eq 0 ]; then
 	PASS
 else
-- 
1.7.8.3

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

* [PATCH 06/10] Remove bashism from run_tests.sh
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (4 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 05/10] Use 'trap' builtin to clean up temporaries in " David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 07/10] Factor signal checks out of test scripts David Gibson
                     ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

The patches introducing fdtget and fdtput inserted a peculiar bashism to
run_tests.sh using non-portable assignment within an (( )) expression.
This patch fixes it.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/run_tests.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 2650559..37c173c 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -478,7 +478,7 @@ fdtput_tests () {
     tac $base | tr a-z A-Z | sort -r >$bigfile2
 
     # Allow just enough space for both file1 and file2
-    (( space = $(stat -c %s $file1) + $(stat -c %s $file2) ))
+    space=$(( $(stat -c %s $file1) + $(stat -c %s $file2) ))
     $DTC -O dtb -p $space -o $file ${file%.dtb}.dts 2>/dev/null
 
     # run_fdtput_test <test-name> <expected-result> <file> <key> <flags>
-- 
1.7.8.3

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

* [PATCH 07/10] Factor signal checks out of test scripts
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (5 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 06/10] Remove bashism from run_tests.sh David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 08/10] Clean up invocation of fdt{get,put} tests David Gibson
                     ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Several test scripts now have some code to check for a program returning
a signal, and reporting a suitable failure.  This patch moves this
duplicated code into a helper function in tests.sh.  At the same time we
remove a bashism found in the current copies (using the non portablr $[ ]
construct for arithmetic).

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/dtc-checkfails.sh |    5 +----
 tests/fdtget-runtest.sh |    5 +----
 tests/fdtput-runtest.sh |   11 +++--------
 tests/tests.sh          |    8 ++++++++
 4 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/tests/dtc-checkfails.sh b/tests/dtc-checkfails.sh
index 87992a0..3f77b13 100755
--- a/tests/dtc-checkfails.sh
+++ b/tests/dtc-checkfails.sh
@@ -17,10 +17,7 @@ trap "rm -f $LOG" 0
 verbose_run_log "$LOG" $VALGRIND "$DTC" -o /dev/null "$@"
 ret="$?"
 
-if [ "$ret" -gt 127 ]; then
-    signame=$(kill -l $[ret - 128])
-    FAIL "Killed by SIG$signame"
-fi
+FAIL_IF_SIGNAL $ret
 
 for c in $CHECKS; do
     if ! grep -E "^(ERROR)|(Warning) \($c\):" $LOG > /dev/null; then
diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
index 44c3529..42dc00c 100755
--- a/tests/fdtget-runtest.sh
+++ b/tests/fdtget-runtest.sh
@@ -18,10 +18,7 @@ if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
 	PASS
 fi
 
-if [ "$ret" -gt 127 ]; then
-    signame=$(kill -l $[ret - 128])
-    FAIL "Killed by SIG$signame"
-fi
+FAIL_IF_SIGNAL $ret
 
 diff $EXPECT $LOG
 ret="$?"
diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
index c4b2135..9178d2f 100644
--- a/tests/fdtput-runtest.sh
+++ b/tests/fdtput-runtest.sh
@@ -29,19 +29,14 @@ ret="$?"
 if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
 	PASS
 fi
-if [ "$ret" -gt 127 ]; then
-    signame=$(kill -l $[ret - 128])
-    FAIL "Killed by SIG$signame"
-fi
+
+FAIL_IF_SIGNAL $ret
 
 # Now fdtget to read the value
 verbose_run_log "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
 ret="$?"
 
-if [ "$ret" -gt 127 ]; then
-    signame=$(kill -l $[ret - 128])
-    FAIL "Killed by SIG$signame"
-fi
+FAIL_IF_SIGNAL $ret
 
 diff $EXPECT $LOG
 ret="$?"
diff --git a/tests/tests.sh b/tests/tests.sh
index 6e5e76a..3b7c6c8 100644
--- a/tests/tests.sh
+++ b/tests/tests.sh
@@ -10,6 +10,14 @@ FAIL () {
     exit 2
 }
 
+FAIL_IF_SIGNAL () {
+    ret="$1"
+    if [ "$ret" -gt 127 ]; then
+	signame=$(kill -l $((ret - 128)))
+	FAIL "Killed by SIG$signame"
+    fi
+}
+
 DTC=../dtc
 DTGET=../fdtget
 DTPUT=../fdtput
-- 
1.7.8.3

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

* [PATCH 08/10] Clean up invocation of fdt{get,put} tests
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (6 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 07/10] Factor signal checks out of test scripts David Gibson
@ 2012-02-03  5:12   ` David Gibson
       [not found]     ` <1328245929-15443-9-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
  2012-02-03  5:12   ` [PATCH 09/10] Don't use diff to check fdt{get,put} results David Gibson
                     ` (3 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

This patch cleans up how the fdtget and fdtput tests are invoked.
Specifically we no longer hide the full command lines with a wrapper
function - this makes it possible to distinguish fdtget from similar
fdtput tests and makes it easier to work out how to manually invoke an
individual failing test.

In addition, we remove the testing for errors from the
fdt{get,put}-runtest.sh script, instead using an internal wrapper
analagous to run_wrap_test which can test for any program invocation
that's expected to return an error.

For a couple of the fdtput tests this would result in printing out
ludicrously large command lines.  Therefore we introduce a new
mechanism to cut those down to something reasonable.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/fdtget-runtest.sh |   11 +---
 tests/fdtput-runtest.sh |   16 +----
 tests/run_tests.sh      |  144 ++++++++++++++++++++++++++---------------------
 tests/tests.sh          |   19 ++++++
 4 files changed, 104 insertions(+), 86 deletions(-)

diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
index 42dc00c..75e7503 100755
--- a/tests/fdtget-runtest.sh
+++ b/tests/fdtget-runtest.sh
@@ -8,17 +8,10 @@ rm -f $LOG $EXPECT
 trap "rm -f $LOG $EXPECT" 0
 
 expect="$1"
-echo "$expect" >$EXPECT
+echo $expect >$EXPECT
 shift
 
-verbose_run_log "$LOG" $VALGRIND "$DTGET" "$@"
-ret="$?"
-
-if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
-	PASS
-fi
-
-FAIL_IF_SIGNAL $ret
+verbose_run_log_check "$LOG" $VALGRIND $DTGET "$@"
 
 diff $EXPECT $LOG
 ret="$?"
diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
index 9178d2f..dbd9c0d 100644
--- a/tests/fdtput-runtest.sh
+++ b/tests/fdtput-runtest.sh
@@ -14,7 +14,7 @@ rm -f $LOG $EXPECT
 trap "rm -f $LOG $EXPECT" 0
 
 expect="$1"
-echo "$expect" >$EXPECT
+echo $expect >$EXPECT
 dtb="$2"
 node="$3"
 property="$4"
@@ -23,20 +23,10 @@ shift 5
 value="$@"
 
 # First run fdtput
-verbose_run $VALGRIND "$DTPUT" "$dtb" "$node" "$property" $value $flags
-ret="$?"
-
-if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
-	PASS
-fi
-
-FAIL_IF_SIGNAL $ret
+verbose_run_check $VALGRIND "$DTPUT" "$dtb" "$node" "$property" $value $flags
 
 # Now fdtget to read the value
-verbose_run_log "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
-ret="$?"
-
-FAIL_IF_SIGNAL $ret
+verbose_run_log_check "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
 
 diff $EXPECT $LOG
 ret="$?"
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 37c173c..999c882 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -36,6 +36,20 @@ base_run_test() {
     fi
 }
 
+shorten_echo () {
+    limit=32
+    echo -n "$1"
+    shift
+    for x; do
+	if [ ${#x} -le $limit ]; then
+	    echo -n " $x"
+	else
+	    short=$(echo "$x" | head -c$limit)
+	    echo -n " \"$short\"...<${#x} bytes>"
+	fi
+    done
+}
+
 run_test () {
     echo -n "$@:	"
     if [ -n "$VALGRIND" -a -f $1.supp ]; then
@@ -70,6 +84,28 @@ run_wrap_test () {
     base_run_test wrap_test "$@"
 }
 
+wrap_error () {
+    (
+	if verbose_run "$@"; then
+	    FAIL "Expected non-zero return code"
+	else
+	    ret="$?"
+	    if [ "$ret" -gt 127 ]; then
+		signame=$(kill -l $((ret - 128)))
+		FAIL "Killed by SIG$signame"
+	    else
+		PASS
+	    fi
+	fi
+    )
+}
+
+run_wrap_error_test () {
+    shorten_echo "$@"
+    echo -n " {!= 0}:	"
+    base_run_test wrap_error "$@"
+}
+
 run_dtc_test () {
     echo -n "dtc $@:	"
     base_run_test wrap_test $VALGRIND $DTC "$@"
@@ -84,25 +120,18 @@ asm_to_so_test () {
 }
 
 run_fdtget_test () {
-    # run_fdtget_test name expected_output dtb_file args...
-    echo -n "$1:	"
+    expect="$1"
     shift
-    base_run_test sh fdtget-runtest.sh "$@"
+    echo -n "fdtget-runtest.sh "$expect" $@:	"
+    base_run_test sh fdtget-runtest.sh "$expect" "$@"
 }
 
 run_fdtput_test () {
-    # run_fdtput_test name expected_output dtb_file node property flags value...
-    echo -n "$1:	"
+    expect="$1"
     shift
-    output="$1"
-    dtb="$2"
-    node="$3"
-    property="$4"
-    flags="$5"
-    shift 5
-    base_run_test sh fdtput-runtest.sh "$output" "$dtb" "$node" "$property" \
-		"$flags" $@
-#     base_run_test sh fdtput-runtest.sh "$@"
+    shorten_echo fdtput-runtest.sh "$expect" "$@"
+    echo -n ":	"
+    base_run_test sh fdtput-runtest.sh "$expect" "$@"
 }
 
 tree1_tests () {
@@ -425,39 +454,32 @@ dtbs_equal_tests () {
 }
 
 fdtget_tests () {
-    file=label01.dtb
-    $DTC -O dtb -o $file ${file%.dtb}.dts 2>/dev/null
+    dts=label01.dts
+    dtb=$dts.fdtget.test.dtb
+    run_dtc_test -O dtb -o $dtb $dts
 
-    # run_fdtget_test <test-name> <expected-result> <args>...
-    run_fdtget_test "Simple string" "MyBoardName" $file / model
-    run_fdtget_test "Multiple string i" "77 121 66 111 \
+    # run_fdtget_test <expected-result> [<flags>] <file> <node> <property>
+    run_fdtget_test "MyBoardName" $dtb / model
+    run_fdtget_test "77 121 66 111 \
 97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 \
-108 121 78 97 109 101 0" $file / compatible
-    run_fdtget_test "Multiple string s" "MyBoardName MyBoardFamilyName" \
-	-t s $file / compatible
-    run_fdtget_test "Integer" "32768" $file /cpus/PowerPC,970@1 d-cache-size
-    run_fdtget_test "Integer hex" "8000" -tx $file \
-	/cpus/PowerPC,970@1 d-cache-size
-    run_fdtget_test "Integer list" "61 62 63 0" -tbx $file \
-	/randomnode tricky1
-    run_fdtget_test "Byte list short" "a b c d de ea ad be ef" -tbx \
-	$file /randomnode blob
+108 121 78 97 109 101 0" $dtb / compatible
+    run_fdtget_test "MyBoardName MyBoardFamilyName" -t s $dtb / compatible
+    run_fdtget_test 32768 $dtb /cpus/PowerPC,970@1 d-cache-size
+    run_fdtget_test 8000 -tx $dtb /cpus/PowerPC,970@1 d-cache-size
+    run_fdtget_test "61 62 63 0" -tbx $dtb /randomnode tricky1
+    run_fdtget_test "a b c d de ea ad be ef" -tbx $dtb /randomnode blob
 
     # Here the property size is not a multiple of 4 bytes, so it should fail
-    run_fdtget_test "Integer list invalid" ERR -tlx \
-	$file /randomnode mixed
-    run_fdtget_test "Integer list halfword" "6162 6300 1234 0 a 0 b 0 c" -thx \
-	$file /randomnode mixed
-    run_fdtget_test "Integer list byte" \
-	"61 62 63 0 12 34 0 0 0 a 0 0 0 b 0 0 0 c" -thhx \
-	$file /randomnode mixed
-    run_fdtget_test "Missing property" ERR -ts \
-	$file /randomnode doctor-who
+    run_wrap_error_test $DTGET -tlx $dtb /randomnode mixed
+    run_fdtget_test "6162 6300 1234 0 a 0 b 0 c" -thx $dtb /randomnode mixed
+    run_fdtget_test "61 62 63 0 12 34 0 0 0 a 0 0 0 b 0 0 0 c" \
+	-thhx $dtb /randomnode mixed
+    run_wrap_error_test $DTGET -ts $dtb /randomnode doctor-who
 }
 
 fdtput_tests () {
-    file=label01.dtb
-    src=label01.dts
+    dts=label01.dts
+    dtb=$dts.fdtput.test.dtb
 
     # Create some test files containing useful strings
     base=tmp.test0
@@ -467,7 +489,7 @@ fdtput_tests () {
     bigfile2=tmp.test4
 
     # Filter out anything the shell might not like
-    cat $src | tr -d "'\"\n\;/\.\*{}\-" | tr -s "[:blank:]" " " >$base
+    cat $dts | tr -d "'\"\n\;/\.\*{}\-" | tr -s "[:blank:]" " " >$base
 
     # Make two small files
     head -5 $base >$file1
@@ -479,31 +501,25 @@ fdtput_tests () {
 
     # Allow just enough space for both file1 and file2
     space=$(( $(stat -c %s $file1) + $(stat -c %s $file2) ))
-    $DTC -O dtb -p $space -o $file ${file%.dtb}.dts 2>/dev/null
-
-    # run_fdtput_test <test-name> <expected-result> <file> <key> <flags>
-    #		<args>...
-    run_fdtput_test "Simple string" "a_model" $file / model -ts "a_model"
-    run_fdtput_test "Multiple string s" "board1 board2" \
-	$file / compatible -ts board1 board2
-    run_fdtput_test "Single string with spaces" "board1 board2" \
-	$file / compatible -ts "board1 board2"
-    run_fdtput_test "Integer" "32768" \
-	$file /cpus/PowerPC,970@1 d-cache-size "" "32768"
-    run_fdtput_test "Integer hex" "8001" \
-	$file /cpus/PowerPC,970@1 d-cache-size -tx 0x8001
-    run_fdtput_test "Integer list" "2 3 12" \
-	$file /randomnode tricky1 -tbi "02 003 12"
-    run_fdtput_test "Byte list short" "a b c ea ad be ef" \
-	$file /randomnode blob -tbx "a b c ea ad be ef"
-    run_fdtput_test "Integer list short" "a0b0c0d deeaae ef000000" \
-	$file /randomnode blob -tx "a0b0c0d deeaae ef000000"
-    run_fdtput_test "Large string list" "`cat $file1 $file2`" \
-	$file /randomnode blob -ts "`cat $file1`" "`cat $file2`"
+    run_dtc_test -O dtb -p $space -o $dtb $dts
+
+    # run_fdtput_test <expected-result> <file> <node> <property> <flags> <value>
+    run_fdtput_test "a_model" $dtb / model -ts "a_model"
+    run_fdtput_test "board1 board2" $dtb / compatible -ts board1 board2
+    run_fdtput_test "board1 board2" $dtb / compatible -ts "board1 board2"
+    run_fdtput_test "32768" $dtb /cpus/PowerPC,970@1 d-cache-size "" "32768"
+    run_fdtput_test "8001" $dtb /cpus/PowerPC,970@1 d-cache-size -tx 0x8001
+    run_fdtput_test "2 3 12" $dtb /randomnode tricky1 -tbi "02 003 12"
+    run_fdtput_test "a b c ea ad be ef" $dtb /randomnode blob \
+	-tbx "a b c ea ad be ef"
+    run_fdtput_test "a0b0c0d deeaae ef000000" $dtb /randomnode blob \
+	-tx "a0b0c0d deeaae ef000000"
+    run_fdtput_test "`cat $file1 $file2`" $dtb /randomnode blob \
+	-ts "`cat $file1`" "`cat $file2`"
 
     # This should be larger than available space in the fdt ($space)
-    run_fdtput_test "Enormous string list" ERR \
-	$file /randomnode blob -ts "`cat $bigfile1`" "`cat $bigfile2`"
+    run_wrap_error_test $DTPUT $dtb /randomnode blob \
+	-ts "`cat $bigfile1`" "`cat $bigfile2`"
 
     # TODO: Add tests for verbose mode?
 }
diff --git a/tests/tests.sh b/tests/tests.sh
index 3b7c6c8..31530d5 100644
--- a/tests/tests.sh
+++ b/tests/tests.sh
@@ -30,6 +30,15 @@ verbose_run () {
     fi
 }
 
+verbose_run_check () {
+    verbose_run "$@"
+    ret="$?"
+    FAIL_IF_SIGNAL $ret
+    if [ $ret != 0 ]; then
+	FAIL "Returned error code $ret"
+    fi
+}
+
 verbose_run_log () {
     LOG="$1"
     shift
@@ -40,3 +49,13 @@ verbose_run_log () {
     fi
     return $ret
 }
+
+verbose_run_log_check () {
+    verbose_run_log "$@"
+    ret="$?"
+    FAIL_IF_SIGNAL $ret
+    if [ $ret != 0 ]; then
+	FAIL "Returned error code $ret"
+    fi
+}
+
-- 
1.7.8.3

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

* [PATCH 09/10] Don't use diff to check fdt{get,put} results
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (7 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 08/10] Clean up invocation of fdt{get,put} tests David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03  5:12   ` [PATCH 10/10] Generate test data for fdtput more sensibly David Gibson
                     ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Currently the fdt{get,put}-runtest.sh scripts invoke diff to check if
fdt{get,put} did the right thing.  This isn't great though: it's not
obvious from the diff output which is the expected and which is the
actual result; diff's line by line behaviour is useless here, since all
the results are a single line and finally, when there is a difference
it always prints information even when the tests are supposed to be
running in quiet mode.

This patch uses cmp instead, and explicitly prints the expected results,
when running in verbose mode (the invocation of fdtget itself will have
already displayed the actual results in this mode.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/fdtget-runtest.sh |   13 +++++++------
 tests/fdtput-runtest.sh |   13 +++++++------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
index 75e7503..dac7f9a 100755
--- a/tests/fdtget-runtest.sh
+++ b/tests/fdtget-runtest.sh
@@ -13,11 +13,12 @@ shift
 
 verbose_run_log_check "$LOG" $VALGRIND $DTGET "$@"
 
-diff $EXPECT $LOG
-ret="$?"
-
-if [ "$ret" -eq 0 ]; then
-	PASS
+if cmp $EXPECT $LOG>/dev/null; then
+    PASS
 else
-	FAIL
+    if [ -z "$QUIET_TEST" ]; then
+	echo "EXPECTED :-:"
+	cat $EXPECT
+    fi
+    FAIL "Results differ from expected"
 fi
diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
index dbd9c0d..527a968 100644
--- a/tests/fdtput-runtest.sh
+++ b/tests/fdtput-runtest.sh
@@ -28,11 +28,12 @@ verbose_run_check $VALGRIND "$DTPUT" "$dtb" "$node" "$property" $value $flags
 # Now fdtget to read the value
 verbose_run_log_check "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
 
-diff $EXPECT $LOG
-ret="$?"
-
-if [ "$ret" -eq 0 ]; then
-	PASS
+if cmp $EXPECT $LOG >/dev/null; then
+    PASS
 else
-	FAIL
+    if [ -z "$QUIET_TEST" ]; then
+	echo "EXPECTED :-:"
+	cat $EXPECT
+    fi
+    FAIL "Results differ from expected"
 fi
-- 
1.7.8.3

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

* [PATCH 10/10] Generate test data for fdtput more sensibly
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (8 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 09/10] Don't use diff to check fdt{get,put} results David Gibson
@ 2012-02-03  5:12   ` David Gibson
  2012-02-03 14:45   ` [0/10] Assorted cleanups (esp for fdt{get,put} tests) Jon Loeliger
  2012-02-03 18:22   ` Simon Glass
  11 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-03  5:12 UTC (permalink / raw)
  To: jdl-CYoMK+44s/E; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Currently run_tests.sh generates several files of text test data.  The
procedure it uses for this is somewhat torturous and has several problems:
 * Since the test data is derived from a dts file, a cursory glance at the
test output suggests something is wrong with the processing of that dts.
This is misleading since in fact it's just being used as an arbirary
string.
 * Since the base input has linefeeds removed, the head and sort commands
used later have no effect.
 * Although an attempt is made to get rid of characters which the shell
will mangle, it's not thorough enough.  Specifically it leaves in \ which
means that some string escapes found in the input data can get expanded
somewhere along the line in some shells.

This patch, therefore, replaces this generation of test data with a
pre-canned "Lorem ipsum" of approximately 2k.  On my system, where /bin/sh
is dash, this fixes a test failure due to the aforementioned string
escapes being evaluated on one but not the other of the two comparison
paths (I haven't tracked down exactly where the expansion is happening).

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 tests/lorem.txt    |   35 +++++++++++++++++++++++++++++++++++
 tests/run_tests.sh |   32 ++++++--------------------------
 2 files changed, 41 insertions(+), 26 deletions(-)
 create mode 100644 tests/lorem.txt

diff --git a/tests/lorem.txt b/tests/lorem.txt
new file mode 100644
index 0000000..acff698
--- /dev/null
+++ b/tests/lorem.txt
@@ -0,0 +1,35 @@
+Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris eros
+arcu, egestas non pellentesque non, euismod eu nibh. Proin arcu metus,
+dapibus vitae sodales rhoncus, suscipit vel nulla. Etiam lorem est,
+aliquam ut fringilla sit amet, condimentum et quam. Duis eu arcu odio,
+at pulvinar nisi. Morbi condimentum eros ut turpis rhoncus
+pharetra. Pellentesque habitant morbi tristique senectus et netus et
+malesuada fames ac turpis egestas. Nam et nulla enim. Etiam fringilla
+eleifend neque, at posuere ante lacinia a. Duis orci tortor, dictum ac
+gravida ac, euismod eu leo. Sed eget dolor tortor. Pellentesque
+venenatis, lectus eu vulputate porta, libero ipsum convallis mi, sit
+amet vehicula arcu elit sit amet odio.
+
+Fusce iaculis massa metus, id sagittis diam. Praesent molestie ante
+vel odio tincidunt auctor. Cum sociis natoque penatibus et magnis dis
+parturient montes, nascetur ridiculus mus. Duis rutrum vehicula nisl
+eget condimentum. In in justo nisl. Nullam id arcu at nisl eleifend
+pretium. Nulla interdum ligula id elit mollis dictum a sit amet
+quam. Nullam iaculis laoreet ipsum at tempus. Vestibulum in cursus
+dui. Curabitur porta lectus eget urna bibendum congue eget elementum
+nisi. Proin sit amet lectus ut neque iaculis consectetur eu sit amet
+nibh. Maecenas rhoncus dolor ac nunc gravida blandit. Fusce sem felis,
+aliquam a porttitor a, porta quis odio.
+
+Nunc purus lorem, sollicitudin non ultricies id, porta vitae
+enim. Nulla tristique gravida leo ut suscipit. Phasellus vitae turpis
+libero. Proin ac purus dolor, in suscipit magna. Sed et enim
+arcu. Morbi semper aliquet suscipit. Aenean laoreet condimentum massa,
+eu pharetra magna fermentum ut. Morbi euismod convallis tortor, eget
+fringilla lacus sagittis non. Nullam bibendum posuere feugiat.
+
+In at pulvinar massa. Mauris nunc lectus, mollis et malesuada
+pharetra, cursus sed lacus. Integer dolor urna, interdum a mollis at,
+vestibulum non nisl. Sed in urna tortor. Mauris arcu felis, volutpat
+quis euismod vel, congue sit amet ipsum. Morbi in aliquet purus. Duis
+cras amet.
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 999c882..a561433 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -480,28 +480,10 @@ fdtget_tests () {
 fdtput_tests () {
     dts=label01.dts
     dtb=$dts.fdtput.test.dtb
+    text=lorem.txt
 
-    # Create some test files containing useful strings
-    base=tmp.test0
-    file1=tmp.test1
-    file2=tmp.test2
-    bigfile1=tmp.test3
-    bigfile2=tmp.test4
-
-    # Filter out anything the shell might not like
-    cat $dts | tr -d "'\"\n\;/\.\*{}\-" | tr -s "[:blank:]" " " >$base
-
-    # Make two small files
-    head -5 $base >$file1
-    cat $file1 | tr a-z A-Z | cut -c10-30 | sort -r >$file2
-
-    # and two larger ones
-    cat $base > $bigfile1
-    tac $base | tr a-z A-Z | sort -r >$bigfile2
-
-    # Allow just enough space for both file1 and file2
-    space=$(( $(stat -c %s $file1) + $(stat -c %s $file2) ))
-    run_dtc_test -O dtb -p $space -o $dtb $dts
+    # Allow just enough space for $text
+    run_dtc_test -O dtb -p $(stat -c %s $text) -o $dtb $dts
 
     # run_fdtput_test <expected-result> <file> <node> <property> <flags> <value>
     run_fdtput_test "a_model" $dtb / model -ts "a_model"
@@ -514,12 +496,10 @@ fdtput_tests () {
 	-tbx "a b c ea ad be ef"
     run_fdtput_test "a0b0c0d deeaae ef000000" $dtb /randomnode blob \
 	-tx "a0b0c0d deeaae ef000000"
-    run_fdtput_test "`cat $file1 $file2`" $dtb /randomnode blob \
-	-ts "`cat $file1`" "`cat $file2`"
+    run_fdtput_test "$(cat $text)" $dtb /randomnode blob -ts "$(cat $text)"
 
-    # This should be larger than available space in the fdt ($space)
-    run_wrap_error_test $DTPUT $dtb /randomnode blob \
-	-ts "`cat $bigfile1`" "`cat $bigfile2`"
+    # This should be larger than available space in the fdt
+    run_wrap_error_test $DTPUT $dtb /randomnode blob -ts "$(cat $text $text)"
 
     # TODO: Add tests for verbose mode?
 }
-- 
1.7.8.3

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

* Re: [0/10] Assorted cleanups (esp for fdt{get,put} tests)
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (9 preceding siblings ...)
  2012-02-03  5:12   ` [PATCH 10/10] Generate test data for fdtput more sensibly David Gibson
@ 2012-02-03 14:45   ` Jon Loeliger
  2012-02-03 18:22   ` Simon Glass
  11 siblings, 0 replies; 14+ messages in thread
From: Jon Loeliger @ 2012-02-03 14:45 UTC (permalink / raw)
  To: David Gibson; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

> As noted earlier, there were a few things I wasn't totally happy with
> in the recently applied fdt{get,put} patches, mostly in the test code.
> I had a look at cleaning up those problems today, which kind of
> expanded into a whole bunch of little cleanups for tests and a few
> other things.
> 
> So, here they all are.  Jon, please apply.

Whole series applied and pushed out.

Thanks,
jdl

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

* Re: [PATCH 08/10] Clean up invocation of fdt{get,put} tests
       [not found]     ` <1328245929-15443-9-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
@ 2012-02-03 15:47       ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2012-02-03 15:47 UTC (permalink / raw)
  To: David Gibson; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Hi David,

On Thu, Feb 2, 2012 at 9:12 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> This patch cleans up how the fdtget and fdtput tests are invoked.
> Specifically we no longer hide the full command lines with a wrapper
> function - this makes it possible to distinguish fdtget from similar
> fdtput tests and makes it easier to work out how to manually invoke an
> individual failing test.
>
> In addition, we remove the testing for errors from the
> fdt{get,put}-runtest.sh script, instead using an internal wrapper
> analagous to run_wrap_test which can test for any program invocation
> that's expected to return an error.
>
> For a couple of the fdtput tests this would result in printing out
> ludicrously large command lines.  Therefore we introduce a new
> mechanism to cut those down to something reasonable.
>
> Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> ---
>  tests/fdtget-runtest.sh |   11 +---
>  tests/fdtput-runtest.sh |   16 +----
>  tests/run_tests.sh      |  144 ++++++++++++++++++++++++++---------------------
>  tests/tests.sh          |   19 ++++++
>  4 files changed, 104 insertions(+), 86 deletions(-)

Well after 15mins of reading through it looks good to me. Would be
easier to understand as three commits I think:

- changing arg order for run_fdtget/put_test
- the shorten_echo feature
- moving to verbose_run_log_check

Thanks for tidying this up. I was not happy with the test invocation
setup I had (and the ugliness of shell variables / quoting /
arguments) and moving the argument to the front makes this much
cleaner.

Regards,
Simon
>
> diff --git a/tests/fdtget-runtest.sh b/tests/fdtget-runtest.sh
> index 42dc00c..75e7503 100755
> --- a/tests/fdtget-runtest.sh
> +++ b/tests/fdtget-runtest.sh
> @@ -8,17 +8,10 @@ rm -f $LOG $EXPECT
>  trap "rm -f $LOG $EXPECT" 0
>
>  expect="$1"
> -echo "$expect" >$EXPECT
> +echo $expect >$EXPECT
>  shift
>
> -verbose_run_log "$LOG" $VALGRIND "$DTGET" "$@"
> -ret="$?"
> -
> -if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
> -       PASS
> -fi
> -
> -FAIL_IF_SIGNAL $ret
> +verbose_run_log_check "$LOG" $VALGRIND $DTGET "$@"
>
>  diff $EXPECT $LOG
>  ret="$?"
> diff --git a/tests/fdtput-runtest.sh b/tests/fdtput-runtest.sh
> index 9178d2f..dbd9c0d 100644
> --- a/tests/fdtput-runtest.sh
> +++ b/tests/fdtput-runtest.sh
> @@ -14,7 +14,7 @@ rm -f $LOG $EXPECT
>  trap "rm -f $LOG $EXPECT" 0
>
>  expect="$1"
> -echo "$expect" >$EXPECT
> +echo $expect >$EXPECT
>  dtb="$2"
>  node="$3"
>  property="$4"
> @@ -23,20 +23,10 @@ shift 5
>  value="$@"
>
>  # First run fdtput
> -verbose_run $VALGRIND "$DTPUT" "$dtb" "$node" "$property" $value $flags
> -ret="$?"
> -
> -if [ "$ret" -ne 0 -a "$expect" = "ERR" ]; then
> -       PASS
> -fi
> -
> -FAIL_IF_SIGNAL $ret
> +verbose_run_check $VALGRIND "$DTPUT" "$dtb" "$node" "$property" $value $flags
>
>  # Now fdtget to read the value
> -verbose_run_log "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
> -ret="$?"
> -
> -FAIL_IF_SIGNAL $ret
> +verbose_run_log_check "$LOG" $VALGRIND "$DTGET" "$dtb" "$node" "$property" $flags
>
>  diff $EXPECT $LOG
>  ret="$?"
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 37c173c..999c882 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -36,6 +36,20 @@ base_run_test() {
>     fi
>  }
>
> +shorten_echo () {
> +    limit=32
> +    echo -n "$1"
> +    shift
> +    for x; do
> +       if [ ${#x} -le $limit ]; then
> +           echo -n " $x"
> +       else
> +           short=$(echo "$x" | head -c$limit)
> +           echo -n " \"$short\"...<${#x} bytes>"
> +       fi
> +    done
> +}
> +
>  run_test () {
>     echo -n "$@:       "
>     if [ -n "$VALGRIND" -a -f $1.supp ]; then
> @@ -70,6 +84,28 @@ run_wrap_test () {
>     base_run_test wrap_test "$@"
>  }
>
> +wrap_error () {
> +    (
> +       if verbose_run "$@"; then
> +           FAIL "Expected non-zero return code"
> +       else
> +           ret="$?"
> +           if [ "$ret" -gt 127 ]; then
> +               signame=$(kill -l $((ret - 128)))
> +               FAIL "Killed by SIG$signame"
> +           else
> +               PASS
> +           fi
> +       fi
> +    )
> +}
> +
> +run_wrap_error_test () {
> +    shorten_echo "$@"
> +    echo -n " {!= 0}:  "
> +    base_run_test wrap_error "$@"
> +}
> +
>  run_dtc_test () {
>     echo -n "dtc $@:   "
>     base_run_test wrap_test $VALGRIND $DTC "$@"
> @@ -84,25 +120,18 @@ asm_to_so_test () {
>  }
>
>  run_fdtget_test () {
> -    # run_fdtget_test name expected_output dtb_file args...
> -    echo -n "$1:       "
> +    expect="$1"
>     shift
> -    base_run_test sh fdtget-runtest.sh "$@"
> +    echo -n "fdtget-runtest.sh "$expect" $@:   "
> +    base_run_test sh fdtget-runtest.sh "$expect" "$@"
>  }
>
>  run_fdtput_test () {
> -    # run_fdtput_test name expected_output dtb_file node property flags value...
> -    echo -n "$1:       "
> +    expect="$1"
>     shift
> -    output="$1"
> -    dtb="$2"
> -    node="$3"
> -    property="$4"
> -    flags="$5"
> -    shift 5
> -    base_run_test sh fdtput-runtest.sh "$output" "$dtb" "$node" "$property" \
> -               "$flags" $@
> -#     base_run_test sh fdtput-runtest.sh "$@"
> +    shorten_echo fdtput-runtest.sh "$expect" "$@"
> +    echo -n ": "
> +    base_run_test sh fdtput-runtest.sh "$expect" "$@"
>  }
>
>  tree1_tests () {
> @@ -425,39 +454,32 @@ dtbs_equal_tests () {
>  }
>
>  fdtget_tests () {
> -    file=label01.dtb
> -    $DTC -O dtb -o $file ${file%.dtb}.dts 2>/dev/null
> +    dts=label01.dts
> +    dtb=$dts.fdtget.test.dtb
> +    run_dtc_test -O dtb -o $dtb $dts
>
> -    # run_fdtget_test <test-name> <expected-result> <args>...
> -    run_fdtget_test "Simple string" "MyBoardName" $file / model
> -    run_fdtget_test "Multiple string i" "77 121 66 111 \
> +    # run_fdtget_test <expected-result> [<flags>] <file> <node> <property>
> +    run_fdtget_test "MyBoardName" $dtb / model
> +    run_fdtget_test "77 121 66 111 \
>  97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 \
> -108 121 78 97 109 101 0" $file / compatible
> -    run_fdtget_test "Multiple string s" "MyBoardName MyBoardFamilyName" \
> -       -t s $file / compatible
> -    run_fdtget_test "Integer" "32768" $file /cpus/PowerPC,970@1 d-cache-size
> -    run_fdtget_test "Integer hex" "8000" -tx $file \
> -       /cpus/PowerPC,970@1 d-cache-size
> -    run_fdtget_test "Integer list" "61 62 63 0" -tbx $file \
> -       /randomnode tricky1
> -    run_fdtget_test "Byte list short" "a b c d de ea ad be ef" -tbx \
> -       $file /randomnode blob
> +108 121 78 97 109 101 0" $dtb / compatible
> +    run_fdtget_test "MyBoardName MyBoardFamilyName" -t s $dtb / compatible
> +    run_fdtget_test 32768 $dtb /cpus/PowerPC,970@1 d-cache-size
> +    run_fdtget_test 8000 -tx $dtb /cpus/PowerPC,970@1 d-cache-size
> +    run_fdtget_test "61 62 63 0" -tbx $dtb /randomnode tricky1
> +    run_fdtget_test "a b c d de ea ad be ef" -tbx $dtb /randomnode blob
>
>     # Here the property size is not a multiple of 4 bytes, so it should fail
> -    run_fdtget_test "Integer list invalid" ERR -tlx \
> -       $file /randomnode mixed
> -    run_fdtget_test "Integer list halfword" "6162 6300 1234 0 a 0 b 0 c" -thx \
> -       $file /randomnode mixed
> -    run_fdtget_test "Integer list byte" \
> -       "61 62 63 0 12 34 0 0 0 a 0 0 0 b 0 0 0 c" -thhx \
> -       $file /randomnode mixed
> -    run_fdtget_test "Missing property" ERR -ts \
> -       $file /randomnode doctor-who
> +    run_wrap_error_test $DTGET -tlx $dtb /randomnode mixed
> +    run_fdtget_test "6162 6300 1234 0 a 0 b 0 c" -thx $dtb /randomnode mixed
> +    run_fdtget_test "61 62 63 0 12 34 0 0 0 a 0 0 0 b 0 0 0 c" \
> +       -thhx $dtb /randomnode mixed
> +    run_wrap_error_test $DTGET -ts $dtb /randomnode doctor-who
>  }
>
>  fdtput_tests () {
> -    file=label01.dtb
> -    src=label01.dts
> +    dts=label01.dts
> +    dtb=$dts.fdtput.test.dtb
>
>     # Create some test files containing useful strings
>     base=tmp.test0
> @@ -467,7 +489,7 @@ fdtput_tests () {
>     bigfile2=tmp.test4
>
>     # Filter out anything the shell might not like
> -    cat $src | tr -d "'\"\n\;/\.\*{}\-" | tr -s "[:blank:]" " " >$base
> +    cat $dts | tr -d "'\"\n\;/\.\*{}\-" | tr -s "[:blank:]" " " >$base
>
>     # Make two small files
>     head -5 $base >$file1
> @@ -479,31 +501,25 @@ fdtput_tests () {
>
>     # Allow just enough space for both file1 and file2
>     space=$(( $(stat -c %s $file1) + $(stat -c %s $file2) ))
> -    $DTC -O dtb -p $space -o $file ${file%.dtb}.dts 2>/dev/null
> -
> -    # run_fdtput_test <test-name> <expected-result> <file> <key> <flags>
> -    #          <args>...
> -    run_fdtput_test "Simple string" "a_model" $file / model -ts "a_model"
> -    run_fdtput_test "Multiple string s" "board1 board2" \
> -       $file / compatible -ts board1 board2
> -    run_fdtput_test "Single string with spaces" "board1 board2" \
> -       $file / compatible -ts "board1 board2"
> -    run_fdtput_test "Integer" "32768" \
> -       $file /cpus/PowerPC,970@1 d-cache-size "" "32768"
> -    run_fdtput_test "Integer hex" "8001" \
> -       $file /cpus/PowerPC,970@1 d-cache-size -tx 0x8001
> -    run_fdtput_test "Integer list" "2 3 12" \
> -       $file /randomnode tricky1 -tbi "02 003 12"
> -    run_fdtput_test "Byte list short" "a b c ea ad be ef" \
> -       $file /randomnode blob -tbx "a b c ea ad be ef"
> -    run_fdtput_test "Integer list short" "a0b0c0d deeaae ef000000" \
> -       $file /randomnode blob -tx "a0b0c0d deeaae ef000000"
> -    run_fdtput_test "Large string list" "`cat $file1 $file2`" \
> -       $file /randomnode blob -ts "`cat $file1`" "`cat $file2`"
> +    run_dtc_test -O dtb -p $space -o $dtb $dts
> +
> +    # run_fdtput_test <expected-result> <file> <node> <property> <flags> <value>
> +    run_fdtput_test "a_model" $dtb / model -ts "a_model"
> +    run_fdtput_test "board1 board2" $dtb / compatible -ts board1 board2
> +    run_fdtput_test "board1 board2" $dtb / compatible -ts "board1 board2"
> +    run_fdtput_test "32768" $dtb /cpus/PowerPC,970@1 d-cache-size "" "32768"
> +    run_fdtput_test "8001" $dtb /cpus/PowerPC,970@1 d-cache-size -tx 0x8001
> +    run_fdtput_test "2 3 12" $dtb /randomnode tricky1 -tbi "02 003 12"
> +    run_fdtput_test "a b c ea ad be ef" $dtb /randomnode blob \
> +       -tbx "a b c ea ad be ef"
> +    run_fdtput_test "a0b0c0d deeaae ef000000" $dtb /randomnode blob \
> +       -tx "a0b0c0d deeaae ef000000"
> +    run_fdtput_test "`cat $file1 $file2`" $dtb /randomnode blob \
> +       -ts "`cat $file1`" "`cat $file2`"
>
>     # This should be larger than available space in the fdt ($space)
> -    run_fdtput_test "Enormous string list" ERR \
> -       $file /randomnode blob -ts "`cat $bigfile1`" "`cat $bigfile2`"
> +    run_wrap_error_test $DTPUT $dtb /randomnode blob \
> +       -ts "`cat $bigfile1`" "`cat $bigfile2`"
>
>     # TODO: Add tests for verbose mode?
>  }
> diff --git a/tests/tests.sh b/tests/tests.sh
> index 3b7c6c8..31530d5 100644
> --- a/tests/tests.sh
> +++ b/tests/tests.sh
> @@ -30,6 +30,15 @@ verbose_run () {
>     fi
>  }
>
> +verbose_run_check () {
> +    verbose_run "$@"
> +    ret="$?"
> +    FAIL_IF_SIGNAL $ret
> +    if [ $ret != 0 ]; then
> +       FAIL "Returned error code $ret"
> +    fi
> +}
> +
>  verbose_run_log () {
>     LOG="$1"
>     shift
> @@ -40,3 +49,13 @@ verbose_run_log () {
>     fi
>     return $ret
>  }
> +
> +verbose_run_log_check () {
> +    verbose_run_log "$@"
> +    ret="$?"
> +    FAIL_IF_SIGNAL $ret
> +    if [ $ret != 0 ]; then
> +       FAIL "Returned error code $ret"
> +    fi
> +}
> +
> --
> 1.7.8.3
>

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

* Re: [0/10] Assorted cleanups (esp for fdt{get,put} tests)
       [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
                     ` (10 preceding siblings ...)
  2012-02-03 14:45   ` [0/10] Assorted cleanups (esp for fdt{get,put} tests) Jon Loeliger
@ 2012-02-03 18:22   ` Simon Glass
  11 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2012-02-03 18:22 UTC (permalink / raw)
  To: David Gibson; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Hi David,

On Thu, Feb 2, 2012 at 9:11 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> As noted earlier, there were a few things I wasn't totally happy with
> in the recently applied fdt{get,put} patches, mostly in the test code.
> I had a look at cleaning up those problems today, which kind of
> expanded into a whole bunch of little cleanups for tests and a few
> other things.
>
> So, here they all are.  Jon, please apply.
>

Thanks for doing this. It would have been a few weeks before I got to
it, and still would surely have required several rounds to get to a
point where you were comfortable with it, anyway.

Regards,
Simon

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

end of thread, other threads:[~2012-02-03 18:22 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-03  5:11 [0/10] Assorted cleanups (esp for fdt{get,put} tests) David Gibson
     [not found] ` <1328245929-15443-1-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
2012-02-03  5:12   ` [PATCH 01/10] Update .gitignore for tests David Gibson
2012-02-03  5:12   ` [PATCH 02/10] Add quilt files to .gitignore David Gibson
2012-02-03  5:12   ` [PATCH 03/10] Trivial style fixup David Gibson
2012-02-03  5:12   ` [PATCH 04/10] Remove unused variable from test scripts David Gibson
2012-02-03  5:12   ` [PATCH 05/10] Use 'trap' builtin to clean up temporaries in " David Gibson
2012-02-03  5:12   ` [PATCH 06/10] Remove bashism from run_tests.sh David Gibson
2012-02-03  5:12   ` [PATCH 07/10] Factor signal checks out of test scripts David Gibson
2012-02-03  5:12   ` [PATCH 08/10] Clean up invocation of fdt{get,put} tests David Gibson
     [not found]     ` <1328245929-15443-9-git-send-email-david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
2012-02-03 15:47       ` Simon Glass
2012-02-03  5:12   ` [PATCH 09/10] Don't use diff to check fdt{get,put} results David Gibson
2012-02-03  5:12   ` [PATCH 10/10] Generate test data for fdtput more sensibly David Gibson
2012-02-03 14:45   ` [0/10] Assorted cleanups (esp for fdt{get,put} tests) Jon Loeliger
2012-02-03 18:22   ` Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).