Test exit code fixlets
cover Test exit code fixlets
A small set of exit code fixes to tests.
There are some other issues that are worth discussing, like pipelines and ignoring the exit code there, but a lot of that stuff doesn't matter *too* much given the test output is checked anyway.
Sam James (3):
test: fix exit code in tst.ipv4remotetcp.sh
test: fix exit code typo in tst.core.sh
test: fix exit code in tst.newprobes.sh
test/unittest/ip/tst.ipv4remotetcp.sh | 2 +- test/unittest/options/tst.core.sh | 2 +- test/unittest/pid/tst.newprobes.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
base-commit: c84954cdc6fb017034e2c3fe4445a7c8981d35e9
Signature · 1 line
2.46.0
PATCH 1/3 test: fix exit code in tst.ipv4remotetcp.sh
$? was clobbered by the status=$? assignment above, see:
```
$ bash -c 'false ; x=$? ; echo $? ; echo ${x}'
0
1
```We could just drop the status assignment but I just chose to do this instead.
--- test/unittest/ip/tst.ipv4remotetcp.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
test/unittest/ip/tst.ipv4remotetcp.sh +1 -1
@@ -98,4 +98,4 @@ EODTRACE status=$? -exit $? +exit ${status}
Signature · 1 line
2.46.0
Re: [PATCH 1/3] test: fix exit code in tst.ipv4remotetcp.sh
Quoted 9 lines · Sam James
$? was clobbered by the status=$? assignment above, see:
``` $ bash -c 'false ; x=$? ; echo $? ; echo ${x}' 0 1 ```We could just drop the status assignment but I just chose to do this instead.
... and committed with Signed-off-by added.
Quoted 17 lines
--- test/unittest/ip/tst.ipv4remotetcp.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/ip/tst.ipv4remotetcp.sh +1
-1@@ -98,4 +98,4 @@ EODTRACE status=$? -exit $? +exit ${status}Signature · 1 line
2.46.0
PATCH 2/3 test: fix exit code typo in tst.core.sh
In reply to Sam James$rcc is not set, but $rc is. Use that.
--- test/unittest/options/tst.core.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
test/unittest/options/tst.core.sh +1 -1
@@ -16,4 +16,4 @@ file core | tee /dev/stderr | \ rc=$? rm -f core -exit $rcc +exit $rc
Signature · 1 line
2.46.0
Re: [PATCH 2/3] test: fix exit code typo in tst.core.sh
Quoted 1 line · Sam James
$rcc is not set, but $rc is. Use that.
... and committed with Signed-off-by added.
Quoted 17 lines
--- test/unittest/options/tst.core.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/options/tst.core.sh +1
-1@@ -16,4 +16,4 @@ file core | tee /dev/stderr | \ rc=$? rm -f core -exit $rcc +exit $rcSignature · 1 line
2.46.0
PATCH 3/3 test: fix exit code in tst.newprobes.sh
In reply to Sam James$status is never assigned, just use $?.
--- test/unittest/pid/tst.newprobes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
test/unittest/pid/tst.newprobes.sh +1 -1
@@ -44,4 +44,4 @@ tick-1s } EOF -exit $status +exit $?
Signature · 1 line
2.46.0
Re: [PATCH 3/3] test: fix exit code in tst.newprobes.sh
Another question about this test is whether "pid*:awk::" is even a legitimate probe description for pid probes; do we allow wildcards for pid numbers in pid provider names? Anyhow, that's a separate issue from the one raised in this patch; it does, however, perhaps influence where we want to go with the test.
Quoted 15 lines · Sam James
$status is never assigned, just use $?.
--- test/unittest/pid/tst.newprobes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/pid/tst.newprobes.sh +1
-1@@ -44,4 +44,4 @@ tick-1s } EOF -exit $status +exit $?
Re: [PATCH 3/3] test: fix exit code in tst.newprobes.sh
In reply to Sam JamesQuoted 1 line · Sam James
$status is never assigned, just use $?.
... and committed with Signed-off-by added.
Quoted 17 lines
--- test/unittest/pid/tst.newprobes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/pid/tst.newprobes.sh +1
-1@@ -44,4 +44,4 @@ tick-1s } EOF -exit $status +exit $?Signature · 1 line
2.46.0
Re: [PATCH 3/3] test: fix exit code in tst.newprobes.sh
Maybe I am a few patches behind. I thought this test used pid*:awk:: and was therefore XFAIL. I do not object to the patch, but I was only curious if we wanted a "real fix" to the test. There are other cases where tests have multiple problems and we do not generally seem to settle for fixing only one of the problems. Again, though, I'm okay with the patch... it's an improvement over what's there.
Quoted 23 lines · Kris Van Hees
Quoted 1 line · Sam James
$status is never assigned, just use $?.
Reviewed-by Kris Van Hees <[email protected]>... and committed with Signed-off-by added.
Quoted 17 lines
--- test/unittest/pid/tst.newprobes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/pid/tst.newprobes.sh +1
-1@@ -44,4 +44,4 @@ tick-1s } EOF -exit $status +exit $?Signature · 1 line
2.46.0
Re: [PATCH 3/3] test: fix exit code in tst.newprobes.sh
Quoted 6 lines · Eugene Loh
Maybe I am a few patches behind. I thought this test used pid*:awk:: and was therefore XFAIL. I do not object to the patch, but I was only curious if we wanted a "real fix" to the test. There are other cases where tests have multiple problems and we do not generally seem to settle for fixing only one of the problems. Again, though, I'm okay with the patch... it's an improvement over what's there.
In general, yes, when we encounter a case like this we tend to fix other more fundamental problems with the test. In this case, since it is part of a larger series of test clean up work, I took it as-is so as to not delay the cleanup any longer. But yes, the test will still need extra work to ensure it is actually correct in terms of functionality.
Quoted 23 lines · Kris Van Hees
Quoted 22 lines · Sam James
Quoted 1 line
$status is never assigned, just use $?.
Reviewed-by Kris Van Hees <[email protected]>... and committed with Signed-off-by added.
Quoted 16 lines
--- test/unittest/pid/tst.newprobes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)test/unittest/pid/tst.newprobes.sh +1
-1@@ -44,4 +44,4 @@ tick-1s } EOF -exit $status +exit $?Signature · 1 line
2.46.0