| Message ID | 53a3e9975f00c50c78528994472ec7e9f8adeb90.1592475774.git.jstancek@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clocks/invaliddates.c: relax acceptable delta | expand |
Hi Jan, > Test allows just 5ms delta for PASS, and test randomly fails in > environments with shared resources and increased steal time. Good. > Relax the condition and also print deltas if test fails. > Remove DEBUG ifdefs and main parameters to avoid unused variable > warning. Also thanks for cleanup. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
On Thu, Jun 18, 2020 at 6:24 PM Jan Stancek <jstancek@redhat.com> wrote: > Test allows just 5ms delta for PASS, and test randomly fails in > environments with shared resources and increased steal time. > > Relax the condition and also print deltas if test fails. > Remove DEBUG ifdefs and main parameters to avoid unused variable > warning. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > .../functional/timers/clocks/invaliddates.c | 25 +++++++------------ > 1 file changed, 9 insertions(+), 16 deletions(-) > > diff --git > a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c > b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c > index face334fd250..d4116b1e9bc0 100644 > --- > a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c > +++ > b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c > @@ -18,8 +18,7 @@ > > #define NUMTESTS 5 > > -#define ACCEPTABLESECDELTA 0 > -#define ACCEPTABLENSECDELTA 5000000 > +#define ACCEPTABLESECDELTA 2 > It's hard to say what size of the second-delta is a proper value, but to increase it can obviously decrease the failure probability. If there is no better way I'd go with this patch. Reviewed-by: Li Wang <liwang@redhat.com> > > static int testtimes[NUMTESTS][2] = { > {INT32_MAX, 999999999}, // large number > @@ -29,7 +28,7 @@ static int testtimes[NUMTESTS][2] = { > {1049623200, 999999999}, // daylight savings 2003 > }; > > -int main(int argc, char *argv[]) > +int main(void) > { > struct timespec tpset, tpget, tsreset; > int secdelta, nsecdelta; > @@ -44,9 +43,6 @@ int main(int argc, char *argv[]) > for (i = 0; i < NUMTESTS; i++) { > tpset.tv_sec = testtimes[i][0]; > tpset.tv_nsec = testtimes[i][1]; > -#ifdef DEBUG > - printf("Test: %ds %dns\n", testtimes[i][0], > testtimes[i][1]); > -#endif > if (clock_settime(CLOCK_REALTIME, &tpset) == 0) { > if (clock_gettime(CLOCK_REALTIME, &tpget) == -1) { > printf("Error in clock_gettime()\n"); > @@ -58,16 +54,13 @@ int main(int argc, char *argv[]) > nsecdelta = nsecdelta + 1000000000; > secdelta = secdelta - 1; > } > -#ifdef DEBUG > - printf("Delta: %ds %dns\n", secdelta, nsecdelta); > -#endif > - if ((secdelta > ACCEPTABLESECDELTA) || (secdelta < > 0)) { > - printf("clock does not appear to be > set\n"); > - failure = 1; > - } > - if ((nsecdelta > ACCEPTABLENSECDELTA) || > - (nsecdelta < 0)) { > - printf("clock does not appear to be > set\n"); > + > + if ((secdelta > ACCEPTABLESECDELTA) > + || (secdelta < 0)) { > + printf("FAIL: test(%d,%d), secdelta: %d," > + " nsecdelta: %d\n", > + testtimes[i][0], testtimes[i][1], > + secdelta, nsecdelta); > failure = 1; > } > } else { > -- > 2.18.1 > >
Li Wang <liwang@redhat.com> wrote: > On Thu, Jun 18, 2020 at 6:24 PM Jan Stancek <jstancek@redhat.com> wrote: > >> Test allows just 5ms delta for PASS, and test randomly fails in >> environments with shared resources and increased steal time. >> >> Relax the condition and also print deltas if test fails. >> Remove DEBUG ifdefs and main parameters to avoid unused variable >> warning. >> >> Signed-off-by: Jan Stancek <jstancek@redhat.com> >> --- >> .../functional/timers/clocks/invaliddates.c | 25 +++++++------------ >> 1 file changed, 9 insertions(+), 16 deletions(-) >> >> diff --git >> a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c >> b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c >> index face334fd250..d4116b1e9bc0 100644 >> --- >> a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c >> +++ >> b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c >> @@ -18,8 +18,7 @@ >> >> #define NUMTESTS 5... >> >> -#define ACCEPTABLESECDELTA 0 >> -#define ACCEPTABLENSECDELTA 5000000 >> +#define ACCEPTABLESECDELTA 2 >> > > It's hard to say what size of the second-delta is a proper value, but to > increase it can obviously decrease the failure probability. If there is no > better way I'd go with this patch. > But in another word, this test is just focused on invalid dates[1], so the deviation(caused by sharing resource) in seconds(even 10 seconds) bound is acceptable I think. If someone insists to make thing more precise, we can make use of function tst_is_virt() to apply this method only for virtual machines. But anyway that's not very important:). [1]. static int testtimes[NUMTESTS][2] = { {INT32_MAX, 999999999}, // large number {946713600, 999999999}, // Y2K - Jan 1, 2000 {951811200, 999999999}, // Y2K - Feb 29, 2000 {1078041600, 999999999}, // Y2K - Feb 29, 2004 {1049623200, 999999999}, // daylight savings 2003 };
Hi Jan Acked-by. ps: I like leaving a comment in delta, just my personal habit. Best Regards Yang Xu
Pushed.
diff --git a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c index face334fd250..d4116b1e9bc0 100644 --- a/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c +++ b/testcases/open_posix_testsuite/functional/timers/clocks/invaliddates.c @@ -18,8 +18,7 @@ #define NUMTESTS 5 -#define ACCEPTABLESECDELTA 0 -#define ACCEPTABLENSECDELTA 5000000 +#define ACCEPTABLESECDELTA 2 static int testtimes[NUMTESTS][2] = { {INT32_MAX, 999999999}, // large number @@ -29,7 +28,7 @@ static int testtimes[NUMTESTS][2] = { {1049623200, 999999999}, // daylight savings 2003 }; -int main(int argc, char *argv[]) +int main(void) { struct timespec tpset, tpget, tsreset; int secdelta, nsecdelta; @@ -44,9 +43,6 @@ int main(int argc, char *argv[]) for (i = 0; i < NUMTESTS; i++) { tpset.tv_sec = testtimes[i][0]; tpset.tv_nsec = testtimes[i][1]; -#ifdef DEBUG - printf("Test: %ds %dns\n", testtimes[i][0], testtimes[i][1]); -#endif if (clock_settime(CLOCK_REALTIME, &tpset) == 0) { if (clock_gettime(CLOCK_REALTIME, &tpget) == -1) { printf("Error in clock_gettime()\n"); @@ -58,16 +54,13 @@ int main(int argc, char *argv[]) nsecdelta = nsecdelta + 1000000000; secdelta = secdelta - 1; } -#ifdef DEBUG - printf("Delta: %ds %dns\n", secdelta, nsecdelta); -#endif - if ((secdelta > ACCEPTABLESECDELTA) || (secdelta < 0)) { - printf("clock does not appear to be set\n"); - failure = 1; - } - if ((nsecdelta > ACCEPTABLENSECDELTA) || - (nsecdelta < 0)) { - printf("clock does not appear to be set\n"); + + if ((secdelta > ACCEPTABLESECDELTA) + || (secdelta < 0)) { + printf("FAIL: test(%d,%d), secdelta: %d," + " nsecdelta: %d\n", + testtimes[i][0], testtimes[i][1], + secdelta, nsecdelta); failure = 1; } } else {
Test allows just 5ms delta for PASS, and test randomly fails in environments with shared resources and increased steal time. Relax the condition and also print deltas if test fails. Remove DEBUG ifdefs and main parameters to avoid unused variable warning. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- .../functional/timers/clocks/invaliddates.c | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-)