[PATCH] D99073: [lit] Reliable progress indicator and ETA

[Julian Lettner via Phabricator via llvm-commits]

yln accepted this revision.
yln added a comment.
This revision is now accepted and ready to land.

Very cool, thanks! :)

LGTM, with nits.



================
Comment at: llvm/utils/lit/lit/display.py:4
 
 def create_display(opts, tests, total_tests, workers):
     if opts.quiet:
----------------
`tests` is now the list of test objects, not just a count, right?


================
Comment at: llvm/utils/lit/lit/display.py:9-10
+    num_tests = len(tests)
+    of_total = (' of %d' % total_tests) if (num_tests != total_tests) else ''
+    header = '-- Testing: %d%s tests, %d workers --' % (num_tests, of_total, workers)
 
----------------
Could now use string interpolation (because we require Python 3.6).


================
Comment at: llvm/utils/lit/lit/display.py:34
+        for test in tests:
+          if test.previous_elapsed == 0.0:
+              self.unpredictable_tests_remaining += 1
----------------
The test here should be `if test.previous_elapsed:` (and flip the if-else) to make sure `None` is counted as "zero" (in case that could ever happen) and avoid floating point comparison with `0.0`.  Same below in `update()`.


================
Comment at: llvm/utils/lit/lit/display.py:70-76
+        self.progress_predictor = None
         self.progress_bar = progress_bar
         self.completed = 0
 
+        if self.progress_bar:
+            self.progress_predictor = ProgressPredictor(tests)
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99073