Refactoring and minor corrections:

Refactoring and other minor corrections:

 - Corrected handling of the BISECT_MODE_RETURN_CODE when using BisectOptions.FromDict.
 - Refactored code to avoid parsing metrics when bisect is running in BISECT_MODE_RETURN_CODE.
 - Other minor corrections.

R=qyearsley@chromium.org

Committed: https://crrev.com/f1ae325a5db81e728b4e3075546cfdd1f24bc304
Cr-Commit-Position: refs/heads/master@{#295436}

[qyearsley, 2014-09-15 20:30:40]

Thanks, these changes appear to be good.

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
File tools/auto_bisect/bisect_utils.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
tools/auto_bisect/bisect_utils.py:278: spec = 'solutions=' + json.dumps(spec)
Is the spec interpreted as Python or JSON?

Most JSON is also valid Python, unless it contains the value true, false, or
null.

=>>> json.dumps({'managed': True})
=> '{"managed": true}'

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression.py
File tools/bisect-perf-regression.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression.py:2258: cmd = ['log', '--format=%ct', '-1',
good_revision]
Does this make a difference?

[Sergiy Byelozyorov, 2014-09-16 11:24:42]

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
File tools/auto_bisect/bisect_utils.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
tools/auto_bisect/bisect_utils.py:278: spec = 'solutions=' + json.dumps(spec)
On 2014/09/15 20:30:40, qyearsley wrote:
> Is the spec interpreted as Python or JSON?
> 
> Most JSON is also valid Python, unless it contains the value true, false, or
> null.
> 
> =>>> json.dumps({'managed': True})
> => '{"managed": true}'
Does gclient interpret this as Python via eval? I would think using json.load is
a lot safer.

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression.py
File tools/bisect-perf-regression.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression.py:2258: cmd = ['log', '--format=%ct', '-1',
good_revision]
On 2014/09/15 20:30:40, qyearsley wrote:
> Does this make a difference?

Not functionally, but it is consistent with grouping of statements below (cmd
goes before CheckRunGit), while cwd should be separate as it's used in both
pieces.

[qyearsley, 2014-09-16 18:59:18]

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
File tools/auto_bisect/bisect_utils.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
tools/auto_bisect/bisect_utils.py:278: spec = 'solutions=' + json.dumps(spec)
On 2014/09/16 11:24:42, Sergiy Byelozyorov wrote:
> On 2014/09/15 20:30:40, qyearsley wrote:
> > Is the spec interpreted as Python or JSON?
> > 
> > Most JSON is also valid Python, unless it contains the value true, false, or
> > null.
> > 
> > =>>> json.dumps({'managed': True})
> > => '{"managed": true}'
> Does gclient interpret this as Python via eval? I would think using json.load
is
> a lot safer.

Yeah, I think that gclient interprets it as Python using exec.
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/gcli...

I also think that JSON would be better, but given how gclient behaves, I suppose
this must still be given as Python without newlines here.

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression.py
File tools/bisect-perf-regression.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression.py:2258: cmd = ['log', '--format=%ct', '-1',
good_revision]
On 2014/09/16 11:24:42, Sergiy Byelozyorov wrote:
> On 2014/09/15 20:30:40, qyearsley wrote:
> > Does this make a difference?
> 
> Not functionally, but it is consistent with grouping of statements below (cmd
> goes before CheckRunGit), while cwd should be separate as it's used in both
> pieces.

I see; makes sense.

[Sergiy Byelozyorov, 2014-09-17 15:45:18]

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
File tools/auto_bisect/bisect_utils.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/auto_bisect/bisect_uti...
tools/auto_bisect/bisect_utils.py:278: spec = 'solutions=' + json.dumps(spec)
On 2014/09/16 18:59:18, qyearsley wrote:
> On 2014/09/16 11:24:42, Sergiy Byelozyorov wrote:
> > On 2014/09/15 20:30:40, qyearsley wrote:
> > > Is the spec interpreted as Python or JSON?
> > > 
> > > Most JSON is also valid Python, unless it contains the value true, false,
or
> > > null.
> > > 
> > > =>>> json.dumps({'managed': True})
> > > => '{"managed": true}'
> > Does gclient interpret this as Python via eval? I would think using
json.load
> is
> > a lot safer.
> 
> Yeah, I think that gclient interprets it as Python using exec.
>
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/gcli...
> 
> I also think that JSON would be better, but given how gclient behaves, I
suppose
> this must still be given as Python without newlines here.

Done.

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression.py
File tools/bisect-perf-regression.py (right):

https://codereview.chromium.org/568423002/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression.py:2258: cmd = ['log', '--format=%ct', '-1',
good_revision]
On 2014/09/16 18:59:18, qyearsley wrote:
> On 2014/09/16 11:24:42, Sergiy Byelozyorov wrote:
> > On 2014/09/15 20:30:40, qyearsley wrote:
> > > Does this make a difference?
> > 
> > Not functionally, but it is consistent with grouping of statements below
(cmd
> > goes before CheckRunGit), while cwd should be separate as it's used in both
> > pieces.
> 
> I see; makes sense.

Acknowledged.

[qyearsley, 2014-09-17 16:48:36]

lgtm now

[qyearsley, 2014-09-17 16:49:28]

On 2014/09/17 16:48:36, qyearsley wrote:
> lgtm now

Although, you should update the description as well to delete the "Use json
module to serialize gClient config." line.

[Sergiy Byelozyorov, 2014-09-18 07:50:32]

The CQ bit was checked by sergiyb@chromium.org

[commit-bot: I haz the power, 2014-09-18 07:50:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568423002/20001

[commit-bot: I haz the power, 2014-09-18 08:46:37]

Committed patchset #2 (id:20001) as d3b63d440320c82375aed8bd71a760db47531174

[commit-bot: I haz the power, 2014-09-18 08:49:10]

Patchset 2 (id:??) landed as
https://crrev.com/f1ae325a5db81e728b4e3075546cfdd1f24bc304
Cr-Commit-Position: refs/heads/master@{#295436}