Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[David Marchand <david.marchand@redhat.com>]

On Thu, Oct 17, 2019 at 3:16 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Amit Gupta <agupta3@marvell.com> writes:
>
> >> -----Original Message-----
> >> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> >> Sent: Friday, September 13, 2019 9:17 PM
> >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> >> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> >> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> >> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> >> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> >> to dpdk perf testsuite
> >>
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> <snip>
> >>
> >> > >
> >> > > <agupta3@marvell.com> writes:
> >> > >
> >> > > > From: Amit Gupta <agupta3@marvell.com>
> >> > > >
> >> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> >> > > > finish this test was much longer compare to time required for fast
> >> > > > tests(10s). Hence, the test is being renamed moved to perf test
> >> > > > category for its execution to complete.
> >> > > >
> >> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >> > > > ---
> >> > >
> >> > > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
> >> > > timing out with 30s instead of the default 10.  We do this because
> >> > > occasionally the lpm6 and table tests would also exceed the 10s
> >> > > timeout in the travis environment.  I agree, it's better to pull the
> >> > > perf part
> >> > of tests out.
> >> > >
> >> > > I think there isn't any additional functional test in this readwrite - is that
> >> so?
> >> > > If it is, then we need to also prioritize adding back in some of the
> >> > > functional testing.  Maybe I misread the lf_autotest, though.
> >> > >
> >> > [Wang, Yipeng]
> >> > Yes that is my concern too, if we just move all the lock-free test
> >> > into perf test then we miss the functional test.
> >> > Would any of you like to consider adding a small functional test into
> >> > the readwrite or readwrite_lf_functional?
> >> > That would be great :)
> >> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> >> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> >> instead for 19.11, this would serve both the purposes.
> >>
> >> Amit, would it be possible to check what tests will run within the timeout
> >> period?
> >> >
> > @Wang, Yipeng1, is it good if we do the change as @Honnappa
> > Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to
> > 'hash_readwrite_lf_perf_autotest' for the time being and later once
> > have sufficient bandwidth we can move only perf part of the test to
> > perf tests.
>
> NAK.
>
> I don't like that proposal.  While it's true that there are occasional
> TIMEOUT failures with the current setup, I'd much prefer these timeouts
> (which we can easily distinguish) to removing the test from the travis
> chain.  My understanding is that there *are* some functionality being
> exercised by this test that isn't exercised elsewhere.  I'd prefer we
> don't sacrifice the coverage.

+1 and marking this patch as rejected.

On a sidenote, Amit, please be careful about the versioning of your
patches and update their status in patchwork.
I had two patches named the same with one marked as NEW (but no
comment on it) and this current thread patch marked as SUPERSEDED.

Thanks.

-- 
David Marchand