[igt-dev] [PATCH i-g-t] tests/i915/gem_shrink: Fix memory requirement assertion

[Petri Latvala]

On Mon, Nov 21, 2022 at 09:44:37AM +0000, Matthew Auld wrote:
> On 21/11/2022 09:35, Petri Latvala wrote:
> > On Mon, Nov 21, 2022 at 09:12:57AM +0000, Matthew Auld wrote:
> > > On 18/11/2022 19:14, Niranjana Vishwanathapura wrote:
> > > > On Fri, Nov 18, 2022 at 02:54:40PM +0000, Matthew Auld wrote:
> > > > > On 15/11/2022 19:05, Niranjana Vishwanathapura wrote:
> > > > > > The reclaim subtest do not require the alloc_size memory
> > > > > > allocations. Ensure the memory requirement assert is only
> > > > > > applied for other subtests.
> > > > > > 
> > > > > > Signed-off-by: Niranjana Vishwanathapura
> > > > > > <niranjana.vishwanathapura at intel.com>
> > > > > > ---
> > > > > >   tests/i915/gem_shrink.c | 8 ++++----
> > > > > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/i915/gem_shrink.c b/tests/i915/gem_shrink.c
> > > > > > index e3e20dfc9..380d2c846 100644
> > > > > > --- a/tests/i915/gem_shrink.c
> > > > > > +++ b/tests/i915/gem_shrink.c
> > > > > > @@ -456,9 +456,6 @@ igt_main
> > > > > >                num_processes, alloc_size);
> > > > > >           alloc_size <<= 20;
> > > > > > -        igt_require_memory(num_processes, alloc_size,
> > > > > > -                   CHECK_SWAP | CHECK_RAM);
> > > > > > -
> > > > > 
> > > > > If that's the case then all the other stuff in the fixture is not
> > > > > really used either (like alloc_size, num_processes etc). Maybe just
> > > > > move the fixture instead?
> > > > > 
> > > > > igt_subtest_group {
> > > > >      igt_fixture {
> > > > > 
> > > > >      }
> > > > > 
> > > > >      for(const struct test *t = tests; t->name; t++) {
> > > > >      ....
> > > > > }
> > > > > 
> > > > > igt_subtest("reclaim")
> > > > > ...
> > > > > 
> > > > 
> > > > Ok, but the igt_fixture() inside the igt_subtest_group still gets executed
> > > > when we run reclaim subtest (with '--r reclaim').
> > > 
> > > Petri, do you know if that's expected?
> > 
> > Yes, all fixtures get executed.
> 
> Ok, so it's not really possible to have a sub-fixture, which is only run for
> a particular set of subtests? In such a case is a completely new test the
> correct approach?

Yeah that's not possible with the current architecture. We have no way
of neatly tying a fixture to a set of subtests.


> 
> I recently needed something similar here:
> https://patchwork.freedesktop.org/patch/511628/?series=110908&rev=2
> 
> But I guess that should rather be a new test? Or is there a better approach?

What you have in that fixture can just be inside the subtest. A
fixture is really for _common_ (shared) setup code.

(A fixture after a test on the other hand is useful for teardown,
since failing an igt_assert/igt_require jumps out of the subtest
immediately)


-- 
Petri Latvala