mbox series

[0/4] RFC: consolidate testing apps to app dir

Message ID 20190125202642.66559-1-bruce.richardson@intel.com (mailing list archive)
Headers
Series RFC: consolidate testing apps to app dir |

Message

Bruce Richardson Jan. 25, 2019, 8:26 p.m. UTC
  The apps for testing are split between the "test" and "app" directories,
with a not-very-clear distinction between the two (at least to my mind).

Given how the apps are being built, the easiest path to having
cmdline_test, test-acl and test-pipeline build using meson is to
consolidate all these apps back into the app folder and use the logic
there. The bpf folder doesn't contain an actual application, but
rather example bpf code which can be loaded into testpmd. That is
possibly best moved to the examples folder, but I'm open to feedback
on the best place for it.

With these changes, the "test" folder then becomes the location for
unit tests only.


Bruce Richardson (4):
  test/cmdline_test: move to app directory
  test/test-acl: move to app directory
  test/test-pipeline: move to app directory
  test/bpf: move to examples folder

 app/Makefile                                    |  3 +++
 {test => app}/cmdline_test/Makefile             |  0
 {test => app}/cmdline_test/cmdline_test.c       |  0
 {test => app}/cmdline_test/cmdline_test.h       |  0
 {test => app}/cmdline_test/cmdline_test.py      |  0
 {test => app}/cmdline_test/cmdline_test_data.py |  0
 {test => app}/cmdline_test/commands.c           |  0
 app/cmdline_test/meson.build                    |  5 +++++
 app/meson.build                                 |  5 ++++-
 {test => app}/test-acl/Makefile                 |  0
 {test => app}/test-acl/main.c                   |  0
 app/test-acl/meson.build                        |  5 +++++
 {test => app}/test-pipeline/Makefile            |  0
 {test => app}/test-pipeline/config.c            |  0
 {test => app}/test-pipeline/init.c              |  0
 {test => app}/test-pipeline/main.c              |  0
 {test => app}/test-pipeline/main.h              |  0
 app/test-pipeline/meson.build                   | 14 ++++++++++++++
 {test => app}/test-pipeline/pipeline_acl.c      |  0
 {test => app}/test-pipeline/pipeline_hash.c     |  0
 {test => app}/test-pipeline/pipeline_lpm.c      |  0
 {test => app}/test-pipeline/pipeline_lpm_ipv6.c |  0
 {test => app}/test-pipeline/pipeline_stub.c     |  0
 {test => app}/test-pipeline/runtime.c           |  0
 doc/guides/testpmd_app_ug/testpmd_funcs.rst     |  8 ++++----
 examples/bpf/README                             |  8 ++++++++
 {test => examples}/bpf/dummy.c                  |  0
 {test => examples}/bpf/mbuf.h                   |  0
 examples/bpf/meson.build                        |  6 ++++++
 {test => examples}/bpf/t1.c                     |  0
 {test => examples}/bpf/t2.c                     |  0
 {test => examples}/bpf/t3.c                     |  0
 test/Makefile                                   |  3 ---
 33 files changed, 49 insertions(+), 8 deletions(-)
 rename {test => app}/cmdline_test/Makefile (100%)
 rename {test => app}/cmdline_test/cmdline_test.c (100%)
 rename {test => app}/cmdline_test/cmdline_test.h (100%)
 rename {test => app}/cmdline_test/cmdline_test.py (100%)
 rename {test => app}/cmdline_test/cmdline_test_data.py (100%)
 rename {test => app}/cmdline_test/commands.c (100%)
 create mode 100644 app/cmdline_test/meson.build
 rename {test => app}/test-acl/Makefile (100%)
 rename {test => app}/test-acl/main.c (100%)
 create mode 100644 app/test-acl/meson.build
 rename {test => app}/test-pipeline/Makefile (100%)
 rename {test => app}/test-pipeline/config.c (100%)
 rename {test => app}/test-pipeline/init.c (100%)
 rename {test => app}/test-pipeline/main.c (100%)
 rename {test => app}/test-pipeline/main.h (100%)
 create mode 100644 app/test-pipeline/meson.build
 rename {test => app}/test-pipeline/pipeline_acl.c (100%)
 rename {test => app}/test-pipeline/pipeline_hash.c (100%)
 rename {test => app}/test-pipeline/pipeline_lpm.c (100%)
 rename {test => app}/test-pipeline/pipeline_lpm_ipv6.c (100%)
 rename {test => app}/test-pipeline/pipeline_stub.c (100%)
 rename {test => app}/test-pipeline/runtime.c (100%)
 create mode 100644 examples/bpf/README
 rename {test => examples}/bpf/dummy.c (100%)
 rename {test => examples}/bpf/mbuf.h (100%)
 create mode 100644 examples/bpf/meson.build
 rename {test => examples}/bpf/t1.c (100%)
 rename {test => examples}/bpf/t2.c (100%)
 rename {test => examples}/bpf/t3.c (100%)
  

Comments

