| Message ID | 20240502025325.1924923-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e1bb5e65de8355ee76f51c6bfee2328ac5b2be15 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] selftests: net: py: check process exit code in bkg() and background cmd() | expand |
Jakub Kicinski wrote: > We're a bit too loose with error checking for background > processes. cmd() completely ignores the fail argument > passed to the constructor if background is True. > Default to checking for errors if process is not terminated > explicitly. Caller can override with True / False. > > For bkg() the processing step is called magically by __exit__ > so record the value passed in the constructor. > > Reported-by: Willem de Bruijn <willemb@google.com> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Tested-by: Willem de Bruijn <willemb@google.com> Thanks!
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 1 May 2024 19:53:25 -0700 you wrote: > We're a bit too loose with error checking for background > processes. cmd() completely ignores the fail argument > passed to the constructor if background is True. > Default to checking for errors if process is not terminated > explicitly. Caller can override with True / False. > > For bkg() the processing step is called magically by __exit__ > so record the value passed in the constructor. > > [...] Here is the summary with links: - [net-next] selftests: net: py: check process exit code in bkg() and background cmd() https://git.kernel.org/netdev/net-next/c/e1bb5e65de83 You are awesome, thank you!
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index b57d467afd0f..ec8b086b4fcb 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -26,6 +26,9 @@ import time self.process(terminate=False, fail=fail) def process(self, terminate=True, fail=None): + if fail is None: + fail = not terminate + if terminate: self.proc.terminate() stdout, stderr = self.proc.communicate(timeout=5) @@ -43,17 +46,18 @@ import time class bkg(cmd): - def __init__(self, comm, shell=True, fail=True, ns=None, host=None, + def __init__(self, comm, shell=True, fail=None, ns=None, host=None, exit_wait=False): super().__init__(comm, background=True, shell=shell, fail=fail, ns=ns, host=host) self.terminate = not exit_wait + self.check_fail = fail def __enter__(self): return self def __exit__(self, ex_type, ex_value, ex_tb): - return self.process(terminate=self.terminate) + return self.process(terminate=self.terminate, fail=self.check_fail) def tool(name, args, json=None, ns=None, host=None):
We're a bit too loose with error checking for background processes. cmd() completely ignores the fail argument passed to the constructor if background is True. Default to checking for errors if process is not terminated explicitly. Caller can override with True / False. For bkg() the processing step is called magically by __exit__ so record the value passed in the constructor. Reported-by: Willem de Bruijn <willemb@google.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- tools/testing/selftests/net/lib/py/utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)