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

Jakub Kicinski
2024-04-02T17:26:43Z

On Tue, 2 Apr 2024 17:53:41 +0200 Petr Machata wrote:
> > +def ksft_ge(a, b, comment=""):
> > + global KSFT_RESULT
> > + if a < b:
> > + KSFT_RESULT = False
>
> Hmm, instead of this global KSFT_RESULT business, have you considered
> adding and raising an XsftFailEx, like for the other outcomes? We need
> to use KSFT_RESULT-like approach in bash tests, because, well, bash.
>
> Doing it all through exceptions likely requires consistent use of
> context managers for resource clean-up. But if we do, we'll get
> guaranteed cleanups as well. I see that you use __del__ and explicit
> "finally: del cfg" later on, which is exactly the sort of lifetime
> management boilerplate that context managers encapsulate.
>
> This stuff is going to get cut'n'pasted around, and I worry we'll end up
> with a mess of mutable globals and forgotten cleanups if the right
> patterns are not introduced early on.

I wanted to support the semantics which the C kselftest harness has,
by which I mean EXPECT and ASSERT. The helpers don't raise, just record
the failure and keep going. ASSERT semantics are provided by the
exceptions.

I thought it may be easier to follow and write correct code if we raise
ASSERTS explicitly in the test, rather than in the helpers. I mean - if
the programmer has to type in "raise" they are more likely to realize
they need to also add cleanup.

But TBH I'm happy to be persuaded otherwise, I couldn't find a strong
reason to do it one way or the other. I have tried to integrate with
unittest and that wasn't great (I have one huge test I need to port
back now). I don't know if pytest is better, but I decided that we
should probably roll our own. What "our own" exactly is I don't have
strong opinion.