Bruce Richardson Jan. 25, 2019, 8:30 p.m. UTC | #1
On Fri, Jan 25, 2019 at 08:26:38PM +0000, Bruce Richardson wrote:
> The apps for testing are split between the "test" and "app" directories,
> with a not-very-clear distinction between the two (at least to my mind).
> 
> Given how the apps are being built, the easiest path to having
> cmdline_test, test-acl and test-pipeline build using meson is to
> consolidate all these apps back into the app folder and use the logic
> there. The bpf folder doesn't contain an actual application, but
> rather example bpf code which can be loaded into testpmd. That is
> possibly best moved to the examples folder, but I'm open to feedback
> on the best place for it.
> 
> With these changes, the "test" folder then becomes the location for
> unit tests only.
> 
> 
> Bruce Richardson (4):
>   test/cmdline_test: move to app directory
>   test/test-acl: move to app directory
>   test/test-pipeline: move to app directory
>   test/bpf: move to examples folder
> 

Although these patches are for 19.05, I'm sending them now for some early
review, as I think we need a deprecation notice for this change in 19.02 if
these changes are acceptable to people.

Therefore, please review and let me know any issues people see with this
change.

/Bruce
  
Thomas Monjalon Jan. 29, 2019, 11:40 a.m. UTC | #2
25/01/2019 21:30, Bruce Richardson:
> On Fri, Jan 25, 2019 at 08:26:38PM +0000, Bruce Richardson wrote:
> > The apps for testing are split between the "test" and "app" directories,
> > with a not-very-clear distinction between the two (at least to my mind).
> > 
> > Given how the apps are being built, the easiest path to having
> > cmdline_test, test-acl and test-pipeline build using meson is to
> > consolidate all these apps back into the app folder and use the logic
> > there. The bpf folder doesn't contain an actual application, but
> > rather example bpf code which can be loaded into testpmd. That is
> > possibly best moved to the examples folder, but I'm open to feedback
> > on the best place for it.
> > 
> > With these changes, the "test" folder then becomes the location for
> > unit tests only.
> > 
> > 
> > Bruce Richardson (4):
> >   test/cmdline_test: move to app directory
> >   test/test-acl: move to app directory
> >   test/test-pipeline: move to app directory
> >   test/bpf: move to examples folder
> > 
> 
> Although these patches are for 19.05, I'm sending them now for some early
> review, as I think we need a deprecation notice for this change in 19.02 if
> these changes are acceptable to people.

We don't need a deprecation notice, as it is an internal change
without any impact for the users.

> Therefore, please review and let me know any issues people see with this
> change.

We should move all test apps into the same directory.
Is there any benefit of keeping unit tests separate?
It looks more logic to move everything in test/
but it would make the git history of testpmd more complex.
The other choice is to move all tests in app/.

The other split to discuss is between app/ and usertools/.
We could move user-oriented apps to usertools.
pdump and proc-info fall into this basket because they
can be used for debugging purpose.
  
Bruce Richardson Jan. 29, 2019, 11:52 a.m. UTC | #3
On Tue, Jan 29, 2019 at 12:40:37PM +0100, Thomas Monjalon wrote:
> 25/01/2019 21:30, Bruce Richardson:
> > On Fri, Jan 25, 2019 at 08:26:38PM +0000, Bruce Richardson wrote:
> > > The apps for testing are split between the "test" and "app" directories,
> > > with a not-very-clear distinction between the two (at least to my mind).
> > > 
> > > Given how the apps are being built, the easiest path to having
> > > cmdline_test, test-acl and test-pipeline build using meson is to
> > > consolidate all these apps back into the app folder and use the logic
> > > there. The bpf folder doesn't contain an actual application, but
> > > rather example bpf code which can be loaded into testpmd. That is
> > > possibly best moved to the examples folder, but I'm open to feedback
> > > on the best place for it.
> > > 
> > > With these changes, the "test" folder then becomes the location for
> > > unit tests only.
> > > 
> > > 
> > > Bruce Richardson (4):
> > >   test/cmdline_test: move to app directory
> > >   test/test-acl: move to app directory
> > >   test/test-pipeline: move to app directory
> > >   test/bpf: move to examples folder
> > > 
> > 
> > Although these patches are for 19.05, I'm sending them now for some early
> > review, as I think we need a deprecation notice for this change in 19.02 if
> > these changes are acceptable to people.
> 
> We don't need a deprecation notice, as it is an internal change
> without any impact for the users.
> 

Yes, good point. With the make build system these test apps end up in the
app folder anyway. With meson, they aren't yet built, so either way it
isn't a user-visible change.

> > Therefore, please review and let me know any issues people see with this
> > change.
> 
> We should move all test apps into the same directory.
> Is there any benefit of keeping unit tests separate?
> It looks more logic to move everything in test/
> but it would make the git history of testpmd more complex.
> The other choice is to move all tests in app/.
> 

I'd prefer to keep things in app, for legacy reasons more than anything
else. Not sure about moving the unit tests there too - I sort of like
having them in a separate test directory. However, if others generally
prefer that they too get consolidated into app, I won't complain! :-)

If we do end up moving the tests, another thing to consider is whether we
want to split them up by topic or suite. There are now a lot of C files in
the test folder, which suggests to me we need to split things a little.

> The other split to discuss is between app/ and usertools/.
> We could move user-oriented apps to usertools.
> pdump and proc-info fall into this basket because they
> can be used for debugging purpose.
> 

Yes. However, is this better handled via the installation rather than the
build? Even if the source code is moved to the usertools directory, the
actual binaries are going to be placed in the build folder, separate from
the scripts anyway. I'd suggest leaving them as they are, though perhaps
putting in a symlink to them from the usertools folder to make them easier
to find.

/Bruce