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

Petr Machata
2024-04-04T10:39:38Z

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>