Re: [PATCHv3 net-next 3/3] tools: ynl: add YNL test framework

[Hangbin Liu <liuhangbin@gmail.com>]

On Tue, Nov 11, 2025 at 11:51:38AM +0000, Donald Hunter wrote:
> > diff --git a/tools/net/ynl/tests/Makefile b/tools/net/ynl/tests/Makefile
> > new file mode 100644
> > index 000000000000..4d527f9c3de9
> > --- /dev/null
> > +++ b/tools/net/ynl/tests/Makefile
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Makefile for YNL tests
> > +
> > +TESTS := \
> > +	test_ynl_cli.sh \
> > +	test_ynl_ethtool.sh \
> > +# end of TESTS
> > +
> > +all: $(TESTS)
> > +
> > +run_tests:
> > +	@echo "Running YNL tests..."
> > +	@failed=0; \
> > +	echo "Running test_ynl_cli.sh..."; \
> > +	./test_ynl_cli.sh || failed=$$(($$failed + 1)); \
> > +	echo "Running test_ynl_ethtool.sh..."; \
> > +	./test_ynl_ethtool.sh || failed=$$(($$failed + 1)); \
> 
> This could iterate through $(TESTS) instead of being hard coded.
> 
> > +	if [ $$failed -eq 0 ]; then \
> > +		echo "All tests passed!"; \
> > +	else \
> > +		echo "$$failed test(s) failed!"; \
> 
> AFAICS this will never be reported since the scripts only ever exit 0.
> The message is also a bit misleading since it would be the count of
> scripts that failed, not individual tests.
> 
> It would be great if the scripts exited with the number of test failures
> so the make file could report a total.

Yes, indeed, I will fix it. Thanks!

> > +# Test netdev family operations (dev-get, queue-get)
> > +cli_netdev_ops() {
> > +	local dev_output
> > +	local ifindex
> > +
> > +	ifindex=$(ip netns exec "$testns" cat /sys/class/net/"$NSIM_DEV_NAME"/ifindex 2>/dev/null)
> > +	if [[ -z "$ifindex" ]]; then
> > +		echo "FAIL: YNL CLI netdev operations (failed to get ifindex)"
> 
> This is a bit misleading, there's no ynl command here so I don't think
> it should be a FAIL. Can we just report SKIP when it is an infra issue?

The test reported SKIP for infra issue in setup() function.
Here the netdevsim device has already setup. The ifindex should be there.
Maybe I should use ip cmd to get the ifindex number and skip this checking.

> > +
> > +# Test rt-* family operations (route, addr, link, neigh, rule)
> > +cli_rt_ops() {
> > +	local ifindex
> > +
> > +	if ! $ynl --list-families 2>/dev/null | grep -q "rt-"; then
> > +		echo "SKIP: YNL CLI rt-* operations (no rt-* families available)"
> > +		return
> > +	fi
> > +
> > +	ifindex=$(ip netns exec "$testns" cat /sys/class/net/"$NSIM_DEV_NAME"/ifindex 2>/dev/null)
> > +	if [[ -z "$ifindex" ]]; then
> > +		echo "FAIL: YNL CLI rt-* operations (failed to get ifindex)"
> 
> Also FAIL -> SKIP ?

Same here, the dev has created, I will remove this checking.

Thanks
Hangbin