[llvm] [lit] Fix some issues from --per-test-coverage (PR #65242)

Joel E. Denny via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 11 08:02:05 PDT 2023


jdenny-ornl wrote:

> I noticed that the python formatting check failed so I looked into that, the problem is the file `llvm/utils/lit/tests/per-test-coverage.py`

Thanks for pointing that out.  I noticed it too but made the bad assumption that something was wrong with the pre-commit check itself, and I ran out of time to investigate.  This is the diagnostic I saw after a traceback:

```
black.parsing.InvalidInput: Cannot parse: 12:13: t-cfg.py ({{[^)]*}})
```

I didn't see `llvm/utils/lit/tests/per-test-coverage.py` mentioned anywhere.  Did I miss it?

In contrast, when I run black on the command line on my local system, I see:

```
$ black llvm/utils/lit/tests/per-test-coverage.py 
error: cannot format llvm/utils/lit/tests/per-test-coverage.py: Cannot parse: 12:13: t-cfg.py ({{[^)]*}})

Oh no! 💥 💔 💥
1 file failed to reformat.
```

Is there any way to get the pre-commit check to mention the name of the problematic file?

(By the way, I have noticed that many python files were formatted with >80 columns, probably the black default of 88.  That doesn't fit in the 80-column editor/terminal windows I use for most of LLVM.  Is there any good reason not to follow [the LLVM coding standard for source code width](https://llvm.org/docs/CodingStandards.html#source-code-width) in python files?  I see no exception there for python, or did I misunderstand something?  Black just needs a `-l80` command-line option.)

> 
> this file is not valid python, neither `black` or `python` can parse it. Since that's the case, can we change the extension on it so that we don't have to create a lot of exceptions in our CI and pre-commit hook?

Lit's tests use the `.py` suffix.  Most just contain comments that contain lit/FileCheck directives.  Some contain actual python code called by RUN lines.  The above test contains garbage outside of a comment, so the test works fine, but it's invalid as python.  I'll push a commit to this PR to fix it.  Thanks again for pointing me in the right direction.

https://github.com/llvm/llvm-project/pull/65242


More information about the llvm-commits mailing list