Merge lp:~jeff-licquia/bzr-bisect/yesno into lp:bzr-bisect

Proposed by Jeff Licquia on 2012-09-16
Status: Needs review
Proposed branch: lp:~jeff-licquia/bzr-bisect/yesno
Merge into: lp:bzr-bisect
Diff against target: 69 lines (+37/-2)
2 files modified
cmds.py (+5/-2)
tests.py (+32/-0)
To merge this branch: bzr merge lp:~jeff-licquia/bzr-bisect/yesno
Reviewer Review Type Date Requested Status
Bazaar Developers 2012-09-16 Pending
Review via email: mp+124576@code.launchpad.net

Description of the change

This branch fixes bug 1051395. The meaning of "yes" and "no" has always been unclear, largely because the original idea for how they should work hasn't matched how it actually works. So, this patch makes things work the way they're described in the docs.

I've put a more detailed explanation in the bug.

To post a comment you must log in.

Unmerged revisions

85. By Jeff Licquia on 2012-09-16

Don't hardcode the association between "yes" and "high revision".

Originally, the intent was for "yes" and "no" to mean whatever the user
wanted it to mean, and figure out at runtime whether that made "yes" or
"no" the high rev. This feature was lost at some point (if it ever
existed) and "yes" was assumed to mean "high rev". This was confusing,
because the docs implied you could make "yes" or "no" mean whatever you
wanted.

Now, we figure out from the bisect log which identifier is high, and
stick with that moving forward. Also, there's now a test testing the
"no is high" case.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmds.py'
2--- cmds.py 2012-07-19 22:22:27 +0000
3+++ cmds.py 2012-09-16 15:30:25 +0000
4@@ -111,6 +111,7 @@
5 self._low_revid = None
6 self._middle_revid = None
7 self._filename = filename
8+ self._high_indicator = None
9 self.load()
10
11 def _open_for_read(self):
12@@ -160,10 +161,12 @@
13 continue
14 if len(matches) > 1:
15 raise RuntimeError("revision %s duplicated" % revision)
16- if matches[0] == "yes":
17+ if not self._high_indicator:
18+ self._high_indicator = matches[0]
19+ if matches[0] == self._high_indicator:
20 high_revid = revision
21 between_revs = []
22- elif matches[0] == "no":
23+ else:
24 low_revid = revision
25 del between_revs[0]
26 break
27
28=== modified file 'tests.py'
29--- tests.py 2011-11-24 16:04:11 +0000
30+++ tests.py 2012-09-16 15:30:25 +0000
31@@ -253,6 +253,38 @@
32 self.run_bzr(['bisect', 'no'])
33 self.assertRevno(3)
34
35+ def testWorkflowReverse(self):
36+ """Run through a basic usage scenario, except with senses of
37+ 'yes' and 'no' reversed. The plugin should pick up on that
38+ and adjust."""
39+
40+ # Start up the bisection. When the two ends are set, we should
41+ # end up in the middle.
42+
43+ self.run_bzr(['bisect', 'start'])
44+ self.run_bzr(['bisect', 'no'])
45+ self.run_bzr(['bisect', 'yes', '-r', '1'])
46+ self.assertRevno(3)
47+
48+ # Mark feature as absent in the middle. Should move us
49+ # halfway back between the current middle and the start.
50+
51+ self.run_bzr(['bisect', 'no'])
52+ self.assertRevno(2)
53+
54+ # Mark feature as present. Since this is only one
55+ # rev back from the lowest marked revision without the feature,
56+ # the process should end, with the current rev set to the
57+ # rev following.
58+
59+ self.run_bzr(['bisect', 'yes'])
60+ self.assertRevno(3)
61+
62+ # Run again. Since we're done, this should do nothing.
63+
64+ self.run_bzr(['bisect', 'yes'])
65+ self.assertRevno(3)
66+
67 def testWorkflowSubtree(self):
68 """Run through a usage scenario where the offending change
69 is in a subtree."""

Subscribers

People subscribed via source and target branches

to all changes: