[PATCH] D97046: [lit] Generalize `early_tests`

[Julian Lettner via Phabricator via llvm-commits]

yln added a comment.

@lebedev.ri
I don't think we need a RFC for this as long as nothing changes for people who don't use the early_tests or new, generalized ordering feature.  We shouldn't make Dave pay for past debts. (early_tests was introduced 5 years ago!)

@davezarzycki
Thanks for explaining so patiently!

I now understand that there are two separate, but related features: record-and-reorder, and manually declared test phases.  Let me summarize my understanding so you can check it.

**record-and-reorder**:
The sole goal here is to optimize execution time.  lit can do this transparently, so maybe it would even make a good default so everyone can benefit. Making it the default is maybe something that benefit from an RFC so we can learn about people's concerns.
The one detail I want to push back on is that Dave argued that this would not apply to clean/full builds.  Assume we store the order into a file.  With enough motivation there is certainly a way to configure CI bots not to delete this file (or place it somewhere where it wouldn't be deleted).

**manually-declared order**:
Multiple goals: optimize execution time, execute high/low value tests first/last.  Any other reasons to control order?
Although it is unfortunate that we have some overlap with `--incremental` here, I get that it doesn't accomplish all of these goals at the same time.

For me, the question is now whether the additional utility of "manually-declared order" justifies having it or if we should just got with "record-and-reorder".  I think Dave is trying to make the argument that the answer here is "yes".  I can see his arguments and would be happy to sign-off on an implementation, especially if the underlying machinery could be re-used to implement the other.

Design-wise I would prefer a solution that directly states test path prefixes, not "test phases".  I think your sketch already did this, but let me restate:

  // simply a list or dict of `path prefix -> priority`
  config.test_order = [
  'very/slow/test.c',
  'directory/of/slow-tests/',
  '<magic-value-for_all-other-tests>',  // name just for demonstration purposes ;)
  'directory/of/low-value-tests/',
  ]

In lit configs, prepending to this list would mean "execute first", appending would mean "execute last".
And I imagine that "record-and-reorder" could be implemented (in a separate effort) on top of this by expanding `<magic-value-for_all-other-tests>` with the recorded order. (This would mean that the manually declared order is always respected.)

Let me know what you think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97046