Hi Sughosh,

On Mon, 29 Jul 2024 at 02:56, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>
> On Fri, 26 Jul 2024 at 05:03, Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 24 Jul 2024 at 00:05, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> > >
> > > Add the LMB unit tests under a separate class of tests. The LMB tests
> > > involve changing the LMB's memory map. With the memory map now
> > > persistent and global, running these tests has a side effect and
> > > impact any subsequent tests. Run these tests separately so that the
> > > system can be reset on completion of these tests.
> > >
> >
> > This is just not a good idea...we should fix the tests so they don't
> > have these side effects.
>
> But this test *will* have side-effects -- if we are making the lmb
> memory maps persistent, testing that code will have an impact. There
> is no getting around it. Btw, I got this idea based on how the VBE
> tests are run. This is similar to what is being done for the VBE
> tests.

You can set up the state before the test and restore it afterwards.
That is what we do with other tests and it is why you should have the
lmb state in a struct. See for example state_reset_for_test() which is
called if the test has a UT_TESTF_DM flag.

You can add a save/restore function (since you don't want a lmb pointer).

The _norun thing is because the tests are written in C but need some
Python setup. So the setup is done and then the test is run
immediately afterwards.

>
> As an aside, what issue do you see with running these tests separately?

They don't run with the 'ut' command so must be done specially. There
is no Python setup needed. We have DM tests which save and restore all
sorts of things...so we should do the same with lmb.


>
> -sughosh
>
> >
> > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> > > ---
> > > Changes since rfc: None
> > >
> > >  include/test/suites.h        |  1 +
> > >  test/Kconfig                 |  9 ++++++
> > >  test/Makefile                |  1 +
> > >  test/cmd_ut.c                |  7 +++++
> > >  test/lib/Makefile            |  1 -
> > >  test/{lib/lmb.c => lmb_ut.c} | 53 ++++++++++++++++++++++--------------
> > >  6 files changed, 50 insertions(+), 22 deletions(-)
> > >  rename test/{lib/lmb.c => lmb_ut.c} (93%)

- Simon

Reply via email to