On Wed, Sep 27, 2023 at 10:11:15PM +0200, Thomas Haller wrote: > On Wed, 2023-09-27 at 21:16 +0200, Pablo Neira Ayuso wrote: > > On Wed, Sep 27, 2023 at 07:50:27PM +0200, Thomas Haller wrote: > > > > > > > IMO the netfilter projects should require contributors to provide > > > tests > > > (as sensible). That is, tests that are simply invoked via `make > > > check` > > > and don't require to build special features in the kernel > > > (CONFIG_NFT_OSF). > > > > You mean, some way to exercise userspace code without involving the > > kernel at all. > > Yes, the relevant part is parsing some strings. That should be tested > in isolation. Or just to validate the pf.os file... I understand, that would also require some sort of dump of the parsing, to validate this is correct. I think I understand what you mean by unit test here: You could make a program that imports this .c file, the parse pf.os and dump an output that you could validate in some automated fashion. This osf support from iptables, and tests/py (which was the automated test infrastructure it had) was only added 2012, more than 10 years after iptables was in place. > > > I have patches that would add unit tests to the project (merely as > > > a > > > place where more unit tests could be added). I will add a test > > > there. > > > > We have tests/py/ as unit tests, if that might look similar to what > > you have in mind? Or are you thinking of more tests/shell/ scripts? > > Those only use the public API of libnftables.so. What would be also > useful, is to statically link against the code and have more immediate > access. I see, some internal tests for private API then it is your idea, I am all in for more test coverage. > Also, currently they don't unshare and cannot run rootless. That should > be fixed by extending tests/shell/run-tests.sh script. Well, you > already hack that via `./tests/shell/run-tests.sh ./tests/py/nft- > test.py`, but this should integrate better. Yes, unshare and rootless for tests/py would be good to have if I understood this correctly. > It's waiting on the WIP branch: > https://gitlab.freedesktop.org/thaller/nftables/-/commits/th/no-recursive-make > https://gitlab.freedesktop.org/thaller/nftables/-/blob/545f40babb90584fd188ebe80a1103b93ba49707/tests/unit/test-libnftables-static.c#L177 > > > > > > > But that is based on top of "no recursive make", and I'd like to > > > get > > > that changed first. > > > > I would like to make a release before such change is applied, build > > infrastructure and python support was messy in the previous release. > > Then we look into this, OK? > > Sounds great. Thank you. OK, let's prepare for release then.
