Refactor rebaseline.py into functions ; no behavioral changes.

Refactor rebaseline.py into functions ; no behavioral changes.

R=senorblanco@chromium.org

Committed: https://code.google.com/p/skia/source/detail?r=9318

[epoger, 2013-05-29 16:19:37]

Ready for review at patchset 2.

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py
File tools/rebaseline.py (right):

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py#newcode1
tools/rebaseline.py:1: #!/usr/bin/python
I have confirmed that there are no behavioral changes, as follows:

# Record results before making any changes...
WORKING_DIR=/tmp/rebaseline/before
TRUNK_DIR=$PWD
rm -rf $WORKING_DIR
mkdir -p $WORKING_DIR
pushd $WORKING_DIR
svn co -r 9314 --depth immediates http://skia.googlecode.com/svn/gm-expected .
python $TRUNK_DIR/tools/rebaseline.py verttext 
svn stat | sort >svn-stat
popd
chmod a-w -R /tmp/rebaseline/before

# After making changes to rebaseline.py ...
WORKING_DIR=/tmp/rebaseline/after
TRUNK_DIR=$PWD
rm -rf $WORKING_DIR
mkdir -p $WORKING_DIR
pushd $WORKING_DIR
svn co -r 9314 --depth immediates http://skia.googlecode.com/svn/gm-expected .
python $TRUNK_DIR/tools/rebaseline.py verttext 
svn stat | sort >svn-stat
popd
diff -r --exclude=.* /tmp/rebaseline/before $WORKING_DIR

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py#newcode25
tools/rebaseline.py:25: 'base-shuttle-win7-intel-float':
Using a mapping, instead of a list of tuples, will make it easier to add an
optional parameter selecting a subset of expectations subdirs to rebaseline
(instead of all of them).

[Stephen White, 2013-05-29 16:45:52]

LGTM

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py
File tools/rebaseline.py (right):

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py#newcode1
tools/rebaseline.py:1: #!/usr/bin/python
More my fault than yours, but we should probably add some tests for this before
it gets much bigger. :)

[bsalomon, 2013-05-29 16:58:54]

looks good to Stephen :)

[epoger, 2013-05-29 17:09:35]

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py
File tools/rebaseline.py (right):

https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py#newcode1
tools/rebaseline.py:1: #!/usr/bin/python
On 2013/05/29 16:45:52, Stephen White wrote:
> More my fault than yours, but we should probably add some tests for this
before
> it gets much bigger. :)

Good idea.  What would you think of this approach: adding a --dry_run flag that
would make the tool print out what it would do, without actually doing it. 
(Otherwise, I'm not sure how we could make a test that wouldn't hit SVN)

https://codereview.chromium.org/16160008/diff/8001/tools/rebaseline.py
File tools/rebaseline.py (right):

https://codereview.chromium.org/16160008/diff/8001/tools/rebaseline.py#newcod...
tools/rebaseline.py:104: for expectations_subdir in
sorted(subdir_mapping.keys()):
Patchset 3 iterates over the platforms in deterministic order, which should help
with testing.

[epoger, 2013-05-29 17:09:45]

Committed patchset #3 manually as r9318 (presubmit successful).

[Stephen White, 2013-05-29 17:11:38]

On 2013/05/29 17:09:35, epoger wrote:
> https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py
> File tools/rebaseline.py (right):
> 
>
https://codereview.chromium.org/16160008/diff/2001/tools/rebaseline.py#newcode1
> tools/rebaseline.py:1: #!/usr/bin/python
> On 2013/05/29 16:45:52, Stephen White wrote:
> > More my fault than yours, but we should probably add some tests for this
> before
> > it gets much bigger. :)
> 
> Good idea.  What would you think of this approach: adding a --dry_run flag
that
> would make the tool print out what it would do, without actually doing it.

That sounds good, and generally useful, independent of testing.
 
> (Otherwise, I'm not sure how we could make a test that wouldn't hit SVN)

We'd have to stub/fake/mock it out somehow, I suppose. Might be worth pinging
dpranke@ to see how they handle testing for the Blink rebaseline tools.

> https://codereview.chromium.org/16160008/diff/8001/tools/rebaseline.py
> File tools/rebaseline.py (right):
> 
>
https://codereview.chromium.org/16160008/diff/8001/tools/rebaseline.py#newcod...
> tools/rebaseline.py:104: for expectations_subdir in
> sorted(subdir_mapping.keys()):
> Patchset 3 iterates over the platforms in deterministic order, which should
help
> with testing.