| Message ID | 20190808064353.8410-1-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | gard: Skip tests on FSP platform | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (0e1db80c70477d89a73c7f2a1a7e19c7d8292c5f) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > > gard is supported on OpenPower platform only. Skip gard > tests on FSP based system. > > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > external/gard/test/test-gard | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard > index 10da35156..e577e0c16 100755 > --- a/external/gard/test/test-gard > +++ b/external/gard/test/test-gard > @@ -1,5 +1,10 @@ > #! /bin/sh > > + > +if [ -d "/proc/device-tree/fsps" ] ; then > + exit 0 > +fi Doesn't this just run the units tests? They're self contained and shouldn't be touching the host PNOR even when run on an OpenPower system. > + > . test/test.sh > > run_tests "test/tests/*" "test/results" > -- > 2.14.3 >
On 08/08/2019 12:52 PM, Oliver O'Halloran wrote: > On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde > <hegdevasant@linux.vnet.ibm.com> wrote: >> >> gard is supported on OpenPower platform only. Skip gard >> tests on FSP based system. >> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> --- >> external/gard/test/test-gard | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard >> index 10da35156..e577e0c16 100755 >> --- a/external/gard/test/test-gard >> +++ b/external/gard/test/test-gard >> @@ -1,5 +1,10 @@ >> #! /bin/sh >> >> + >> +if [ -d "/proc/device-tree/fsps" ] ; then >> + exit 0 >> +fi > > Doesn't this just run the units tests? They're self contained and > shouldn't be touching the host PNOR even when run on an OpenPower > system. But we have is_fsp() check inside main(). So its failing. Alternatively we can consider skipping is_fsp() check whenever user passes `-f` option. But then we have to re-arrange the code (parse command line argument first and then check is_fsp()). This may results in output behaviour change. -Vasant
On Thu, Aug 8, 2019, at 8:13 PM, Vasant Hegde wrote: > On 08/08/2019 12:52 PM, Oliver O'Halloran wrote: > > On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde > > <hegdevasant@linux.vnet.ibm.com> wrote: > >> > >> gard is supported on OpenPower platform only. Skip gard > >> tests on FSP based system. > >> > >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > >> --- > >> external/gard/test/test-gard | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard > >> index 10da35156..e577e0c16 100755 > >> --- a/external/gard/test/test-gard > >> +++ b/external/gard/test/test-gard > >> @@ -1,5 +1,10 @@ > >> #! /bin/sh > >> > >> + > >> +if [ -d "/proc/device-tree/fsps" ] ; then > >> + exit 0 > >> +fi > > > > Doesn't this just run the units tests? They're self contained and > > shouldn't be touching the host PNOR even when run on an OpenPower > > system. > > But we have is_fsp() check inside main(). So its failing. > > Alternatively we can consider skipping is_fsp() check whenever user > passes `-f` > option. > But then we have to re-arrange the code (parse command line argument > first and then > check is_fsp()). This may results in output behaviour change. My guess is we're also not running the gard tests as part of 'make check' then? Or we're ignoring the result.
On 08/09/2019 06:10 AM, Stewart Smith wrote: > On Thu, Aug 8, 2019, at 8:13 PM, Vasant Hegde wrote: >> On 08/08/2019 12:52 PM, Oliver O'Halloran wrote: >>> On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde >>> <hegdevasant@linux.vnet.ibm.com> wrote: >>>> .../... > > My guess is we're also not running the gard tests as part of 'make check' then? Or we're ignoring the result. I think we didn't run tests on FSP based system. It works fine on other systems. -Vasant
On 8/8/19 3:42 PM, Vasant Hegde wrote: > On 08/08/2019 12:52 PM, Oliver O'Halloran wrote: >> On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde >> <hegdevasant@linux.vnet.ibm.com> wrote: >>> >>> gard is supported on OpenPower platform only. Skip gard >>> tests on FSP based system. >>> >>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >>> --- >>> external/gard/test/test-gard | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard >>> index 10da35156..e577e0c16 100755 >>> --- a/external/gard/test/test-gard >>> +++ b/external/gard/test/test-gard >>> @@ -1,5 +1,10 @@ >>> #! /bin/sh >>> >>> + >>> +if [ -d "/proc/device-tree/fsps" ] ; then >>> + exit 0 >>> +fi >> >> Doesn't this just run the units tests? They're self contained and >> shouldn't be touching the host PNOR even when run on an OpenPower >> system. > > But we have is_fsp() check inside main(). So its failing. > > Alternatively we can consider skipping is_fsp() check whenever user passes `-f` > option. > But then we have to re-arrange the code (parse command line argument first and then > check is_fsp()). This may results in output behaviour change. Oliver, Are you ok with fixing test case (what this patch) -OR- do you prefer to skip is_fsp() check inside code itself? -Vasant
On Tue, Aug 27, 2019 at 8:28 PM Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > > On 8/8/19 3:42 PM, Vasant Hegde wrote: > > On 08/08/2019 12:52 PM, Oliver O'Halloran wrote: > >> On Thu, Aug 8, 2019 at 4:44 PM Vasant Hegde > >> <hegdevasant@linux.vnet.ibm.com> wrote: > >>> > >>> gard is supported on OpenPower platform only. Skip gard > >>> tests on FSP based system. > >>> > >>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > >>> --- > >>> external/gard/test/test-gard | 5 +++++ > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard > >>> index 10da35156..e577e0c16 100755 > >>> --- a/external/gard/test/test-gard > >>> +++ b/external/gard/test/test-gard > >>> @@ -1,5 +1,10 @@ > >>> #! /bin/sh > >>> > >>> + > >>> +if [ -d "/proc/device-tree/fsps" ] ; then > >>> + exit 0 > >>> +fi > >> > >> Doesn't this just run the units tests? They're self contained and > >> shouldn't be touching the host PNOR even when run on an OpenPower > >> system. > > > > But we have is_fsp() check inside main(). So its failing. > > > > Alternatively we can consider skipping is_fsp() check whenever user passes `-f` > > option. > > But then we have to re-arrange the code (parse command line argument first and then > > check is_fsp()). This may results in output behaviour change. > > Oliver, > Are you ok with fixing test case (what this patch) -OR- do you prefer to skip > is_fsp() check inside code itself? We should check is_fsp() if we weren't given an explicit file argument. If there's no file argument the default behaviour is to try open the system's PNOR which is nonsensical on FSP systems. We want to exit with a useful error in that case so we still need to do the check in the default case. > > -Vasant >
diff --git a/external/gard/test/test-gard b/external/gard/test/test-gard index 10da35156..e577e0c16 100755 --- a/external/gard/test/test-gard +++ b/external/gard/test/test-gard @@ -1,5 +1,10 @@ #! /bin/sh + +if [ -d "/proc/device-tree/fsps" ] ; then + exit 0 +fi + . test/test.sh run_tests "test/tests/*" "test/results"
gard is supported on OpenPower platform only. Skip gard tests on FSP based system. Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- external/gard/test/test-gard | 5 +++++ 1 file changed, 5 insertions(+)