Skip to contentSkip to conversation

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

PATCH In review Thomas Huth 2 participants 5 messages Jul 1, 2020 KVM

lore
Share & tools
lore

Thomas Huth

author
#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 - and "grep -w" does not consider the "-" character as part of a word.

To fix the issue, convert the dash into an underscore character before running "grep -w".

Signed-off-by Thomas Huth <[email protected]>
---
 scripts/runtime.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
scripts/runtime.bash +2 -1
index 8bfe31c..03fd20a 100644
@@ -84,7 +84,8 @@ function run()
         return
     fi
 
-    if [ -n "$only_tests" ] && ! grep -qw "$testname" <<<$only_tests; then
+    if [ -n "$only_tests" ] && ! sed s/-/_/ <<<$only_tests \
+                               | grep -qw $(sed s/-/_/ <<< "$testname") ; then
         return
     fi
Signature · 1 line
2.18.1
maintainer
#1 lore raw
Quoted 24 lines · Thomas Huth

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 - and "grep -w" does not consider the "-" character as part of a word.

To fix the issue, convert the dash into an underscore character before running "grep -w".

Signed-off-by Thomas Huth <[email protected]>
---
 scripts/runtime.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 8bfe31c..03fd20a 100644

scripts/runtime.bash function run() · line 91

Quoted 9 lines
scripts/runtime.bash +2 -1
@@ -84,7 +84,8 @@ function run()
         return
     fi
 
-    if [ -n "$only_tests" ] && ! grep -qw "$testname" <<<$only_tests; then
+    if [ -n "$only_tests" ] && ! sed s/-/_/ <<<$only_tests \
+                               | grep -qw $(sed s/-/_/ <<< "$testname") ; then
         return
     fi

Simpler: grep -q " $testname " <<< " $only_tests "

Also, please do the same for groups in the two "if" statements right below.

Paolo

author
#2 lore raw
Quoted 27 lines · Paolo Bonzini
Quoted 26 lines · Thomas Huth

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 - and "grep -w" does not consider the "-" character as part of a word.

To fix the issue, convert the dash into an underscore character before running "grep -w".

Signed-off-by Thomas Huth <[email protected]>
---
  scripts/runtime.bash | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 8bfe31c..03fd20a 100644
@@ -84,7 +84,8 @@ function run()
          return
      fi
Quoted 9 lines

scripts/runtime.bash function run() · line 91

Quoted 7 lines
scripts/runtime.bash +2 -1
-    if [ -n "$only_tests" ] && ! grep -qw "$testname" <<<$only_tests; then
+    if [ -n "$only_tests" ] && ! sed s/-/_/ <<<$only_tests \
+                               | grep -qw $(sed s/-/_/ <<< "$testname") ; then
          return
      fi

Simpler: grep -q " $testname " <<< " $only_tests "

That doesn't work:

$ ./run_tests.sh ioapic-split
PASS apic-split (53 tests)
PASS ioapic-split (19 tests)
PASS apic (53 tests)
PASS ioapic (26 tests)

... because the $testname comes from unittests.cfg and $only_tests is the list that has been given on the command line. It would maybe work if the check was the other way round ... but that would require to rewrite quite a bit of the script logic...

By the way, you can currently also run "./run_test.sh badname" and it does *not* complain that "badname" is an illegal test name...

Thomas

maintainer
#3 lore raw
Quoted 12 lines · Thomas Huth
Quoted 3 lines
Quoted 1 line

Simpler: grep -q " $testname " <<< " $only_tests "

That doesn't work:

$ ./run_tests.sh ioapic-split
PASS apic-split (53 tests)
PASS ioapic-split (19 tests)
PASS apic (53 tests)
PASS ioapic (26 tests)
Quoted 4 lines

... because the $testname comes from unittests.cfg and $only_tests is the list that has been given on the command line. It would maybe work if the check was the other way round ... but that would require to rewrite quite a bit of the script logic...

It works here. I'll send a patch.

Paolo

Quoted 2 lines

By the way, you can currently also run "./run_test.sh badname" and it does *not* complain that "badname" is an illegal test name...

author
#4 lore raw
In reply to Thomas Huth
Quoted 38 lines · Thomas Huth
Quoted 28 lines · Paolo Bonzini
Quoted 27 lines · Thomas Huth

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 - and "grep -w" does not consider the "-" character as part of a word.

To fix the issue, convert the dash into an underscore character before running "grep -w".

Signed-off-by Thomas Huth <[email protected]>

---
  scripts/runtime.bash | 3 ++-

  1 file changed, 2 insertions(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 8bfe31c..03fd20a 100644
@@ -84,7 +84,8 @@ function run()

          return
      fi
-    if [ -n "$only_tests" ] && ! grep -qw "$testname" <<<$only_tests; then

Quoted 8 lines
Quoted 6 lines

+    if [ -n "$only_tests" ] && ! sed s/-/_/ <<<$only_tests \
+                               | grep -qw $(sed s/-/_/ <<< "$testname") ; then
          return
      fi

Simpler: grep -q " $testname " <<< " $only_tests "

Quoted 1 line

That doesn't work:

I obviously missed the surrounding spaces ... not enough coffee... ;-)

Thomas