[PATCH] D83924: [mlir] Add update_mlir_test_checks.py

[River Riddle via Phabricator via llvm-commits]

rriddle added a comment.

There seems to be a lot of hardcoding in here related to `func`. Given that `func` is not even the only function in upstream MLIR, and that for several dialects(existing and upcoming) `func` isn't even used at all, what is the story for those users?



================
Comment at: llvm/utils/update_mlir_test_checks.py:29
+The script is designed to make adding checks to a test case fast, it is *not*
+designed to be authoratitive about what constitutes a good test!
+"""
----------------
stephenneuendorffer wrote:
> mehdi_amini wrote:
> > stephenneuendorffer wrote:
> > > mehdi_amini wrote:
> > > > Something that I believe is important is to not just have blindly generated CHECKs but have a human editing the tests (documenting, minimizing the number of checks, etc.). 
> > > > 
> > > > Can we have the script adding a paragraph mentioning in the actual generated test? (with a link to the testing guide, etc.)
> > > Conceivably...but I'm guessing that we'd end up with the same paragraph in lots of files.  I tend to use this to auto-update tests in a mechanical way which is easy to verify, but hard to do by hand.  I'm specifically trying to avoid opening every file when updating lots of tests, so I think adding a bunch of text in the file that is expected to be removed would be a bad idea.  What if we just added a boilerplate message about usage and norms that was generated when the script is run?
> > > Conceivably...but I'm guessing that we'd end up with the same paragraph in lots of files.
> > 
> > I mean: since the user is supposed to manually edit significantly the output of the tool, removing the paragraph doesn't seem much.
> > 
> > The challenge is not get the output of the script just good enough to be checked in as-is but leading to actual bad tests
> Personally, I think we'll get better tests if we get people to focus on making good tests, rather than tedious manual editing.  I think that this goes more into the input of tests, than the format of the check lines.  I think forcing people to 'manually edit significantly' should definitely not be the goal.
Forcing lots of manual edits is not the goal, but a more important goal is not to have "diff" tests. IMO having a large amount of users manually edit is much better than having one person have to deal with the fallout of having to update hundreds of tests that broke because they were testing something unnecessary. It spreads out the maintenance efforts and makes evolution easier. (Speaking from having had to deal with this a lot in practice.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83924/new/

https://reviews.llvm.org/D83924