netdev - RE: [PATCH RFC net-next v4 2/2] selftests/dpll: add DPLL system integration selftests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CH3PR11MB84143BBDDE886E6479146365E382A@CH3PR11MB8414.namprd11.prod.outlook.com>
Date: Thu, 30 Nov 2023 17:46:37 +0000
From: "Michalik, Michal" <michal.michalik@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>, "Kubalewski,
 Arkadiusz" <arkadiusz.kubalewski@...el.com>, "jonathan.lemon@...il.com"
	<jonathan.lemon@...il.com>, "pabeni@...hat.com" <pabeni@...hat.com>, poros
	<poros@...hat.com>, "Olech, Milena" <milena.olech@...el.com>, mschmidt
	<mschmidt@...hat.com>, "linux-clk@...r.kernel.org"
	<linux-clk@...r.kernel.org>, "bvanassche@....org" <bvanassche@....org>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>
Subject: RE: [PATCH RFC net-next v4 2/2] selftests/dpll: add DPLL system
 integration selftests

 On 29 November 2023 6:40 PM CET, Jakub Kicinski wrote:
> 
> On Thu, 23 Nov 2023 05:52:43 -0500 Michal Michalik wrote:
>> The tests are written in Python3 (3.7+) and pytest testing framework.
>> Framework is basing on the ynl library available in the kernel tree
>> at: tools/net/ynl
> 
> LGTM!
> 
> Somewhat tangential question, a nit, and a comment..
>  
>> The DPLL system integration tests are meant to be part of selftests, so
>> they can be build and run using command:
>>   make -C tools/testing/selftests
>> 
>> Alternatively, they can be run using single command [1]:
>>   make kselftest
>> 
>> If we want to run only DPLL tests, we should set the TARGETS variable:
>>   make -C tools/testing/selftests TARGETS=drivers/net/netdevsim/dpll
>> 
>> They can also be run standalone using starter script:
>>   ./run_dpll_tests.sh
>> 
>> There is a possibliy to set optional PYTEST_PARAMS environment variable
>> to set the pytest options, like tests filtering ("-k <filter>") or
>> verbose output ("-v").
>> 
>> [1] https://www.kernel.org/doc/html/v5.0/dev-tools/kselftest.html
> 
> nit: s/v5.0/v6.6/ ? Or /v5.0/latest/

Ohh - yeah, definitely will change that. Thanks!

> 
> Did you try to run it in vmtest or virtme-ng?
> https://www.youtube.com/watch?v=NT-325hgXjY
> https://lpc.events/event/17/contributions/1506/attachments/1143/2441/virtme-ng.pdf
> 
> I'm thinking of using those for continuous testing, curious all 
> the Python setup works okay with them.

Very interesting idea, I didn't try to use those - will get familiar with that and
see if I can make any improvements to go with vmtest/virtme-ng before I will send
out the RFC v5.

> 
>> +@...est.fixture(scope="class", params=((0,), (1, 0), (0, 1)))
> 
> We have both uses of pytest and unittest in the kernel:
> 
> $ git grep --files-with-matches '^import .*unittest'
> scripts/rust_is_available_test.py
> tools/crypto/ccp/test_dbc.py
> tools/perf/pmu-events/metric_test.py
> tools/testing/kunit/kunit_tool_test.py
> tools/testing/selftests/bpf/test_bpftool.py
> tools/testing/selftests/tpm2/tpm2.py
> tools/testing/selftests/tpm2/tpm2_tests.py
> 
> $ git grep --files-with-matches '^import .*pytest'
> scripts/kconfig/tests/conftest.py
> tools/testing/selftests/drivers/sdsi/sdsi.sh
> tools/testing/selftests/drivers/sdsi/sdsi_test.py
> tools/testing/selftests/hid/tests/base.py
> tools/testing/selftests/hid/tests/conftest.py
> tools/testing/selftests/hid/tests/test_gamepad.py
> tools/testing/selftests/hid/tests/test_mouse.py
> tools/testing/selftests/hid/tests/test_multitouch.py
> tools/testing/selftests/hid/tests/test_sony.py
> tools/testing/selftests/hid/tests/test_tablet.py
> tools/testing/selftests/hid/tests/test_usb_crash.py
> tools/testing/selftests/hid/tests/test_wacom_generic.py
> 
> unittest seems a bit more popular but pytest does seem like
> a better fit indeed.

Yeah, even official Python documentation points to pytest as a good alternative
with lighter syntax comparing to their built-in library in "see also" section:
https://docs.python.org/3/library/unittest.html

> 
> Did you see what the sdsi test does? It seems to assume everything 
> is installed locally, without the venv. I wonder if that may be simpler
> to get going with vmtest?

To be honest I did not see that. I agree that this is a simpler solution, but I am
not sure if that is not "too simple". What I mean, I'm not sure who wrote the sdsi
tests, but maybe they were not aware about the Python best practices? Python used
to be my first language, and I would vote for using the venvs if you asked me.
I understand that it haven't been done before, but we are here to try to improve
the things, yes? Of course if you outvote me, I won't act as Tadeusz Rejtan in
Matejko's painting "The Fall of Poland" and just remove the virtual environments. :)

Thanks,
M^2