Re: [PATCH 2/6] test: Add linting of Python test scripts

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5250 bytes --]

On Wed, Oct 01, 2025 at 12:23:51PM +0200, Stefano Brivio wrote:
> On Wed,  1 Oct 2025 19:51:58 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > We currently have one test moved to the new exeter based framwork written
> > in Python.  We plan to add many more, so add linting (flake8) and type
> > checking (mypy) of those scripts.  This can be invoked manually with
> > "make flake8" or "make mypy" in test/, and is also added to the static
> > checkers test set.
> 
> I never used a Python linter, so I'm not sure if it's as bad as Go or
> Rust linters taking the whole poetry away, as it happened for instance
> in my most recent experience with 'cargo fmt':
> 
>   https://github.com/AsahiLinux/muvm/compare/68094c02c19b6f5d5e3def6d29379c1244c9a5e4..9af11c334a1ce37f533c056d982f8608c8d80d27#diff-e1a95ce380b9a8a317f97cccce1cbfd3dccd343dc62169ed1340208ab304fab9L106
> 
>   https://github.com/AsahiLinux/muvm/pull/111#discussion_r1863551727
>   (you need to click around before you get to it, no idea how to share
>   a proper link that opens that comment right away)
> 
> Maybe with Python it's not as annoying? I don't have a strong
> preference against this, I just wanted to raise a possible downside.

I've been using flake8 for a while now.  It _is_ pedantic about
formatting, but so far at least, I haven't felt like it conflicted
with anything I wanted to express.  Finding missing or redundant
imports and the like is certainly useful, since there's no compiler to
do so.  There is certainly some overlap with what mypy does.

> In general, my thought is that coding style serves a purpose, and it's
> used by humans for humans, so it's flexible, with exceptions, and
> reasons behind it and behind those exceptions.
> 
> If we just pass everything through a linter, well, except for the
> purposes of revision control, we don't really need a coding style?

flake8 is partly about code formatting, but not entirely.  Think of it
as sitting somewhere in between indent and cppcheck, maybe?

> 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  test/Makefile                 | 14 +++++++++++++-
> >  test/build/build.py           |  5 +++--
> >  test/build/static_checkers.sh |  6 +++++-
> >  3 files changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/test/Makefile b/test/Makefile
> > index 69987d0b..2637e0ed 100644
> > --- a/test/Makefile
> > +++ b/test/Makefile
> > @@ -8,6 +8,8 @@
> >  BATS = bats -j $(shell nproc)
> >  EXETOOL = exeter/exetool/exetool
> >  WGET = wget -c
> > +FLAKE8 = flake8
> > +MYPY = mypy --strict
> >  
> >  DEBIAN_IMGS = debian-8.11.0-openstack-amd64.qcow2 \
> >  	debian-10-nocloud-amd64.qcow2 \
> > @@ -65,11 +67,15 @@ LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \
> >  ASSETS = $(DOWNLOAD_ASSETS) $(LOCAL_ASSETS)
> >  
> >  EXETER_PYPATH = exeter/py3
> > +EXETER_PYTHON = build/build.py
> >  EXETER_BATS = smoke/smoke.sh.bats \
> > -	build/build.py.bats build/static_checkers.sh.bats
> > +	$(EXETER_PYTHON:%=%.bats) build/static_checkers.sh.bats
> >  BATS_FILES = $(EXETER_BATS) \
> >  	podman/test/system/505-networking-pasta.bats
> >  
> > +# Python test code (for linters)
> > +PYPKGS = $(EXETER_PYTHON)
> > +
> >  CFLAGS = -Wall -Werror -Wextra -pedantic -std=c99
> >  
> >  assets: $(ASSETS)
> > @@ -130,6 +136,12 @@ medium.bin:
> >  big.bin:
> >  	dd if=/dev/urandom bs=1M count=10 of=$@
> >  
> > +flake8: pull-exeter
> > +	PYTHONPATH=$(EXETER_PYPATH) $(FLAKE8) $(PYPKGS)
> > +
> > +mypy: pull-exeter
> > +	PYTHONPATH=$(EXETER_PYPATH) $(MYPY) $(PYPKGS)
> > +
> >  $(EXETER_BATS): %.bats: % $(EXETOOL)
> >  	PYTHONPATH=$(EXETER_PYPATH) $(EXETOOL) bats -- $< > $@
> >  
> > diff --git a/test/build/build.py b/test/build/build.py
> > index e49287c9..e3de8305 100755
> > --- a/test/build/build.py
> > +++ b/test/build/build.py
> > @@ -18,11 +18,12 @@ import os
> >  from pathlib import Path
> >  import subprocess
> >  import tempfile
> > -from typing import Iterable, Iterator
> > +from typing import Iterator
> >  
> >  import exeter
> >  
> > -def sh(cmd):
> > +
> > +def sh(cmd: str) -> None:
> >      """Run given command in a shell"""
> >      subprocess.run(cmd, shell=True)
> >  
> > diff --git a/test/build/static_checkers.sh b/test/build/static_checkers.sh
> > index 42806e79..228b99ae 100755
> > --- a/test/build/static_checkers.sh
> > +++ b/test/build/static_checkers.sh
> > @@ -21,6 +21,10 @@ exeter_set_description cppcheck "passt sources pass cppcheck"
> >  exeter_register clang_tidy make -C .. clang-tidy
> >  exeter_set_description clang_tidy "passt sources pass clang-tidy"
> >  
> > -exeter_main "$@"
> > +exeter_register flake8 make flake8
> > +exeter_set_description flake8 "passt tests in Python pass flake8"
> >  
> > +exeter_register mypy make mypy
> > +exeter_set_description mypy "passt tests in Python pass mypy --strict"
> >  
> > +exeter_main "$@"
> 
> -- 
> Stefano
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]