* [PATCH 0/4 v2] xfstests: extended names fixes
@ 2016-07-11 9:26 Jan Tulak
2016-07-11 9:26 ` [PATCH 1/4] xfstests: Fix installation for extended names Jan Tulak
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Jan Tulak @ 2016-07-11 9:26 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
Some fixes for extended names in tests (tests/baz/123-foo-bar). This version
has more patches than previous, because I split the second patch into three.
Jan Tulak (4):
xfstests: Fix installation for extended names
fstests: filename handling for extended names in ./check was on a
wrong place
xfstests: remove unused variable
xfstests: filename handling - fix early wildcard expansion
check | 26 +++++++++++++-------------
include/buildrules | 31 +++++++++++++++++++++++++++++++
tests/btrfs/Makefile | 4 ++--
tests/cifs/Makefile | 4 ++--
tests/ext4/Makefile | 4 ++--
tests/f2fs/Makefile | 4 ++--
tests/generic/Makefile | 4 ++--
tests/overlay/Makefile | 4 ++--
tests/shared/Makefile | 4 ++--
tests/udf/Makefile | 4 ++--
tests/xfs/Makefile | 4 ++--
11 files changed, 62 insertions(+), 31 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] xfstests: Fix installation for extended names
2016-07-11 9:26 [PATCH 0/4 v2] xfstests: extended names fixes Jan Tulak
@ 2016-07-11 9:26 ` Jan Tulak
2016-07-15 10:05 ` Eryu Guan
2016-07-11 9:26 ` [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Jan Tulak @ 2016-07-11 9:26 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
xfstests supports extended test names like 314-foo-bar, but installation of
these tests was skipped (not matching a regexp). So this patch fixes the
makefiles in tests/*/
The include/buildrules change was written by Dave Chinner.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
UPDATE:
Use Dave's code to at first select all files and then separate output and test
files. BTW, in this version, when most of this version is written by Dave,
should I add Signed-off-by, or is better to keep it mentioned only in the text?
---
include/buildrules | 31 +++++++++++++++++++++++++++++++
tests/btrfs/Makefile | 4 ++--
tests/cifs/Makefile | 4 ++--
tests/ext4/Makefile | 4 ++--
tests/f2fs/Makefile | 4 ++--
tests/generic/Makefile | 4 ++--
tests/overlay/Makefile | 4 ++--
tests/shared/Makefile | 4 ++--
tests/udf/Makefile | 4 ++--
tests/xfs/Makefile | 4 ++--
10 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/include/buildrules b/include/buildrules
index c8a7c47..1269c8c 100644
--- a/include/buildrules
+++ b/include/buildrules
@@ -100,3 +100,34 @@ MAKEDEP := $(MAKEDEPEND) $(CFLAGS)
cp /dev/null .dep; \
fi
+# Gather files for installing into two lists:
+# TESTS with executable scripts and OUTFILES with all the test outputs.
+# Makefile has a very small matching, so we have to go this long way.
+
+# Start with all test related files:
+ALLFILES = $(wildcard [0-9]??*)
+
+# Now build a list of known output files. Unfortunately, the
+# multiple output test files are poorly handled as makefiles don't
+# handle wildcarded multi-suffix matching. Hence we separate the
+# processing of these right now.
+EXTENDED_OUTFILES = $(wildcard [0-9]??*.out.*)
+BASIC_OUTFILES = $(wildcard [0-9]??*.out)
+OUTFILES = $(EXTENDED_OUTFILES) $(BASIC_OUTFILES)
+
+# Strip suffix to get matching tests. We want to strip from the
+# first "." to the end, but makefiles don't have a built in
+# operative for that. So:
+#
+# Hack: strip the multiple segments after .out with repeated basename calls.
+EXTFILTER1 = $(basename $(EXTENDED_OUTFILES))
+EXTFILTER2 = $(basename $(EXTFILTER1))
+EXTFILTER3 = $(basename $(EXTFILTER2))
+EXTFILTER4 = $(basename $(EXTFILTER3))
+
+# final filter list
+FILTER = $(basename $(EXTFILTER4) $(BASIC_OUTFILES))
+
+# finally, select the test files by filtering against against the
+# stripped output files and sort them to remove duplicates.
+TESTS = $(sort $(filter $(ALLFILES), $(FILTER)))
diff --git a/tests/btrfs/Makefile b/tests/btrfs/Makefile
index e1a5be1..2d93642 100644
--- a/tests/btrfs/Makefile
+++ b/tests/btrfs/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/cifs/Makefile b/tests/cifs/Makefile
index 9176e5c..0c5cf3b 100644
--- a/tests/cifs/Makefile
+++ b/tests/cifs/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/ext4/Makefile b/tests/ext4/Makefile
index 7a3c8e1..beb1541 100644
--- a/tests/ext4/Makefile
+++ b/tests/ext4/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/f2fs/Makefile b/tests/f2fs/Makefile
index 4d00e9e..d13bca3 100644
--- a/tests/f2fs/Makefile
+++ b/tests/f2fs/Makefile
@@ -13,9 +13,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/generic/Makefile b/tests/generic/Makefile
index 9529fb8..3878d05 100644
--- a/tests/generic/Makefile
+++ b/tests/generic/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/overlay/Makefile b/tests/overlay/Makefile
index 63c9878..b07f892 100644
--- a/tests/overlay/Makefile
+++ b/tests/overlay/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/shared/Makefile b/tests/shared/Makefile
index cbd87f9..8a83278 100644
--- a/tests/shared/Makefile
+++ b/tests/shared/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/udf/Makefile b/tests/udf/Makefile
index 1d96658..c9c9f1b 100644
--- a/tests/udf/Makefile
+++ b/tests/udf/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/xfs/Makefile b/tests/xfs/Makefile
index db94be0..d64800e 100644
--- a/tests/xfs/Makefile
+++ b/tests/xfs/Makefile
@@ -12,9 +12,9 @@ include $(BUILDRULES)
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
--
2.5.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place
2016-07-11 9:26 [PATCH 0/4 v2] xfstests: extended names fixes Jan Tulak
2016-07-11 9:26 ` [PATCH 1/4] xfstests: Fix installation for extended names Jan Tulak
@ 2016-07-11 9:26 ` Jan Tulak
2016-07-13 12:34 ` Eryu Guan
2016-07-11 9:26 ` [PATCH 3/4] xfstests: remove unused variable Jan Tulak
2016-07-11 9:26 ` [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion Jan Tulak
3 siblings, 1 reply; 13+ messages in thread
From: Jan Tulak @ 2016-07-11 9:26 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
was moved at the earliest position, so everything working with the test name or
path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
check | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/check b/check
index 5be183f..ef6bd47 100755
--- a/check
+++ b/check
@@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
for seq in $list
do
err=false
+ if [ ! -f $seq ]; then
+ # Try to get full name in case the user supplied only seq id
+ # and the test has a name. A bit of hassle to find really
+ # the test and not its sample output or helping files.
+ bname=$(basename $seq)
+ full_seq=$(find $(dirname $seq) -name $bname* -executable |
+ awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
+ END { print shortest }')
+ if [ -f $full_seq ] \
+ && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
+ seq=$full_seq
+ seqnum=${full_seq#*/}
+ fi
+ fi
# the filename for the test and the name output are different.
# we don't include the tests/ directory in the name output.
@@ -566,19 +580,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
if $showme; then
echo
continue
- elif [ ! -f $seq ]; then
- # Try to get full name in case the user supplied only seq id
- # and the test has a name. A bit of hassle to find really
- # the test and not its sample output or helping files.
- bname=$(basename $seq)
- full_seq=$(find $(dirname $seq) -name $bname* -executable |
- awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
- END { print shortest }')
- if [ -f $full_seq ] \
- && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
- seq=$full_seq
- seqnum=${full_seq#*/}
- fi
fi
if [ ! -f $seq ]; then
--
2.5.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] xfstests: remove unused variable
2016-07-11 9:26 [PATCH 0/4 v2] xfstests: extended names fixes Jan Tulak
2016-07-11 9:26 ` [PATCH 1/4] xfstests: Fix installation for extended names Jan Tulak
2016-07-11 9:26 ` [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
@ 2016-07-11 9:26 ` Jan Tulak
2016-07-11 9:26 ` [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion Jan Tulak
3 siblings, 0 replies; 13+ messages in thread
From: Jan Tulak @ 2016-07-11 9:26 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
After the previous patch moved few lines of code, one seqnum assignment is now
immediately overwritten by another. Remove the useless one.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
check | 1 -
1 file changed, 1 deletion(-)
diff --git a/check b/check
index ef6bd47..58d8869 100755
--- a/check
+++ b/check
@@ -554,7 +554,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
if [ -f $full_seq ] \
&& [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
seq=$full_seq
- seqnum=${full_seq#*/}
fi
fi
--
2.5.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion
2016-07-11 9:26 [PATCH 0/4 v2] xfstests: extended names fixes Jan Tulak
` (2 preceding siblings ...)
2016-07-11 9:26 ` [PATCH 3/4] xfstests: remove unused variable Jan Tulak
@ 2016-07-11 9:26 ` Jan Tulak
2016-07-13 12:28 ` Eryu Guan
3 siblings, 1 reply; 13+ messages in thread
From: Jan Tulak @ 2016-07-11 9:26 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
Add quotation marks around the wildcarded name.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
check | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/check b/check
index 58d8869..5ff897b 100755
--- a/check
+++ b/check
@@ -548,7 +548,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
# and the test has a name. A bit of hassle to find really
# the test and not its sample output or helping files.
bname=$(basename $seq)
- full_seq=$(find $(dirname $seq) -name $bname* -executable |
+ full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
END { print shortest }')
if [ -f $full_seq ] \
--
2.5.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion
2016-07-11 9:26 ` [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion Jan Tulak
@ 2016-07-13 12:28 ` Eryu Guan
2016-07-14 8:56 ` Jan Tulak
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2016-07-13 12:28 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, david
On Mon, Jul 11, 2016 at 11:26:09AM +0200, Jan Tulak wrote:
> Add quotation marks around the wildcarded name.
This makes no difference for me, the original code is working well. Did
I miss anything?
[root@dhcp-66-86-11 xfstests]# bname=999
[root@dhcp-66-86-11 xfstests]# find tests/xfs -name "$bname*"
tests/xfs/999-test-case
tests/xfs/999-test-case.out
[root@dhcp-66-86-11 xfstests]# find tests/xfs -name $bname*
tests/xfs/999-test-case
tests/xfs/999-test-case.out
Thanks,
Eryu
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
> check | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/check b/check
> index 58d8869..5ff897b 100755
> --- a/check
> +++ b/check
> @@ -548,7 +548,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> # and the test has a name. A bit of hassle to find really
> # the test and not its sample output or helping files.
> bname=$(basename $seq)
> - full_seq=$(find $(dirname $seq) -name $bname* -executable |
> + full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
> awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
> END { print shortest }')
> if [ -f $full_seq ] \
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place
2016-07-11 9:26 ` [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
@ 2016-07-13 12:34 ` Eryu Guan
2016-07-14 9:11 ` Jan Tulak
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2016-07-13 12:34 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, david
On Mon, Jul 11, 2016 at 11:26:07AM +0200, Jan Tulak wrote:
> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
> was moved at the earliest position, so everything working with the test name or
> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
Can you please include more details in the description? It seems not so
clear to me what the problem is from the description.
If I understand it correctly (after playing around), prior to the patch:
[root@dhcp-66-86-11 xfstests]# ./check xfs/999
...
xfs/999 0s ... 0s
Ran: xfs/999-test-case
Passed all 1 tests
After applying the patch:
[root@dhcp-66-86-11 xfstests]# ./check xfs/999
...
xfs/999-test-case 0s ... 0s
Ran: xfs/999-test-case
Passed all 1 tests
So the test name is always correct, right?
Thanks,
Eryu
> ---
> check | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/check b/check
> index 5be183f..ef6bd47 100755
> --- a/check
> +++ b/check
> @@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
> for seq in $list
> do
> err=false
> + if [ ! -f $seq ]; then
> + # Try to get full name in case the user supplied only seq id
> + # and the test has a name. A bit of hassle to find really
> + # the test and not its sample output or helping files.
> + bname=$(basename $seq)
> + full_seq=$(find $(dirname $seq) -name $bname* -executable |
> + awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
> + END { print shortest }')
> + if [ -f $full_seq ] \
> + && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
> + seq=$full_seq
> + seqnum=${full_seq#*/}
> + fi
> + fi
>
> # the filename for the test and the name output are different.
> # we don't include the tests/ directory in the name output.
> @@ -566,19 +580,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
> if $showme; then
> echo
> continue
> - elif [ ! -f $seq ]; then
> - # Try to get full name in case the user supplied only seq id
> - # and the test has a name. A bit of hassle to find really
> - # the test and not its sample output or helping files.
> - bname=$(basename $seq)
> - full_seq=$(find $(dirname $seq) -name $bname* -executable |
> - awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
> - END { print shortest }')
> - if [ -f $full_seq ] \
> - && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
> - seq=$full_seq
> - seqnum=${full_seq#*/}
> - fi
> fi
>
> if [ ! -f $seq ]; then
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion
2016-07-13 12:28 ` Eryu Guan
@ 2016-07-14 8:56 ` Jan Tulak
2016-07-14 12:13 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Jan Tulak @ 2016-07-14 8:56 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, Chinner, Dave
On Wed, Jul 13, 2016 at 2:28 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Jul 11, 2016 at 11:26:09AM +0200, Jan Tulak wrote:
>> Add quotation marks around the wildcarded name.
>
> This makes no difference for me, the original code is working well. Did
> I miss anything?
>
> [root@dhcp-66-86-11 xfstests]# bname=999
> [root@dhcp-66-86-11 xfstests]# find tests/xfs -name "$bname*"
> tests/xfs/999-test-case
> tests/xfs/999-test-case.out
> [root@dhcp-66-86-11 xfstests]# find tests/xfs -name $bname*
> tests/xfs/999-test-case
> tests/xfs/999-test-case.out
>
Hmmm. The move of the code alone was not enough to fix an issue I had,
but now I can't remember when it happened, no matter what. :( I'm
beginning to think I might forget to save the file after moving the
code. In which case...
So either drop it and if it appears again, then I will resubmit, or
keep it there "just to be sure". I'm OK with both these options, so
whatever is more in line with your idea of a clean repo. :-)
Thanks,
Jan
> Thanks,
> Eryu
>
>>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>> ---
>> check | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/check b/check
>> index 58d8869..5ff897b 100755
>> --- a/check
>> +++ b/check
>> @@ -548,7 +548,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
>> # and the test has a name. A bit of hassle to find really
>> # the test and not its sample output or helping files.
>> bname=$(basename $seq)
>> - full_seq=$(find $(dirname $seq) -name $bname* -executable |
>> + full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
>> awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
>> END { print shortest }')
>> if [ -f $full_seq ] \
>> --
>> 2.5.5
>>
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place
2016-07-13 12:34 ` Eryu Guan
@ 2016-07-14 9:11 ` Jan Tulak
2016-07-14 12:18 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Jan Tulak @ 2016-07-14 9:11 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, Chinner, Dave
On Wed, Jul 13, 2016 at 2:34 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Jul 11, 2016 at 11:26:07AM +0200, Jan Tulak wrote:
>> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
>> was moved at the earliest position, so everything working with the test name or
>> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
>>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>
> Can you please include more details in the description? It seems not so
> clear to me what the problem is from the description.
>
> If I understand it correctly (after playing around), prior to the patch:
> [root@dhcp-66-86-11 xfstests]# ./check xfs/999
> ...
> xfs/999 0s ... 0s
> Ran: xfs/999-test-case
> Passed all 1 tests
>
> After applying the patch:
> [root@dhcp-66-86-11 xfstests]# ./check xfs/999
> ...
> xfs/999-test-case 0s ... 0s
> Ran: xfs/999-test-case
> Passed all 1 tests
>
> So the test name is always correct, right?
>
Yes. Most importantly, this fixes issues around other name-based
operations, see the new message:
---
The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
was moved to the earliest position, so everything working with the test name or
path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
An example of this issue is $testname.notrun file. When _notrun "foo" was run
during ./check foo/$name command, it created $name.notrun. But few lines later,
it wanted $fullname.notrun. So if you did ./check foo/999, but the file was
999-bar-baz, then you got comparing outputs (and most likely a fail)
instead of a skip.
Another example of this mix is in xfstests output:
./check xfs/999
[...]
xfs/999 0s ... 0s
Ran: xfs/999-test-case
---
Do you like it now? And do want it as a new version of the patch? :-)
Cheers,
Jan
> Thanks,
> Eryu
>
>> ---
>> check | 27 ++++++++++++++-------------
>> 1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/check b/check
>> index 5be183f..ef6bd47 100755
>> --- a/check
>> +++ b/check
>> @@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
>> for seq in $list
>> do
>> err=false
>> + if [ ! -f $seq ]; then
>> + # Try to get full name in case the user supplied only seq id
>> + # and the test has a name. A bit of hassle to find really
>> + # the test and not its sample output or helping files.
>> + bname=$(basename $seq)
>> + full_seq=$(find $(dirname $seq) -name $bname* -executable |
>> + awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
>> + END { print shortest }')
>> + if [ -f $full_seq ] \
>> + && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
>> + seq=$full_seq
>> + seqnum=${full_seq#*/}
>> + fi
>> + fi
>>
>> # the filename for the test and the name output are different.
>> # we don't include the tests/ directory in the name output.
>> @@ -566,19 +580,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
>> if $showme; then
>> echo
>> continue
>> - elif [ ! -f $seq ]; then
>> - # Try to get full name in case the user supplied only seq id
>> - # and the test has a name. A bit of hassle to find really
>> - # the test and not its sample output or helping files.
>> - bname=$(basename $seq)
>> - full_seq=$(find $(dirname $seq) -name $bname* -executable |
>> - awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
>> - END { print shortest }')
>> - if [ -f $full_seq ] \
>> - && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
>> - seq=$full_seq
>> - seqnum=${full_seq#*/}
>> - fi
>> fi
>>
>> if [ ! -f $seq ]; then
>> --
>> 2.5.5
>>
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion
2016-07-14 8:56 ` Jan Tulak
@ 2016-07-14 12:13 ` Eryu Guan
0 siblings, 0 replies; 13+ messages in thread
From: Eryu Guan @ 2016-07-14 12:13 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, Chinner, Dave
On Thu, Jul 14, 2016 at 10:56:51AM +0200, Jan Tulak wrote:
> On Wed, Jul 13, 2016 at 2:28 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Mon, Jul 11, 2016 at 11:26:09AM +0200, Jan Tulak wrote:
> >> Add quotation marks around the wildcarded name.
> >
> > This makes no difference for me, the original code is working well. Did
> > I miss anything?
> >
> > [root@dhcp-66-86-11 xfstests]# bname=999
> > [root@dhcp-66-86-11 xfstests]# find tests/xfs -name "$bname*"
> > tests/xfs/999-test-case
> > tests/xfs/999-test-case.out
> > [root@dhcp-66-86-11 xfstests]# find tests/xfs -name $bname*
> > tests/xfs/999-test-case
> > tests/xfs/999-test-case.out
> >
>
> Hmmm. The move of the code alone was not enough to fix an issue I had,
> but now I can't remember when it happened, no matter what. :( I'm
> beginning to think I might forget to save the file after moving the
> code. In which case...
>
> So either drop it and if it appears again, then I will resubmit, or
> keep it there "just to be sure". I'm OK with both these options, so
> whatever is more in line with your idea of a clean repo. :-)
I'd prefer to drop it for now, and fix it when we know what the exact
issue is :)
Thanks,
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place
2016-07-14 9:11 ` Jan Tulak
@ 2016-07-14 12:18 ` Eryu Guan
2016-07-14 12:47 ` Jan Tulak
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2016-07-14 12:18 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, Chinner, Dave
On Thu, Jul 14, 2016 at 11:11:17AM +0200, Jan Tulak wrote:
> On Wed, Jul 13, 2016 at 2:34 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Mon, Jul 11, 2016 at 11:26:07AM +0200, Jan Tulak wrote:
> >> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
> >> was moved at the earliest position, so everything working with the test name or
> >> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
> >>
> >> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> >
> > Can you please include more details in the description? It seems not so
> > clear to me what the problem is from the description.
> >
> > If I understand it correctly (after playing around), prior to the patch:
> > [root@dhcp-66-86-11 xfstests]# ./check xfs/999
> > ...
> > xfs/999 0s ... 0s
> > Ran: xfs/999-test-case
> > Passed all 1 tests
> >
> > After applying the patch:
> > [root@dhcp-66-86-11 xfstests]# ./check xfs/999
> > ...
> > xfs/999-test-case 0s ... 0s
> > Ran: xfs/999-test-case
> > Passed all 1 tests
> >
> > So the test name is always correct, right?
> >
>
> Yes. Most importantly, this fixes issues around other name-based
> operations, see the new message:
> ---
> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
> was moved to the earliest position, so everything working with the test name or
> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
>
> An example of this issue is $testname.notrun file. When _notrun "foo" was run
> during ./check foo/$name command, it created $name.notrun. But few lines later,
> it wanted $fullname.notrun. So if you did ./check foo/999, but the file was
> 999-bar-baz, then you got comparing outputs (and most likely a fail)
> instead of a skip.
>
> Another example of this mix is in xfstests output:
> ./check xfs/999
> [...]
> xfs/999 0s ... 0s
> Ran: xfs/999-test-case
> ---
>
> Do you like it now? And do want it as a new version of the patch? :-)
This does read better to me, thanks! And seems you're going to send a
new patchset, then please send a new version with updated description.
Thanks!
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place
2016-07-14 12:18 ` Eryu Guan
@ 2016-07-14 12:47 ` Jan Tulak
0 siblings, 0 replies; 13+ messages in thread
From: Jan Tulak @ 2016-07-14 12:47 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, Chinner, Dave
On Thu, Jul 14, 2016 at 2:18 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Thu, Jul 14, 2016 at 11:11:17AM +0200, Jan Tulak wrote:
>> On Wed, Jul 13, 2016 at 2:34 PM, Eryu Guan <eguan@redhat.com> wrote:
>> > On Mon, Jul 11, 2016 at 11:26:07AM +0200, Jan Tulak wrote:
>> >> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
>> >> was moved at the earliest position, so everything working with the test name or
>> >> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
>> >>
>> >> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>> >
>> > Can you please include more details in the description? It seems not so
>> > clear to me what the problem is from the description.
>> >
>> > If I understand it correctly (after playing around), prior to the patch:
>> > [root@dhcp-66-86-11 xfstests]# ./check xfs/999
>> > ...
>> > xfs/999 0s ... 0s
>> > Ran: xfs/999-test-case
>> > Passed all 1 tests
>> >
>> > After applying the patch:
>> > [root@dhcp-66-86-11 xfstests]# ./check xfs/999
>> > ...
>> > xfs/999-test-case 0s ... 0s
>> > Ran: xfs/999-test-case
>> > Passed all 1 tests
>> >
>> > So the test name is always correct, right?
>> >
>>
>> Yes. Most importantly, this fixes issues around other name-based
>> operations, see the new message:
>> ---
>> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
>> was moved to the earliest position, so everything working with the test name or
>> path will know the full name. Thus, no "123" and "123-bar-baz" mix is possible.
>>
>> An example of this issue is $testname.notrun file. When _notrun "foo" was run
>> during ./check foo/$name command, it created $name.notrun. But few lines later,
>> it wanted $fullname.notrun. So if you did ./check foo/999, but the file was
>> 999-bar-baz, then you got comparing outputs (and most likely a fail)
>> instead of a skip.
>>
>> Another example of this mix is in xfstests output:
>> ./check xfs/999
>> [...]
>> xfs/999 0s ... 0s
>> Ran: xfs/999-test-case
>> ---
>>
>> Do you like it now? And do want it as a new version of the patch? :-)
>
> This does read better to me, thanks! And seems you're going to send a
> new patchset, then please send a new version with updated description.
>
New version submitted with updated description.
Thanks,
Jan
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] xfstests: Fix installation for extended names
2016-07-11 9:26 ` [PATCH 1/4] xfstests: Fix installation for extended names Jan Tulak
@ 2016-07-15 10:05 ` Eryu Guan
0 siblings, 0 replies; 13+ messages in thread
From: Eryu Guan @ 2016-07-15 10:05 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, david
On Mon, Jul 11, 2016 at 11:26:06AM +0200, Jan Tulak wrote:
> xfstests supports extended test names like 314-foo-bar, but installation of
> these tests was skipped (not matching a regexp). So this patch fixes the
> makefiles in tests/*/
>
> The include/buildrules change was written by Dave Chinner.
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
> UPDATE:
> Use Dave's code to at first select all files and then separate output and test
> files. BTW, in this version, when most of this version is written by Dave,
> should I add Signed-off-by, or is better to keep it mentioned only in the text?
> ---
> include/buildrules | 31 +++++++++++++++++++++++++++++++
> tests/btrfs/Makefile | 4 ++--
> tests/cifs/Makefile | 4 ++--
> tests/ext4/Makefile | 4 ++--
> tests/f2fs/Makefile | 4 ++--
> tests/generic/Makefile | 4 ++--
> tests/overlay/Makefile | 4 ++--
> tests/shared/Makefile | 4 ++--
> tests/udf/Makefile | 4 ++--
> tests/xfs/Makefile | 4 ++--
> 10 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/include/buildrules b/include/buildrules
> index c8a7c47..1269c8c 100644
> --- a/include/buildrules
> +++ b/include/buildrules
> @@ -100,3 +100,34 @@ MAKEDEP := $(MAKEDEPEND) $(CFLAGS)
> cp /dev/null .dep; \
> fi
>
> +# Gather files for installing into two lists:
> +# TESTS with executable scripts and OUTFILES with all the test outputs.
> +# Makefile has a very small matching, so we have to go this long way.
> +
> +# Start with all test related files:
> +ALLFILES = $(wildcard [0-9]??*)
> +
> +# Now build a list of known output files. Unfortunately, the
> +# multiple output test files are poorly handled as makefiles don't
> +# handle wildcarded multi-suffix matching. Hence we separate the
> +# processing of these right now.
> +EXTENDED_OUTFILES = $(wildcard [0-9]??*.out.*)
> +BASIC_OUTFILES = $(wildcard [0-9]??*.out)
This turns out to be not sufficient, [0-9]??.cfg files are missed, so
tests like ext4/001 and generic/088 fail due to "no qualified output",
because _link_out_file() failed to work correctly without these *.cfg
files.
Adding another wildcard to match cfg files works for me. e.g.
EXTENDED_OUTFILE_CFGS = $(wildcard [0-9]??.cfg)
then update OUTFILES accordingly
> +OUTFILES = $(EXTENDED_OUTFILES) $(BASIC_OUTFILES)
OUTFILES = $(EXTENDED_OUTFILES) $(EXTENDED_OUTFILE_CFGS) $(BASIC_OUTFILES)
Can you please send an updated version of this patch?
Thanks,
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-07-15 10:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-11 9:26 [PATCH 0/4 v2] xfstests: extended names fixes Jan Tulak
2016-07-11 9:26 ` [PATCH 1/4] xfstests: Fix installation for extended names Jan Tulak
2016-07-15 10:05 ` Eryu Guan
2016-07-11 9:26 ` [PATCH 2/4] fstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
2016-07-13 12:34 ` Eryu Guan
2016-07-14 9:11 ` Jan Tulak
2016-07-14 12:18 ` Eryu Guan
2016-07-14 12:47 ` Jan Tulak
2016-07-11 9:26 ` [PATCH 3/4] xfstests: remove unused variable Jan Tulak
2016-07-11 9:26 ` [PATCH 4/4] xfstests: filename handling - fix early wildcard expansion Jan Tulak
2016-07-13 12:28 ` Eryu Guan
2016-07-14 8:56 ` Jan Tulak
2016-07-14 12:13 ` Eryu Guan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.