| Message ID | 20220127171455.9809-1-pvorel@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,1/1] lib: Print in summary also tests not run at all | expand |
Hi Petr > We count only tests which TCONF. In case tst_brk() some tests might be > skipped without even trying to run them), thus print them. > > Signed-off-by: Petr Vorel<pvorel@suse.cz> > --- > Hi, > > probably quite confusing to have "skipped" and "not run". > Maybe rename s/skipped/cancelled/ and s/not run/skipped/ ? > Or just to increase cound of skipped? > > E.g. instead of: > > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > Summary: > passed 8 > failed 0 > broken 0 > skipped 1 > warnings 0 > not run 5 > > have > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > Summary: > passed 8 > failed 0 > broken 0 > skipped 6 > warnings 0 > > Kind regards, > Petr > > lib/tst_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 844756fbd7..e5ea9e6165 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -811,6 +811,9 @@ static void do_exit(int ret) > fprintf(stderr, "broken %d\n", results->broken); > fprintf(stderr, "skipped %d\n", results->skipped); > fprintf(stderr, "warnings %d\n", results->warnings); > + fprintf(stderr, "not run %d\n", tst_test->tcnt - > + results->passed - results->failed - results->broken - > + results->skipped - results->warnings); IMO, these values are not necessary related. Refer to quotactl01.c( tst_test->tcnt is less than results->passed) Also, if you want to add a new tag, you should define the situation for it. Best Regards Yang Xu > } > > do_cleanup();
On Sun, Jan 30, 2022 at 11:44 AM xuyang2018.jy@fujitsu.com <xuyang2018.jy@fujitsu.com> wrote: > > Hi Petr > > We count only tests which TCONF. In case tst_brk() some tests might be > > skipped without even trying to run them), thus print them. > > > > Signed-off-by: Petr Vorel<pvorel@suse.cz> > > --- > > Hi, > > > > probably quite confusing to have "skipped" and "not run". > > Maybe rename s/skipped/cancelled/ and s/not run/skipped/ ? > > Or just to increase cound of skipped? Both fine to me, I slightly think "not-run" might more precise to describe that. > > > > E.g. instead of: > > > > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > > > Summary: > > passed 8 > > failed 0 > > broken 0 > > skipped 1 > > warnings 0 > > not run 5 > > > > have > > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > > > Summary: > > passed 8 > > failed 0 > > broken 0 > > skipped 6 > > warnings 0 > > > > Kind regards, > > Petr > > > > lib/tst_test.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/tst_test.c b/lib/tst_test.c > > index 844756fbd7..e5ea9e6165 100644 > > --- a/lib/tst_test.c > > +++ b/lib/tst_test.c > > @@ -811,6 +811,9 @@ static void do_exit(int ret) > > fprintf(stderr, "broken %d\n", results->broken); > > fprintf(stderr, "skipped %d\n", results->skipped); > > fprintf(stderr, "warnings %d\n", results->warnings); > > + fprintf(stderr, "not run %d\n", tst_test->tcnt - > > + results->passed - results->failed - results->broken - > > + results->skipped - results->warnings); > IMO, these values are not necessary related. Refer to quotactl01.c( > tst_test->tcnt is less than results->passed) That's because of the test looping 'tst_variant + 1' times. Maybe we can just multiply it eliminate the distractions? tst_test->tcnt * (tst_variant + 1) > > Also, if you want to add a new tag, you should define the situation for it. Theoritically yes, but the problem here is hard to count the not-run numbers. Because some of the test items will never be performed if test return by test environment unmatch. I'm sure we have quite lot of test doing like that. > > Best Regards > Yang Xu > > > } > > > > do_cleanup(); > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp >
Yes，but it still be wrong if we use serval TPASS in sub test case. Best Regards Yang Xu | | xuyang_jy_0410 | | 邮箱：xuyang_jy_0410@163.com | 签名由 网易邮箱大师 定制 On 01/30/2022 14:09, Li Wang wrote: On Sun, Jan 30, 2022 at 11:44 AM xuyang2018.jy@fujitsu.com <xuyang2018.jy@fujitsu.com> wrote: > > Hi Petr > > We count only tests which TCONF. In case tst_brk() some tests might be > > skipped without even trying to run them), thus print them. > > > > Signed-off-by: Petr Vorel<pvorel@suse.cz> > > --- > > Hi, > > > > probably quite confusing to have "skipped" and "not run". > > Maybe rename s/skipped/cancelled/ and s/not run/skipped/ ? > > Or just to increase cound of skipped? Both fine to me, I slightly think "not-run" might more precise to describe that. > > > > E.g. instead of: > > > > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > > > Summary: > > passed 8 > > failed 0 > > broken 0 > > skipped 1 > > warnings 0 > > not run 5 > > > > have > > bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) > > > > Summary: > > passed 8 > > failed 0 > > broken 0 > > skipped 6 > > warnings 0 > > > > Kind regards, > > Petr > > > > lib/tst_test.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/tst_test.c b/lib/tst_test.c > > index 844756fbd7..e5ea9e6165 100644 > > --- a/lib/tst_test.c > > +++ b/lib/tst_test.c > > @@ -811,6 +811,9 @@ static void do_exit(int ret) > > fprintf(stderr, "broken %d\n", results->broken); > > fprintf(stderr, "skipped %d\n", results->skipped); > > fprintf(stderr, "warnings %d\n", results->warnings); > > + fprintf(stderr, "not run %d\n", tst_test->tcnt - > > + results->passed - results->failed - results->broken - > > + results->skipped - results->warnings); > IMO, these values are not necessary related. Refer to quotactl01.c( > tst_test->tcnt is less than results->passed) That's because of the test looping 'tst_variant + 1' times. Maybe we can just multiply it eliminate the distractions? tst_test->tcnt * (tst_variant + 1) > > Also, if you want to add a new tag, you should define the situation for it. Theoritically yes, but the problem here is hard to count the not-run numbers. Because some of the test items will never be performed if test return by test environment unmatch. I'm sure we have quite lot of test doing like that. > > Best Regards > Yang Xu > > > } > > > > do_cleanup(); > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > -- Regards, Li Wang -- Mailing list info: https://lists.linux.it/listinfo/ltp
... > > +++ b/lib/tst_test.c > > @@ -811,6 +811,9 @@ static void do_exit(int ret) > > fprintf(stderr, "broken %d\n", results->broken); > > fprintf(stderr, "skipped %d\n", results->skipped); > > fprintf(stderr, "warnings %d\n", results->warnings); > > + fprintf(stderr, "not run %d\n", tst_test->tcnt - > > + results->passed - results->failed - results->broken - > > + results->skipped - results->warnings); > IMO, these values are not necessary related. Refer to quotactl01.c( > tst_test->tcnt is less than results->passed) Ah, correct. Also .test_all means tst_test->tcnt to be 0. So my patch is utterly wrong. > Also, if you want to add a new tag, you should define the situation for it. What is my point: number of the defined tests does not mean they are all run. Maybe printing number of the tests in the test (tst_test->tcnt or 1 when tst_test->test_all)? My point is to have an idea without looking into the code know how many tests have been skipped on certain setup. Kind regards, Petr > Best Regards > Yang Xu
. On Mon, Jan 31, 2022 at 3:21 PM Petr Vorel <pvorel@suse.cz> wrote: > > ... > > > +++ b/lib/tst_test.c > > > @@ -811,6 +811,9 @@ static void do_exit(int ret) > > > fprintf(stderr, "broken %d\n", results->broken); > > > fprintf(stderr, "skipped %d\n", results->skipped); > > > fprintf(stderr, "warnings %d\n", results->warnings); > > > + fprintf(stderr, "not run %d\n", tst_test->tcnt - > > > + results->passed - results->failed - results->broken - > > > + results->skipped - results->warnings); > > IMO, these values are not necessary related. Refer to quotactl01.c( > > tst_test->tcnt is less than results->passed) > Ah, correct. Also .test_all means tst_test->tcnt to be 0. > So my patch is utterly wrong. > > > Also, if you want to add a new tag, you should define the situation for it. That sounds like a complicated method, if we go add a new tag, we have to count the times for each arbitrary 'return' of the test. (Because it does not use TCONF) This obviously brings more work to the author and is not recommended IMHO. To strictly, every test should follow the principle of new LTP API, to list all test items in the 'struct tcase{ ... }', and with no tst_test->tcnt defined means that is only one test. Based on these, I think the total test number of a testcase (with 'tst_test->tcnt' defined) should be: (tst_test->tcnt * test_variants) > > What is my point: number of the defined tests does not mean they are all run. Agree, at least it makes sense for those tests explicitly defined tcnt. > > Maybe printing number of the tests in the test (tst_test->tcnt or 1 when > tst_test->test_all)? > > My point is to have an idea without looking into the code know how many tests > have been skipped on certain setup. Yes, I understand. So how about this way: --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -785,6 +785,8 @@ static void print_failure_hints(void) print_failure_hint("known-fail", "hit by known kernel failures", NULL); } +static unsigned int test_variants = 1; + static void do_exit(int ret) { if (results) { @@ -811,9 +813,11 @@ static void do_exit(int ret) fprintf(stderr, "broken %d\n", results->broken); fprintf(stderr, "skipped %d\n", results->skipped); fprintf(stderr, "warnings %d\n", results->warnings); - fprintf(stderr, "not run %d\n", tst_test->tcnt - - results->passed - results->failed - results->broken - - results->skipped - results->warnings); + if (tst_test->tcnt) { + fprintf(stderr, "not-run %d\n", (tst_test->tcnt * test_variants) - + results->passed - results->failed - results->broken - + results->skipped - results->warnings); + } } do_cleanup(); @@ -1529,7 +1533,6 @@ unsigned int tst_variant; void tst_run_tcases(int argc, char *argv[], struct tst_test *self) { int ret = 0; - unsigned int test_variants = 1; lib_pid = getpid(); tst_test = self; -- Regards, Li Wang
Hi Li, ... > Yes, I understand. So how about this way: > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -785,6 +785,8 @@ static void print_failure_hints(void) > print_failure_hint("known-fail", "hit by known kernel failures", NULL); > } > +static unsigned int test_variants = 1; > + > static void do_exit(int ret) > { > if (results) { > @@ -811,9 +813,11 @@ static void do_exit(int ret) > fprintf(stderr, "broken %d\n", results->broken); > fprintf(stderr, "skipped %d\n", results->skipped); > fprintf(stderr, "warnings %d\n", results->warnings); > - fprintf(stderr, "not run %d\n", tst_test->tcnt - > - results->passed - results->failed - > results->broken - > - results->skipped - results->warnings); > + if (tst_test->tcnt) { > + fprintf(stderr, "not-run %d\n", > (tst_test->tcnt * test_variants) - > + results->passed - > results->failed - results->broken - > + results->skipped - results->warnings); > + } > } > do_cleanup(); > @@ -1529,7 +1533,6 @@ unsigned int tst_variant; > void tst_run_tcases(int argc, char *argv[], struct tst_test *self) > { > int ret = 0; > - unsigned int test_variants = 1; > lib_pid = getpid(); > tst_test = self; Yep, ack this one, works well. Will you please send a proper patch? Kind regards, Petr
On Wed, Feb 2, 2022 at 1:39 AM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, > > ... > > Yes, I understand. So how about this way: > > > --- a/lib/tst_test.c > > +++ b/lib/tst_test.c > > @@ -785,6 +785,8 @@ static void print_failure_hints(void) > > print_failure_hint("known-fail", "hit by known kernel failures", NULL); > > } > > > +static unsigned int test_variants = 1; > > + > > static void do_exit(int ret) > > { > > if (results) { > > @@ -811,9 +813,11 @@ static void do_exit(int ret) > > fprintf(stderr, "broken %d\n", results->broken); > > fprintf(stderr, "skipped %d\n", results->skipped); > > fprintf(stderr, "warnings %d\n", results->warnings); > > - fprintf(stderr, "not run %d\n", tst_test->tcnt - > > - results->passed - results->failed - > > results->broken - > > - results->skipped - results->warnings); > > + if (tst_test->tcnt) { > > + fprintf(stderr, "not-run %d\n", > > (tst_test->tcnt * test_variants) - > > + results->passed - > > results->failed - results->broken - > > + results->skipped - results->warnings); > > + } > > } > > > do_cleanup(); > > @@ -1529,7 +1533,6 @@ unsigned int tst_variant; > > void tst_run_tcases(int argc, char *argv[], struct tst_test *self) > > { > > int ret = 0; > > - unsigned int test_variants = 1; > > > lib_pid = getpid(); > > tst_test = self; > > Yep, ack this one, works well. Will you please send a proper patch? Sure, but I'd hold this until Xu Yang review. In case I have something thoughtless. Ps. He might reply late since now is on China NewYear holidays.
On Sun, Jan 30, 2022 at 2:43 PM xuyang <xuyang_jy_0410@163.com> wrote: > Yes，but it still be wrong if we use serval TPASS in sub test case. > We can simply avoid that by skipping tst_test->forks_child. But in this case limited the scope of the "not-run" uses. TBH, I haven't come up with a perfect idea to satisfy all situations. + if (tst_test->tcnt && !tst_test->forks_child) { + fprintf(stderr, "not-run %d\n", (tst_test->tcnt * test_variants) - + results->passed - results->failed - results->broken - + results->skipped - results->warnings); + }
Hi Li, ... > > Yep, ack this one, works well. Will you please send a proper patch? > Sure, but I'd hold this until Xu Yang review. In case I have > something thoughtless. Sure > Ps. He might reply late since now is on China NewYear holidays. Enjoy the New Year and your vacation. Kind regards, Petr
Hi Li > On Wed, Feb 2, 2022 at 1:39 AM Petr Vorel<pvorel@suse.cz> wrote: >> >> Hi Li, >> >> ... >>> Yes, I understand. So how about this way: >> >>> --- a/lib/tst_test.c >>> +++ b/lib/tst_test.c >>> @@ -785,6 +785,8 @@ static void print_failure_hints(void) >>> print_failure_hint("known-fail", "hit by known kernel failures", NULL); >>> } >> >>> +static unsigned int test_variants = 1; >>> + >>> static void do_exit(int ret) >>> { >>> if (results) { >>> @@ -811,9 +813,11 @@ static void do_exit(int ret) >>> fprintf(stderr, "broken %d\n", results->broken); >>> fprintf(stderr, "skipped %d\n", results->skipped); >>> fprintf(stderr, "warnings %d\n", results->warnings); >>> - fprintf(stderr, "not run %d\n", tst_test->tcnt - >>> - results->passed - results->failed - >>> results->broken - >>> - results->skipped - results->warnings); >>> + if (tst_test->tcnt) { >>> + fprintf(stderr, "not-run %d\n", >>> (tst_test->tcnt * test_variants) - >>> + results->passed - >>> results->failed - results->broken - >>> + results->skipped - results->warnings); >>> + } >>> } In fact, we don't have mandatory rules that TAPSS or TFAIL only can occur one time. a example ie memcontrol02.c Best Regards Yang Xu >>> do_cleanup(); >>> @@ -1529,7 +1533,6 @@ unsigned int tst_variant; >>> void tst_run_tcases(int argc, char *argv[], struct tst_test *self) >>> { >>> int ret = 0; >>> - unsigned int test_variants = 1; >> >>> lib_pid = getpid(); >>> tst_test = self; >> >> Yep, ack this one, works well. Will you please send a proper patch? > > Sure, but I'd hold this until Xu Yang review. In case I have > something thoughtless. > > Ps. He might reply late since now is on China NewYear holidays. >
xuyang2018.jy@fujitsu.com <xuyang2018.jy@fujitsu.com> wrote: > In fact, we don't have mandatory rules that TAPSS or TFAIL only can > occur one time. a example ie memcontrol02.c > Right, that is my hesitant part for counting that. Seems many tests abuse the TPASS|TFAIL for defining test fail bound.
Hi Li, Xu, > > In fact, we don't have mandatory rules that TAPSS or TFAIL only can > > occur one time. a example ie memcontrol02.c > Right, that is my hesitant part for counting that. > Seems many tests abuse the TPASS|TFAIL for defining test fail bound. OK, while it'd be useful for some tests, it'd be confusing due this for other. I guess printing (tst_test->tcnt * test_variants) number can be confusing either. Kind regards, Petr
On Wed, Feb 9, 2022 at 8:53 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, Xu, > > > > In fact, we don't have mandatory rules that TAPSS or TFAIL only can > > > occur one time. a example ie memcontrol02.c > > > Right, that is my hesitant part for counting that. > > Seems many tests abuse the TPASS|TFAIL for defining test fail bound. > > OK, while it'd be useful for some tests, it'd be confusing due this for > other. > I guess printing (tst_test->tcnt * test_variants) number can be confusing > either. > Agree, so we might need more time coming up with a better solution. Or, we go another way to limit the abuse in TPASS|TFAIL in the test. But both sound not easy at this moment.
> On Wed, Feb 9, 2022 at 8:53 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, Xu, > > > > In fact, we don't have mandatory rules that TAPSS or TFAIL only can > > > > occur one time. a example ie memcontrol02.c > > > Right, that is my hesitant part for counting that. > > > Seems many tests abuse the TPASS|TFAIL for defining test fail bound. > > OK, while it'd be useful for some tests, it'd be confusing due this for > > other. > > I guess printing (tst_test->tcnt * test_variants) number can be confusing > > either. > Agree, so we might need more time coming up with a better solution. > Or, we go another way to limit the abuse in TPASS|TFAIL in the test. > But both sound not easy at this moment. Given number of the tests we have (and how many of them we need to rewrite) it'd be hard (and probably not worth of doing it). Kind regards, Petr
diff --git a/lib/tst_test.c b/lib/tst_test.c index 844756fbd7..e5ea9e6165 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -811,6 +811,9 @@ static void do_exit(int ret) fprintf(stderr, "broken %d\n", results->broken); fprintf(stderr, "skipped %d\n", results->skipped); fprintf(stderr, "warnings %d\n", results->warnings); + fprintf(stderr, "not run %d\n", tst_test->tcnt - + results->passed - results->failed - results->broken - + results->skipped - results->warnings); } do_cleanup();
We count only tests which TCONF. In case tst_brk() some tests might be skipped without even trying to run them), thus print them. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, probably quite confusing to have "skipped" and "not run". Maybe rename s/skipped/cancelled/ and s/not run/skipped/ ? Or just to increase cound of skipped? E.g. instead of: bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) Summary: passed 8 failed 0 broken 0 skipped 1 warnings 0 not run 5 have bind05.c:132: TCONF: socket(10, 2, 0) failed: EAFNOSUPPORT (97) Summary: passed 8 failed 0 broken 0 skipped 6 warnings 0 Kind regards, Petr lib/tst_test.c | 3 +++ 1 file changed, 3 insertions(+)