Add flag to test harness to stop sorting test cases.

Add flag to test harness to stop sorting test cases.

R=jkummerow@chromium.org, machenbach@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21561

[Michael Starzinger, 2014-05-27 19:47:52]

Jakob: PTAL.
Michael: FYI.

[Jakob Kummerow, 2014-05-28 09:42:40]

LGTM with comment.

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
tools/testrunner/local/execution.py:68: if not context.no_sorting:
As discussed, a better implementation might be to leave this line untouched...

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
tools/testrunner/local/execution.py:98: for test in sorted(self.tests,
key=lambda t: t.duration, reverse=True):
...and instead do here:

if not self.context.no_sorting:
  self.tests = sorted(self.tests, key=lambda t: t.duration, reverse=True)
for test in self.tests:
  ...

[Michael Starzinger, 2014-05-28 10:47:53]

Addressed comments. Landing.

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
tools/testrunner/local/execution.py:68: if not context.no_sorting:
On 2014/05/28 09:42:41, Jakob wrote:
> As discussed, a better implementation might be to leave this line untouched...

Done.

https://codereview.chromium.org/307553003/diff/1/tools/testrunner/local/execu...
tools/testrunner/local/execution.py:98: for test in sorted(self.tests,
key=lambda t: t.duration, reverse=True):
On 2014/05/28 09:42:41, Jakob wrote:
> ...and instead do here:
> 
> if not self.context.no_sorting:
>   self.tests = sorted(self.tests, key=lambda t: t.duration, reverse=True)
> for test in self.tests:
>   ...

Done.

[Michael Starzinger, 2014-05-28 10:49:19]

Committed patchset #2 manually as r21561 (presubmit successful).

[Michael Achenbach, 2014-05-28 11:08:32]

LGTM with comments:

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:68: t.duration =
self.perfdata.FetchPerfData(t) or 1.0
If perf data is conditionally not used, maybe it should also not be harvested?

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:98: self.tests = sorted(self.tests,
key=lambda t: t.duration, reverse=True)
Not totally happy with replacing generators with copied lists. We might loose a
few micro seconds :P

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:136: self.perfdata.UpdatePerfData(test)
Also here...

[Jakob Kummerow, 2014-05-28 11:12:16]

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:98: self.tests = sorted(self.tests,
key=lambda t: t.duration, reverse=True)
On 2014/05/28 11:08:32, Michael Achenbach wrote:
> Not totally happy with replacing generators with copied lists. We might loose
a
> few micro seconds :P

Right, should probably use self.tests.sort(key=...) instead. My bad.

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:136: self.perfdata.UpdatePerfData(test)
On 2014/05/28 11:08:32, Michael Achenbach wrote:
> Also here...

I disagree. Updating the perfstore should be fast enough that we can do it
anyways. More data points = better data :-)

[Michael Starzinger, 2014-05-28 11:24:56]

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:68: t.duration =
self.perfdata.FetchPerfData(t) or 1.0
On 2014/05/28 11:08:32, Michael Achenbach wrote:
> If perf data is conditionally not used, maybe it should also not be harvested?

IMHO we should still harvest perf-data (assuming by harvest you mean "measure
and store it into the DB") because of the following reasons: (1) When the
test-harness is used without the flag from time to time, then the data will
already by populated from previous runs and (2) should we ever decide to use the
perf-data for something else besides sorting, then the pure data won't hinge on
the sorting flag.

https://codereview.chromium.org/307553003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:98: self.tests = sorted(self.tests,
key=lambda t: t.duration, reverse=True)
On 2014/05/28 11:12:16, Jakob wrote:
> On 2014/05/28 11:08:32, Michael Achenbach wrote:
> > Not totally happy with replacing generators with copied lists. We might
loose
> a
> > few micro seconds :P
> 
> Right, should probably use self.tests.sort(key=...) instead. My bad.

Will upload a follow-up CL to address this.