One test runner to rule them all.

One test runner to rule them all.

This CL takes the three test runners (corpus, javascript, pixel) and combines
the code into a single test_runner file. Each of the individual runners still
exists and calls the test runner with their data directory.

With this change, the pixel and javascript test will now run in parallel if
multiple processors are available.

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/a1215ba51a235fb7abcb995f0e768ea0176d9275

[dsinclair, 2015-10-29 01:54:09]

dsinclair@chromium.org changed reviewers:
+ thestig@chromium.org, tsepez@chromium.org

[dsinclair, 2015-10-29 01:54:10]

PTAL.

Good idea or bad idea?

Everything seems to be working locally, but need to run some more comprehensive
tests when I'm back on my Linux box as there are a lot of suppressed tests on
Mac.

[dsinclair, 2015-10-29 13:46:29]

On 2015/10/29 at 01:54:10, dsinclair wrote:
> PTAL.
> 
> Good idea or bad idea?
> 
> Everything seems to be working locally, but need to run some more
comprehensive tests when I'm back on my Linux box as there are a lot of
suppressed tests on Mac.


Hm, turns out there is something wrong with diffing and parallel execution.
Please hold off reviewing for now.

[dsinclair, 2015-10-29 14:00:57]

On 2015/10/29 at 13:46:29, dsinclair wrote:
> On 2015/10/29 at 01:54:10, dsinclair wrote:
> > PTAL.
> > 
> > Good idea or bad idea?
> > 
> > Everything seems to be working locally, but need to run some more
comprehensive tests when I'm back on my Linux box as there are a lot of
suppressed tests on Mac.
> 
> 
> Hm, turns out there is something wrong with diffing and parallel execution.
Please hold off reviewing for now.


Ok, fixed. PTAL.

[Tom Sepez, 2015-10-29 16:35:43]

I think this is good, but get Lei to sign off as well.  I'm suprised we didn't
get an "A+" diff against one of the old files - would make verification easier.

[dsinclair, 2015-10-29 16:40:40]

On 2015/10/29 at 16:35:43, tsepez wrote:
> I think this is good, but get Lei to sign off as well.  I'm suprised we didn't
get an "A+" diff against one of the old files - would make verification easier.

It's because while pixel and javascript were similar, corpus was vastly
different. This is the amalgamation of how all three of them worked so it ends
up looking like none of them, heh.

[Lei Zhang, 2015-10-29 17:35:01]

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
testing/tools/test_runner.py:29: """Wrapper function to call GenerateAndTest()
and redirect output to stdout."""
80 chars / column, ditto below

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
testing/tools/test_runner.py:51: def GenerateAndTest(self, input_filename,
source_dir, redirect_output=False):
Should this be split into Generate(), TestPixel() and TestText() ?

[dsinclair, 2015-10-29 17:36:46]

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
testing/tools/test_runner.py:51: def GenerateAndTest(self, input_filename,
source_dir, redirect_output=False):
On 2015/10/29 at 17:35:01, Lei Zhang wrote:
> Should this be split into Generate(), TestPixel() and TestText() ?

If we want good code and not just me randomly slapping things together....yes.

Let me see what I can do and I'll ping when I'm done (probably early next week).

[Lei Zhang, 2015-10-29 17:48:34]

On 2015/10/29 17:36:46, dsinclair wrote:
> Let me see what I can do and I'll ping when I'm done (probably early next
week).

No rush.

[dsinclair, 2015-11-02 15:22:52]

PTAL.

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
testing/tools/test_runner.py:29: """Wrapper function to call GenerateAndTest()
and redirect output to stdout."""
On 2015/10/29 17:35:01, Lei Zhang wrote:
> 80 chars / column, ditto below

Done.

https://codereview.chromium.org/1430623006/diff/20001/testing/tools/test_runn...
testing/tools/test_runner.py:51: def GenerateAndTest(self, input_filename,
source_dir, redirect_output=False):
On 2015/10/29 17:35:01, Lei Zhang wrote:
> Should this be split into Generate(), TestPixel() and TestText() ?

Done.

[Lei Zhang, 2015-11-03 03:11:21]

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:62: 
no need for indentation since the above code is related to the code below.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:70: pdf_path, redirect_output)
Indent with "source_dir" from the line above? Ditto below.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:95: redirect_output):
funny indentation?

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:106: [sys.executable, self.fixup_path,
'--output-dir=' + self.working_dir,
May be easier to read here if we build a cmd variable, and just call
RunCommand(cmd, redirect_output), because the command is a bit long. The cases
below are ok since they still fit on one line.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:117: redirect_output)
4 spaces too many

[dsinclair, 2015-11-03 14:29:13]

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:62: 
On 2015/11/03 03:11:21, Lei Zhang wrote:
> no need for indentation since the above code is related to the code below.

Which indentation, the for is at the same level as actual_images? Or do you mean
input_filename? What is the indent level for wrapping long lines?

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:70: pdf_path, redirect_output)
On 2015/11/03 03:11:21, Lei Zhang wrote:
> Indent with "source_dir" from the line above? Ditto below.

Done.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:95: redirect_output):
On 2015/11/03 03:11:21, Lei Zhang wrote:
> funny indentation?

That about sums up python, heh.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:106: [sys.executable, self.fixup_path,
'--output-dir=' + self.working_dir,
On 2015/11/03 03:11:21, Lei Zhang wrote:
> May be easier to read here if we build a cmd variable, and just call
> RunCommand(cmd, redirect_output), because the command is a bit long. The cases
> below are ok since they still fit on one line.

Done.

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:117: redirect_output)
On 2015/11/03 03:11:21, Lei Zhang wrote:
> 4 spaces too many

Done.

[Lei Zhang, 2015-11-03 14:41:03]

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:62: 
On 2015/11/03 14:29:12, dsinclair wrote:
> On 2015/11/03 03:11:21, Lei Zhang wrote:
> > no need for indentation since the above code is related to the code below.
> 
> Which indentation, the for is at the same level as actual_images? Or do you
mean
> input_filename? What is the indent level for wrapping long lines?

Sorry, I meant no need for blank line.

[Lei Zhang, 2015-11-03 14:43:10]

lgtm

[dsinclair, 2015-11-03 15:09:25]

Committed patchset #5 (id:80001) manually as
a1215ba51a235fb7abcb995f0e768ea0176d9275 (presubmit successful).

[dsinclair, 2015-11-03 15:10:21]

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
File testing/tools/test_runner.py (right):

https://codereview.chromium.org/1430623006/diff/40001/testing/tools/test_runn...
testing/tools/test_runner.py:62: 
On 2015/11/03 14:41:03, Lei Zhang wrote:
> On 2015/11/03 14:29:12, dsinclair wrote:
> > On 2015/11/03 03:11:21, Lei Zhang wrote:
> > > no need for indentation since the above code is related to the code below.
> > 
> > Which indentation, the for is at the same level as actual_images? Or do you
> mean
> > input_filename? What is the indent level for wrapping long lines?
> 
> Sorry, I meant no need for blank line.

Done.