| Message ID | 20190215145239.51806-2-cristian.marussi@arm.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | cgroup_regression_test.sh fix | expand |
Hi Cristian, > Using TWARN to report that a specific sub-testcase is skipped > caused the whole cgroup_regression_test.sh to be reported as > FAILING. Using TCONF instead. Fixes: 78bfa7e78 ("cgroup: Simplify check_kernel_bug usage") My commit, sorry :(. > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > --- > testcases/kernel/controllers/cgroup/cgroup_regression_test.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh > index 686860923..e197f5d3f 100755 > --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh > +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh > @@ -408,7 +408,7 @@ test_7_2() > grep -q -w "cpu" /proc/cgroups > if [ $? -ne 0 -o ! -e /proc/sched_debug ]; then > - tst_res TWARN "skip rest of testing due possible oops triggered by reading /proc/sched_debug" > + tst_res TCONF "skip rest of testing due possible oops triggered by reading /proc/sched_debug" Right, TWARN is not good (IMHO TWARN is not popular in LTP testsuites due marking test failing), but if you don't mind I'd merge it with TBROK (more appropriate in this case). Other option would be to use TINFO and don't skip test (we're trying to find kernel crashes, don't we), that's probably a bit crazy. Kind regards, Petr
On 26/02/2019 18:42, Petr Vorel wrote: > Hi Cristian, > >> Using TWARN to report that a specific sub-testcase is skipped >> caused the whole cgroup_regression_test.sh to be reported as >> FAILING. Using TCONF instead. > > Fixes: 78bfa7e78 ("cgroup: Simplify check_kernel_bug usage") > My commit, sorry :(. > No worries...:> >> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> >> --- >> testcases/kernel/controllers/cgroup/cgroup_regression_test.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh >> index 686860923..e197f5d3f 100755 >> --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh >> +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh >> @@ -408,7 +408,7 @@ test_7_2() > >> grep -q -w "cpu" /proc/cgroups >> if [ $? -ne 0 -o ! -e /proc/sched_debug ]; then >> - tst_res TWARN "skip rest of testing due possible oops triggered by reading /proc/sched_debug" >> + tst_res TCONF "skip rest of testing due possible oops triggered by reading /proc/sched_debug" > Right, TWARN is not good (IMHO TWARN is not popular in LTP testsuites due marking test failing), > but if you don't mind I'd merge it with TBROK (more appropriate in this case). mmm...I'm NOT so sure that a FAIL should be the outcome here ... we are skipping if cpu is not amongst cgroups or sched_debug is NOT available...BUT at this point we have run (or skipped) 7 tests so far and there are 3 more to go...so why can't we just TCONF ? (beside the fact that as I said in the cover-letter one TCONF will lead to the whole 10-subtest croup suite to be reported as TCONF...) Regards Cristian > > Other option would be to use TINFO and don't skip test (we're trying to find > kernel crashes, don't we), that's probably a bit crazy. > > Kind regards, > Petr >
Hi Cristian, ... > >> +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh > >> @@ -408,7 +408,7 @@ test_7_2() > >> grep -q -w "cpu" /proc/cgroups > >> if [ $? -ne 0 -o ! -e /proc/sched_debug ]; then > >> - tst_res TWARN "skip rest of testing due possible oops triggered by reading /proc/sched_debug" > >> + tst_res TCONF "skip rest of testing due possible oops triggered by reading /proc/sched_debug" > > Right, TWARN is not good (IMHO TWARN is not popular in LTP testsuites due marking test failing), > > but if you don't mind I'd merge it with TBROK (more appropriate in this case). > mmm...I'm NOT so sure that a FAIL should be the outcome here ... we are skipping > if cpu is not amongst cgroups or sched_debug is NOT available...BUT at this > point we have run (or skipped) 7 tests so far and there are 3 more to go...so > why can't we just TCONF ? OK I got that :) /proc/sched_debug is enabled by CONFIG_SCHED_DEBUG, os it's kind of configuration issue. Before I thought that failing "grep -q -w "cpu" /proc/cgroups" means failure/bug found by test, but it's not. So I'm ok with this change. Acked-by: Petr Vorel <pvorel@suse.cz> > (beside the fact that as I said in the cover-letter one TCONF will lead to the > whole 10-subtest croup suite to be reported as TCONF...) IMHO that's not true (see my reply to cover letter). Kind regards, Petr
Hi Cristian, thanks for your work, merged (with my comments in commit message). Kind regards, Petr
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh index 686860923..e197f5d3f 100755 --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh @@ -408,7 +408,7 @@ test_7_2() grep -q -w "cpu" /proc/cgroups if [ $? -ne 0 -o ! -e /proc/sched_debug ]; then - tst_res TWARN "skip rest of testing due possible oops triggered by reading /proc/sched_debug" + tst_res TCONF "skip rest of testing due possible oops triggered by reading /proc/sched_debug" return fi
Using TWARN to report that a specific sub-testcase is skipped caused the whole cgroup_regression_test.sh to be reported as FAILING. Using TCONF instead. Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- testcases/kernel/controllers/cgroup/cgroup_regression_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)