render_pictures unittests: run over a dir with multiple SKPs, like production

render_pictures unittests: run over a dir with multiple SKPs, like production

BUG=skia:1942
Committed: http://code.google.com/p/skia/source/detail?r=13775

[epoger, 2014-03-11 20:28:09]

Ready for review at patchset 1.

I'm working on "make render_pictures read in and compare against expected JSON
results"; this is a step in that direction.

[borenet, 2014-03-12 14:03:50]

LGTM with nit

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
tools/tests/render_pictures_test.py:156:
output_path=os.path.join(self._input_skp_dir, 'red.skp'), red=255)
Nit: shouldn't address a positional parameter by name (not that we're any good
about this elsewhere)

[epoger, 2014-03-12 18:05:30]

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
tools/tests/render_pictures_test.py:156:
output_path=os.path.join(self._input_skp_dir, 'red.skp'), red=255)
On 2014/03/12 14:03:50, borenet wrote:
> Nit: shouldn't address a positional parameter by name (not that we're any good
> about this elsewhere)

Is that in the style guide?  I prefer to *always* specify parameter names, for
safety.

[borenet, 2014-03-12 18:38:52]

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
tools/tests/render_pictures_test.py:156:
output_path=os.path.join(self._input_skp_dir, 'red.skp'), red=255)
On 2014/03/12 18:05:30, epoger wrote:
> On 2014/03/12 14:03:50, borenet wrote:
> > Nit: shouldn't address a positional parameter by name (not that we're any
good
> > about this elsewhere)
> 
> Is that in the style guide?  I prefer to *always* specify parameter names, for
> safety.

I think I share that preference, but I've been told before in code reviews that
it it's wrong.  This is all I can find right now:
http://www.aleksa.org/2013/03/replacing-positional-with-keyword.html

[epoger, 2014-03-12 18:42:01]

On 2014/03/12 18:38:52, borenet wrote:
>
https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
> File tools/tests/render_pictures_test.py (right):
> 
>
https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
> tools/tests/render_pictures_test.py:156:
> output_path=os.path.join(self._input_skp_dir, 'red.skp'), red=255)
> On 2014/03/12 18:05:30, epoger wrote:
> > On 2014/03/12 14:03:50, borenet wrote:
> > > Nit: shouldn't address a positional parameter by name (not that we're any
> good
> > > about this elsewhere)
> > 
> > Is that in the style guide?  I prefer to *always* specify parameter names,
for
> > safety.
> 
> I think I share that preference, but I've been told before in code reviews
that
> it it's wrong.  This is all I can find right now:
> http://www.aleksa.org/2013/03/replacing-positional-with-keyword.html

Is it OK with you if I commit it as-is?  I really prefer the named parameters,
if there is no style guide prohibition...

[borenet, 2014-03-12 18:43:38]

On 2014/03/12 18:42:01, epoger wrote:
> On 2014/03/12 18:38:52, borenet wrote:
> >
>
https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
> > File tools/tests/render_pictures_test.py (right):
> > 
> >
>
https://codereview.chromium.org/194563004/diff/1/tools/tests/render_pictures_...
> > tools/tests/render_pictures_test.py:156:
> > output_path=os.path.join(self._input_skp_dir, 'red.skp'), red=255)
> > On 2014/03/12 18:05:30, epoger wrote:
> > > On 2014/03/12 14:03:50, borenet wrote:
> > > > Nit: shouldn't address a positional parameter by name (not that we're
any
> > good
> > > > about this elsewhere)
> > > 
> > > Is that in the style guide?  I prefer to *always* specify parameter names,
> for
> > > safety.
> > 
> > I think I share that preference, but I've been told before in code reviews
> that
> > it it's wrong.  This is all I can find right now:
> > http://www.aleksa.org/2013/03/replacing-positional-with-keyword.html
> 
> Is it OK with you if I commit it as-is?  I really prefer the named parameters,
> if there is no style guide prohibition...

Yep, since I can't find it explicitly forbidden by the style guide, I'm okay
with it.

[epoger, 2014-03-12 18:44:36]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-03-12 18:44:46]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/epoger@google.com/194563004/1

[commit-bot: I haz the power, 2014-03-12 20:09:59]

Change committed as 13775