Add option (u)nknown to bisect-builds.py.

Add option (u)nknown to bisect-builds.py.

This is useful when the revision being tested has some other problem that prevents testing the original issue.

BUG=none
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=139967

[Robert Sesek, 2012-05-31 19:46:10]

Clever!

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py
File tools/bisect-builds.py (right):

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:409: @param evaluate A function which returns 'g' if the
argument Chromium
Since this can now bisect official builds, just say "if the argument build is
good"

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:512: else:  # answer == 'u'
I'd just make this an elseif and then have an else with something like
NOTREACHED()

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:524: fetch = random.choice([up_fetch, down_fetch])
Any reason to do this instead of [up_fetch, down_fetch][bad % 2]? Gives a
similar pseudorandom effect. (or len(revlist))

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:529: if fetch:
When would |fetch| be None still?

[Alexei Svitkine (slow), 2012-05-31 20:08:15]

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py
File tools/bisect-builds.py (right):

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:409: @param evaluate A function which returns 'g' if the
argument Chromium
On 2012/05/31 19:46:10, rsesek wrote:
> Since this can now bisect official builds, just say "if the argument build is
> good"

Done.

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:512: else:  # answer == 'u'
On 2012/05/31 19:46:10, rsesek wrote:
> I'd just make this an elseif and then have an else with something like
> NOTREACHED()

Done.

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:524: fetch = random.choice([up_fetch, down_fetch])
On 2012/05/31 19:46:10, rsesek wrote:
> Any reason to do this instead of [up_fetch, down_fetch][bad % 2]? Gives a
> similar pseudorandom effect. (or len(revlist))

I like len(revlist), since this will alternate between using up/down. Done!

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:529: if fetch:
On 2012/05/31 19:46:10, rsesek wrote:
> When would |fetch| be None still?

If |down_fetch| is None, which I assumed could happen based on the structure of
the above code, which does:

 if up_fetch:
   up_fetch.Stop()
 if down_fetch:
   ...

... Instead of using an elif.

Although, I haven't reasoned whether such a case is possible. If it's not then,
we should change the code above too.

Let me think about it.

[Alexei Svitkine (slow), 2012-05-31 20:26:46]

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py
File tools/bisect-builds.py (right):

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:529: if fetch:
On 2012/05/31 20:08:15, Alexei Svitkine wrote:
> On 2012/05/31 19:46:10, rsesek wrote:
> > When would |fetch| be None still?
>
> Let me think about it.

So, this condition could happen if there are only 3 revisions in the revlist,
and bad=0,pivot=1,good=2. Since (good - bad > 1), this still qualified for entry
into the loop, but since there are no revisions between bad and pivot and pivot
and good, both down_fetch and up_fetch are None.

However, in this scenario, if we get the 'u' answer, we will have trimmed the
revlist to just two revisions, and hence bad - good > 1 will no longer hold, so
you're right that this shouldn't not happen here (if my reasoning is correct).

I'll remove the check here, but not above.

[Alexei Svitkine (slow), 2012-05-31 20:29:10]

> and bad=0,pivot=1,good=2.

Oops, meant "good=0,pivot=1,bad=2".

[Alexei Svitkine (slow), 2012-05-31 20:32:52]

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py
File tools/bisect-builds.py (right):

http://codereview.chromium.org/10459050/diff/6001/tools/bisect-builds.py#newc...
tools/bisect-builds.py:529: if fetch:
> I'll remove the check here, but not above.

Done.

[Robert Sesek, 2012-05-31 20:56:06]

LGTM!

[commit-bot: I haz the power, 2012-05-31 21:00:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asvitkine@chromium.org/10459050/5002

[commit-bot: I haz the power, 2012-05-31 22:33:46]

Try job failure for 10459050-5002 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-31 23:11:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asvitkine@chromium.org/10459050/5002

[commit-bot: I haz the power, 2012-06-01 04:11:02]

Change committed as 139967