[PATCH] D107695: [llvm] [lit] Support forcing lexical test order

[Julian Lettner via Phabricator via llvm-commits]

yln added a comment.

Thanks for working on this! :)



================
Comment at: llvm/utils/lit/lit/cl_arguments.py:14
     RANDOM = enum.auto()
+    LEXICAL = enum.auto()
 
----------------
While we are at it, can we also give `DEFAULT` a more descriptive name here?

Order predicate: `(not t.previous_failure, -t.previous_elapsed, t.getFullName())`
  # Failures first
  # Longest tests first
  # Then by name

Since putting all of the above in an enum name is quite a mouthful, I am advocating for `smart` as suggested by Joel.

Together with my other comment (see below) the help text could then be:
`--order=(smart|aaa|bbb|...) [default: smart]`

This would align the names used for options and names in the code.


================
Comment at: llvm/utils/lit/lit/cl_arguments.py:154
             type=_positive_int)
+    selection_group.add_argument("--default-order",
+            dest="order",
----------------
jdenny wrote:
> Instead of `--default-order`, I'd prefer something that describes the order.  I'm not sure of the best name, especially given that it's always possible we'll think of other tweaks to the default order in the future.  Perhaps something like `--auto-order` or `--smart-order` to at least suggest it's something clever.  What do you think?
I would also prefer descriptive names instead of "default".

If we already put the effort i to improve ordering, how about: `--order=(aaa|bbb|...)` and make the old ones aliases for the new ones: `--shuffle` --> `--order=random`.

I think argparse spells out all possible options and even shows which one is the default when printing the help: `--order=(smart|aaa|bbb|...) [default: smart]`

So it shows the user the available options, which option is the default, and the explanation of these options is right next to it.
Having all options documented right next to each other also improves discoverability and makes contrasting the differences between options easier.


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

https://reviews.llvm.org/D107695