On Thu, Mar 16, 2023 at 10:43 AM Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2023-03-16 at 10:35 -0700, Daniel Latypov wrote:
> > On Thu, Mar 16, 2023 at 12:42 AM Johannes Berg
> > <johannes@xxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Wed, 2023-03-15 at 23:02 -0700, Daniel Latypov wrote:
> > > > This is a good catch, thanks.
> > > > But we also have a few more bare generic types that warrant attention.
> > >
> > > Oh, that might well be true. I was using kunit_parser in a script, and
> > > that imports kunit_printer, and then tried to check *my* script for type
> > > annotations with mypy. This led it to go check through the dependencies
> > > too, and since it was just one small thing there I decided to just fix
> > > it rather than figure out how to tell mypy that I don't care about those
> > > dependencies :-)
> >
> > There's --exclude='<regex>', if you ever end up needing to ignore other files.
> > But yeah, we should try and make sure that mpyy is happy w/ kunit.py code.
>
> Yes, but that has other complexities I think? Anyway, I didn't even
> really read much into it since it was so easy.
>
> > > Now for everything else? I didn't even look.
> >
> > Oh, does mypy complain about this now?
> > That'd be nice.
> >
> > Hmm, I don't see it even after upgrading my local version.
> > $ pip install --upgrade mypy pytype
> > $ ../tools/testing/kunit/run_checks.py
> > <no errors>
> > # Checking if it doesn't report error but logs a warning:
> > $ mypy ./tools/testing/kunit/*.py
> > Success: no issues found in 9 source files
>
> Oh, I use --strict, and
>
> $ mypy --version
> mypy 0.941
>
> right now. Probably old.
>
> And
>
> $ mypy --strict tools/testing/kunit/*.py
>
> has a _lot_ of complaints.
Yikes.
I either forgot or never knew about --strict, thanks for pointing it out.
Looking into it, I don't think it's worth using for kunit.py, details below.
With mypy 1.1.1, I get
Found 172 errors in 5 files (checked 9 source files)
Counting the different types
... | grep -Po '\[[a-z-]+\]$' | sort | uniq -c | sort -nr
108 [no-untyped-def]
61 [no-untyped-call]
1 [attr-defined]
1 [assignment]
Hmm, the untyped stuff is not too important since we also use pytype,
which is a lot smarter in that it infers types where not specified.
The other two though warrant some attention
* attr-defined: we don't care about this. We have a directive to
squash this "error" for pytype. Ugh, do we need another one for
pytype?
* assignment: not actually a real error, it points to our type
annotations being a bit sloppy
tools/testing/kunit/kunit.py:459: error: Incompatible types in
assignment (expression has type "List[str]", variable has type
"TextIO") [assignment]
So it doesn't like that we assign a list[str] or a IO[str] to `kunit_output`.
But we're passing it in as Iterable[str], which both work as.
Even explicitly annotating it as an Iterable[str] doesn't make it happy [1].
I'll ignore this, then.
Daniel
[1]
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 52853634ba23..a9f1146bcdce 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -453,10 +453,10 @@ def exec_handler(cli_args):
def parse_handler(cli_args):
if cli_args.file is None:
sys.stdin.reconfigure(errors='backslashreplace') #
pytype: disable=attribute-error
- kunit_output = sys.stdin
+ kunit_output = sys.stdin # type[Iterable[str]]
else:
with open(cli_args.file, 'r', errors='backslashreplace') as f:
- kunit_output = f.read().splitlines()
+ kunit_output = f.read().splitlines() #
type[Iterable[str]]
# We know nothing about how the result was created!
metadata = kunit_json.Metadata()
request = KunitParseRequest(raw_output=cli_args.raw_output,
