On Wed, Dec 13, 2017 at 04:54:29PM +0530, Pavan Nikhilesh Bhagavatula wrote:

On Wed, Dec 13, 2017 at 10:34:28AM +0000, Bruce Richardson wrote:

quoted

On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:

quoted

quoted

-----Original Message-----
From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
Sent: Tuesday, December 12, 2017 7:27 PM
To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
[off-list ref]; Van Haaren, Harry
[off-list ref]; Eads, Gage [off-list ref];
hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
[off-list ref]
Cc: dev@dpdk.org; Pavan Nikhilesh <redacted>
Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
to driver

Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
driver/event/octeontx.

<snip patch content>

Replying to 1st patch, as no cover letter;

Summary of patchset:
- Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
- Enable self tests to run when passed the vdev arg "self-test=1"


A few comments on this change;

1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.

From discussion on #IRC, it seems reasonable to call  rte_eal_vdev_init()  with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.

2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.


Specific comments will be sent as replies to the patches. Cheers, -Harry

What I gather from a cursory glance at this set is that the self tests
are designed to be triggered via devargs to the device driver, correct?
I'm not sure I like this approach, though I do agree with having the
tests inside the individual drivers.

What I think I would prefer to see is the self-tests being called via an
API rather than via devargs. I think we should add a
"rte_event_dev_self_test()" API to the eventdev library, and have that
then call into the driver-provided tests. This means that self-tests can
only be called by applications which are set up to allow the tests to be
called, e.g. the autotest binary, while also avoiding the issue of
having lots of driver specifics clutter up test binaries.

Agreed, will modify it to ops based scheme so that application can call driver
specific `event_dev_self_test` and register selftest in
test/test/test_eventdev.c.

Although we would like to retain devargs selftest scheme for event_octeontx. I
will remove it for event_sw. Does that sound good?

Sure, sounds good to me.

Thanks,
/Bruce