Re: [PATCH net-next v2 3/7] selftests: net: add scaffolding for Netlink tests in Python

[Petr Machata <petrm@nvidia.com>]

Jakub Kicinski <kuba@kernel.org> writes:

> Add glue code for accessing the YNL library which lives under
> tools/net and YAML spec files from under Documentation/.
> Automatically figure out if tests are run in tree or not.
> Since we'll want to use this library both from net and
> drivers/net test targets make the library a target as well,
> and automatically include it when net or drivers/net are
> included. Making net/lib a target ensures that we end up
> with only one copy of it, and saves us some path guessing.
>
> Add a tiny bit of formatting support to be able to output KTAP
> from the start.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - include the net/lib only in install
>  - support passing state to tests
>  - don't apply Path() on what's already a Path()
>  - fix spacing in Makefile's filter()
>  - sort imports
>
> CC: shuah@kernel.org
> CC: linux-kselftest@vger.kernel.org
> ---
>  tools/testing/selftests/Makefile              |  9 +-
>  tools/testing/selftests/net/lib/Makefile      |  8 ++
>  .../testing/selftests/net/lib/py/__init__.py  |  6 ++
>  tools/testing/selftests/net/lib/py/consts.py  |  9 ++
>  tools/testing/selftests/net/lib/py/ksft.py    | 96 +++++++++++++++++++
>  tools/testing/selftests/net/lib/py/utils.py   | 47 +++++++++
>  tools/testing/selftests/net/lib/py/ynl.py     | 49 ++++++++++
>  7 files changed, 223 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/net/lib/Makefile
>  create mode 100644 tools/testing/selftests/net/lib/py/__init__.py
>  create mode 100644 tools/testing/selftests/net/lib/py/consts.py
>  create mode 100644 tools/testing/selftests/net/lib/py/ksft.py
>  create mode 100644 tools/testing/selftests/net/lib/py/utils.py
>  create mode 100644 tools/testing/selftests/net/lib/py/ynl.py
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index e1504833654d..f533eb7054fe 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -116,6 +116,13 @@ TARGETS += zram
>  TARGETS_HOTPLUG = cpu-hotplug
>  TARGETS_HOTPLUG += memory-hotplug
>  
> +# Networking tests want the net/lib target, include it automatically
> +ifneq ($(filter net,$(TARGETS)),)
> +ifeq ($(filter net/lib,$(TARGETS)),)
> +	INSTALL_DEP_TARGETS := net/lib
> +endif
> +endif
> +
>  # User can optionally provide a TARGETS skiplist.  By default we skip
>  # BPF since it has cutting edge build time dependencies which require
>  # more effort to install.
> @@ -245,7 +252,7 @@ ifdef INSTALL_PATH
>  	install -m 744 run_kselftest.sh $(INSTALL_PATH)/
>  	rm -f $(TEST_LIST)
>  	@ret=1;	\
> -	for TARGET in $(TARGETS); do \
> +	for TARGET in $(TARGETS) $(INSTALL_DEP_TARGETS); do \
>  		BUILD_TARGET=$$BUILD/$$TARGET;	\
>  		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET install \
>  				INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \
> diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile
> new file mode 100644
> index 000000000000..5730682aeffb
> --- /dev/null
> +++ b/tools/testing/selftests/net/lib/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +TEST_FILES := ../../../../../Documentation/netlink/s*

Not sure I understand the motivation for the wildcard. Currently this
matches Documentation/netlink/specs. Do you expect everything that
starts with s to be a testfile?

> +TEST_FILES += ../../../../net/*

Likewise this -- it's just tools/net/ynl? Will everything that's there
be a testfile?

> +TEST_INCLUDES := $(wildcard py/*.py)
> +
> +include ../../lib.mk

Other than that it looks OK.

Reviewed-by: Petr Machata <petrm@nvidia.com>