Skip to contentSkip to conversation

scripts: Fix the check whether testname is in the only_tests list

PATCH In review Paolo Bonzini 2 participants 4 messages Jul 1, 2020 KVM

lore
Share & tools
lore

Paolo Bonzini

author maintainer
#0 lore raw

PATCH 1/1 scripts: Fix the check whether testname is in the only_tests list

When you currently run

./run_tests.sh ioapic-split

the kvm-unit-tests run scripts do not only execute the "ioapic-split" test, but also the "ioapic" test, which is quite surprising. This happens because we use "grep -w" for checking whether a test should be run or not. Because "grep -w" does not consider the "-" character as part of a word, "ioapic" successfully matches against "ioapic-split".

To fix the issue, use spaces as the only delimiter when running "grep", removing the problematic "-w" flag from the invocation.

---
 scripts/runtime.bash | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
scripts/runtime.bash +8 -3
index 8bfe31c..6158e37 100644
@@ -68,6 +68,11 @@ function print_result()
     fi
 }
 
+function find_word()
+{
+    grep -q " $1 " <<< " $2 "
+}
+
 function run()
 {
     local testname="$1"
@@ -84,15 +89,15 @@ function run()
         return
     fi
 
-    if [ -n "$only_tests" ] && ! grep -qw "$testname" <<<$only_tests; then
+    if [ -n "$only_tests" ] && ! find_word "$testname" "$only_tests"; then
         return
     fi
 
-    if [ -n "$only_group" ] && ! grep -qw "$only_group" <<<$groups; then
+    if [ -n "$only_group" ] && ! find_word "$only_group" "$groups"; then
         return
     fi
 
-    if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups &&
+    if [ -z "$only_group" ] && find_word nodefault "$groups" &&
             skip_nodefault; then
         print_result "SKIP" $testname "" "test marked as manual run only"
         return;
Signature · 1 line
2.25.4
Tested-by Reviewed-by on v1 1/1
#1 lore raw
Quoted 24 lines · Paolo Bonzini

When you currently run

./run_tests.sh ioapic-split

the kvm-unit-tests run scripts do not only execute the "ioapic-split" test, but also the "ioapic" test, which is quite surprising. This happens because we use "grep -w" for checking whether a test should be run or not. Because "grep -w" does not consider the "-" character as part of a word, "ioapic" successfully matches against "ioapic-split".

To fix the issue, use spaces as the only delimiter when running "grep", removing the problematic "-w" flag from the invocation.

---
  scripts/runtime.bash | 11 ++++++++---
  1 file changed, 8 insertions(+), 3 deletions(-)
scripts/runtime.bash +0 -0
index 8bfe31c..6158e37 100644

scripts/runtime.bash function print_result() · line 74

Quoted 7 lines
scripts/runtime.bash +4 -0
@@ -68,6 +68,11 @@ function print_result()
      fi
  }
 
+function find_word()
+{
+    grep -q " $1 " <<< " $2 "
+}

Ah, clever idea with the surrounding spaces here!

Works great for me, so:

author maintainer
#2 lore raw
Quoted 6 lines · Thomas Huth

scripts/runtime.bash function print_result() · line 74

Quoted 4 lines

  +function find_word()

scripts/runtime.bash +3 -0
+{
+    grep -q " $1 " <<< " $2 "
+}

Ah, clever idea with the surrounding spaces here!

That's what you gain from learning to program in GW-BASIC, who wants regular expression when you have INSTR.

But seriously: what do you think about adding "-F"? The use of regex in only_tests/only_group is not documented and might have surprising effects. If we want to keep it, we could replace spaces with newlines and use ^$ in the regex.

Paolo

Quoted 4 lines

Works great for me, so:

#3 lore raw
Quoted 8 lines · Paolo Bonzini
Quoted 6 lines · Thomas Huth

scripts/runtime.bash function print_result() · line 74

Quoted 4 lines

  +function find_word()

scripts/runtime.bash +3 -0
+{
+    grep -q " $1 " <<< " $2 "
+}

Ah, clever idea with the surrounding spaces here!

Quoted 7 lines

That's what you gain from learning to program in GW-BASIC, who wants regular expression when you have INSTR.

But seriously: what do you think about adding "-F"? The use of regex in only_tests/only_group is not documented and might have surprising effects. If we want to keep it, we could replace spaces with newlines and use ^$ in the regex.

-F sounds fine to me. I'm not aware of anybody trying to use regular expressions in the list of tests, so that should be fine, I think.

Thomas