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

[Michał Górny via Phabricator via llvm-commits]

mgorny added inline comments.


================
Comment at: llvm/utils/lit/tests/reorder.py:4
 # RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
-# RUN: %{lit} -j1 %{inputs}/reorder > %t.out
+# RUN: %{lit} --default-order -j1 %{inputs}/reorder > %t.out
 # RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.new
----------------
jdenny wrote:
> jdenny wrote:
> > As far as I can tell, we now have no test that `--default-order` is actually the default.  Sorry, I didn't think of that when I made that suggestion.  If we ever broke that behavior (for example, someone could reorganize the arg specification in a subtly broken way), I wonder how long it would be before anyone realized that test suites started taking much longer than necessary.
> > 
> > What if we have something like a `%{lit-no-order-opt}` that doesn't specify any order options?  `reorder.py` could then run lit twice, once to ensure `--default-order` is indeed the default, and once to ensure it overrides `--lexical-order` and `--shuffle` when specified afterward.
> > 
> > I guess that means `--default-order` isn't so useful in lit's own test suite.  It's just another feature to test.  However, as I mentioned in D107427, I'd like to use it in `clang/test/utils` too for testing test-suite-generation scripts.
> > I guess that means --default-order isn't so useful in lit's own test suite. It's just another feature to test. However, as I mentioned in D107427, I'd like to use it in clang/test/utils too for testing test-suite-generation scripts.
> 
> Well, that actually makes no sense.  I want `--lexical-order` there not `--default-order`.
> 
> Hmm.  I'm not opposed to keeping something like `--default-order` for completeness, but I'm now having trouble seeing a real use case.  What do others think?
To be honest, I don't really see a use case for testing which order is the default. After all, setting the default order is a matter of changing the parameter to `default=`… A test for this sounds like asking people to change the default in two places.


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

https://reviews.llvm.org/D107695