Re: [PATCH net-next v6 1/3] selftests: net: py: support cmd verifying expected failure

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jakub Kicinski wrote:
> On Thu, 30 Apr 2026 09:28:04 -0400 Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> > 
> > Support negative tests, where cmd raises an exception if the command
> > succeeded.
> > 
> > Existing fail values are
> > 
> > - True:            Pass if returncode == 0, raise Exception otherwise
> > - False:           Pass unconditionally
> > - None:            True iff not terminated explicitly
> > 
> > Introduce a variant of True that inverses the condition:
> > 
> > - 'verify_failed': Pass if returncode != 0, raise Exception otherwise
> > 
> > We cannot reuse False for this, because existing tests rely on current
> > behavior to pass unconditionally.
> > 
> > Only suppress regular test failure. Python subprocess may set a
> > negative return code on process crash or timeout. Those are not
> > anticipated failures.
> 
> The fact that subprocess sets retcode to negative on crash / timeout
> may be worth a comment in the code. I didn't know that.
> 
> > diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> > index 6c44a3d2bbf7..ef31c0ba47fc 100644
> > --- a/tools/testing/selftests/net/lib/py/utils.py
> > +++ b/tools/testing/selftests/net/lib/py/utils.py
> > @@ -111,10 +111,14 @@ class cmd:
> >  
> >          stdout, stderr = self._process_terminate(terminate=terminate,
> >                                                   timeout=timeout)
> > -        if self.proc.returncode != 0 and fail:
> > +
> > +        if (self.proc.returncode != 0 and fail and
> > +            (self.proc.returncode < 0 or fail != 'verify_failed')):
> 
> Personal preference but doesn't feel clean. I would have added 
> a dedicated argument for this. Up to you..

Do you mean adding an expect_fail as below? I have to thread it to cmd and bkg,
but it's not too bad.

@@ -112,13 +120,17 @@ class cmd:
         stdout, stderr = self._process_terminate(terminate=terminate,
                                                  timeout=timeout)

-        if (self.proc.returncode != 0 and fail and
-            (self.proc.returncode < 0 or fail != 'verify_failed')):
+        # Fail on unexpected test failure if fail.
+        # Fail on unexpected test success if expect_fail.
+        # Fail on negative returncode if either:
+        # Set by subprocess on crash or signal, this is never expected failure.
+        if (self.proc.returncode != 0 and fail or
+            (self.proc.returncode < 0 and expect_fail)):
             if len(stderr) > 0 and stderr[-1] == "\n":
                 stderr = stderr[:-1]
             raise CmdExitFailure("Command failed", self)
-        elif self.proc.returncode == 0 and fail == 'verify_failed':
-            raise CmdExitFailure("Command succeeded while should fail", self)
+        elif self.proc.returncode == 0 and expect_fail:
+            raise CmdExitZeroFailure("Command succeeded (expected fail)", self)

 
> >              if len(stderr) > 0 and stderr[-1] == "\n":
> >                  stderr = stderr[:-1]
> >              raise CmdExitFailure("Command failed", self)
> > +        elif self.proc.returncode == 0 and fail == 'verify_failed':
> > +            raise CmdExitFailure("Command succeeded while should fail", self)
> 
> Can we create a new exception type? Just inherit CmdExitFailure 
> but name it more appropriately.
> 
> Regarding the message maybe: "Command succeeded (expected fail)" or
> "Command succeeded (unexpectedly)"