| Message ID | 20180503033414.5601-1-liwang@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [v2] tst_test.sh: achieve TST_RETRY_FUNC function in shell | expand |
Hi Li, > The commit c2ce4df67d(include: add an exponential backoff macro for > function retry) involves a new MACRO for function retry in C code, > here achieve it in shell lib and gives a introduction in LTP documents. > Signed-off-by: Li Wang <liwang@redhat.com> > Tested-by: Petr Vorel <pvorel@suse.cz> > --- ... > +++ b/testcases/lib/tst_test.sh > @@ -154,6 +154,40 @@ EXPECT_FAIL() > fi > } > +TST_RETRY_FN_EXP_BACKOFF() > +{ > + local tst_fun=$1 > + local tst_exp=$2 Maybe add check for int, something like this? if ! tst_is_int "$tst_sec"; then tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF: tst_sec must be integer ('$tst_sec')" fi > + local tst_sec=$(expr $3 \* 1000000) > + local tst_delay=1 > + > + if [ $# -ne 3 ]; then > + tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF expects three parameters" I'd personally use number than word, but feel free to ignore that. tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF expects 3 parameters" I'm not sure if checking ... > +TST_RETRY_FUNC() > +{ TST_RETRY_FUNC needs check for number of params as well: if [ $# -ne 2 ]; then tst_brk TBROK "TST_RETRY_FUNC expects 2 parameters" fi > + TST_RETRY_FN_EXP_BACKOFF "$1" "$2" 1 Otherwise you run TST_RETRY_FUNC without params and didn't get check. some-test 1 TBROK: "" failed Kind regards, Petr
Ah, forgot to CC' LTP list. ---------- Forwarded message ---------- From: Li Wang <liwang@redhat.com> Date: Thu, May 17, 2018 at 1:03 PM Subject: Re: [LTP] [PATCH v2] tst_test.sh: achieve TST_RETRY_FUNC function in shell To: Petr Vorel <pvorel@suse.cz> Hi Petr, On Wed, May 16, 2018 at 6:44 PM, Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, > > > The commit c2ce4df67d(include: add an exponential backoff macro for > > function retry) involves a new MACRO for function retry in C code, > > here achieve it in shell lib and gives a introduction in LTP documents. > > > Signed-off-by: Li Wang <liwang@redhat.com> > > Tested-by: Petr Vorel <pvorel@suse.cz> > > --- > ... > > +++ b/testcases/lib/tst_test.sh > > @@ -154,6 +154,40 @@ EXPECT_FAIL() > > fi > > } > > > +TST_RETRY_FN_EXP_BACKOFF() > > +{ > > + local tst_fun=$1 > > + local tst_exp=$2 > > Maybe add check for int, something like this? > if ! tst_is_int "$tst_sec"; then > tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF: tst_sec must be > integer ('$tst_sec')" > fi > Sounds good. If do this, I'd put the check after tst_sec assignment. > > + local tst_sec=$(expr $3 \* 1000000) > > + local tst_delay=1 > > + > > + if [ $# -ne 3 ]; then > > + tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF expects three > parameters" > I'd personally use number than word, but feel free to ignore that. > tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF expects 3 > parameters" > I'm not sure if checking > ... > Ok, fine. > > > +TST_RETRY_FUNC() > > +{ > TST_RETRY_FUNC needs check for number of params as well: > > if [ $# -ne 2 ]; then > tst_brk TBROK "TST_RETRY_FUNC expects 2 parameters" > fi > > + TST_RETRY_FN_EXP_BACKOFF "$1" "$2" 1 > > Otherwise you run TST_RETRY_FUNC without params and didn't get check. > some-test 1 TBROK: "" failed > Agree. > > > Kind regards, > Petr >
Hi Petr,
And also, we'd better skip the TST_ prefix name check in test library,
otherwise we will get LTP warnings like:
<<<test_output>>>
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!
numa01 1 TINFO: The system contains 2 nodes: 0 1
numa01 1 TPASS: NUMA local node and memory affinity
numa01 2 TPASS: NUMA preferred node policy
numa01 3 TPASS: NUMA share memory allocated in preferred node
numa01 4 TPASS: NUMA interleave policy
numa01 5 TPASS: NUMA interleave policy on shared memory
numa01 6 TPASS: NUMA phycpubind policy
numa01 7 TPASS: NUMA local node allocation
numa01 8 TPASS: NUMA MEMHOG policy
----------------
tst_umount()
{
local device="$1"
@@ -255,6 +297,7 @@ tst_run()
OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
NEEDS_ROOT|NEEDS_TMPDIR|NEEDS_DEVICE|DEVICE);;
NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);;
+ RETRY_FUNC|RETRY_FN_EXP_BACKOFF);;
IPV6);;
*) tst_res TWARN "Reserved variable TST_$tst_i
used!";;
esac
I prefer to merge this change along with your suggestions in PATCH v3 as
well.
Thanks,
Li Wang
<div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Petr,<br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><div style="font-size:small" class="gmail_default">And also, we'd better skip the TST_ prefix name check in test library, otherwise we will get LTP warnings like:</div><div style="font-size:small" class="gmail_default"><br></div><div style="font-size:small" class="gmail_default"><<<test_output>>><br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used!<br>numa01 1 TINFO: The system contains 2 nodes: 0 1 <br>numa01 1 TPASS: NUMA local node and memory affinity<br>numa01 2 TPASS: NUMA preferred node policy<br>numa01 3 TPASS: NUMA share memory allocated in preferred node<br>numa01 4 TPASS: NUMA interleave policy<br>numa01 5 TPASS: NUMA interleave policy on shared memory<br>numa01 6 TPASS: NUMA phycpubind policy<br>numa01 7 TPASS: NUMA local node allocation<br>numa01 8 TPASS: NUMA MEMHOG policy<br></div><div style="font-size:small" class="gmail_default"><br></div><div style="font-size:small" class="gmail_default">----------------<br></div><div style="font-size:small" class="gmail_default"> tst_umount()<br> {<br> local device="$1"<br>@@ -255,6 +297,7 @@ tst_run()<br> OPTS|USAGE|PARSE_ARGS|POS_ARGS);;<br> NEEDS_ROOT|NEEDS_TMPDIR|NEEDS_DEVICE|DEVICE);;<br> NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);;<br>+ RETRY_FUNC|RETRY_FN_EXP_BACKOFF);;<br> IPV6);;<br> *) tst_res TWARN "Reserved variable TST_$tst_i used!";;<br> esac<br></div></div><div class="gmail_extra"><br></div><div class="gmail_extra"><div style="font-size:small" class="gmail_default">I prefer to merge this change along with your suggestions in PATCH v3 as well.</div><div style="font-size:small" class="gmail_default"><br></div><div style="font-size:small" class="gmail_default">Thanks,</div><div style="font-size:small" class="gmail_default">Li Wang</div></div></div>
Hi Li, > Hi Petr, > And also, we'd better skip the TST_ prefix name check in test library, > otherwise we will get LTP warnings like: > <<<test_output>>> > numa01 1 TWARN: Reserved variable TST_RETRY_FUNC used! I'm sorry I've noticed it but forget to report it. ... > @@ -255,6 +297,7 @@ tst_run() > OPTS|USAGE|PARSE_ARGS|POS_ARGS);; > NEEDS_ROOT|NEEDS_TMPDIR|NEEDS_DEVICE|DEVICE);; > NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);; > + RETRY_FUNC|RETRY_FN_EXP_BACKOFF);; > IPV6);; > I prefer to merge this change along with your suggestions in PATCH v3 as > well. Sure! > Thanks, > Li Wang Kind regards, Petr
diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt index cbbfe6c..b2dd091 100644 --- a/doc/test-writing-guidelines.txt +++ b/doc/test-writing-guidelines.txt @@ -1640,6 +1640,32 @@ that can sleep for defined amount of seconds, milliseconds or microseconds. tst_sleep 100ms ------------------------------------------------------------------------------- +Retry a function in limited time +++++++++++++++++++++++++++++++++ + +Sometimes LTP test needs retrying a function for many times to get success. +This achievement makes that possible via keeping it retrying if the return +value of the function is NOT as we expected. After exceeding a limited time, +test will break from the retries immediately. + +[source,c] +------------------------------------------------------------------------------- +# retry function in 1 second +TST_RETRY_FUNC(FUNC, EXPECTED_RET) + +# retry function in N second +TST_RETRY_FN_EXP_BACKOFF(FUNC, EXPECTED_RET, N) +------------------------------------------------------------------------------- + +[source,sh] +------------------------------------------------------------------------------- +# retry function in 1 second +TST_RETRY_FUNC "FUNC arg1 arg2 ..." "EXPECTED_RET" + +# retry function in N second +TST_RETRY_FN_EXP_BACKOFF "FUNC arg1 arg2 ..." "EXPECTED_RET" "N" +------------------------------------------------------------------------------- + Checking for integers +++++++++++++++++++++ diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 8d49d34..8b38dcd 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -154,6 +154,40 @@ EXPECT_FAIL() fi } +TST_RETRY_FN_EXP_BACKOFF() +{ + local tst_fun=$1 + local tst_exp=$2 + local tst_sec=$(expr $3 \* 1000000) + local tst_delay=1 + + if [ $# -ne 3 ]; then + tst_brk TBROK "TST_RETRY_FN_EXP_BACKOFF expects three parameters" + fi + + while true; do + $tst_fun + if [ "$?" = "$tst_exp" ]; then + break + fi + + if [ $tst_delay -lt $tst_sec ]; then + tst_sleep ${tst_delay}us + tst_delay=$((tst_delay*2)) + else + tst_brk TBROK "\"$tst_fun\" failed" + fi + done + + return $tst_exp +} + +TST_RETRY_FUNC() +{ + TST_RETRY_FN_EXP_BACKOFF "$1" "$2" 1 + return $2 +} + tst_umount() { local device="$1"