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

Proposed by Jeff Licquia
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 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

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
=== modified file 'cmds.py'
--- cmds.py 2012-07-19 22:22:27 +0000
+++ cmds.py 2012-09-16 15:30:25 +0000
@@ -111,6 +111,7 @@
111 self._low_revid = None111 self._low_revid = None
112 self._middle_revid = None112 self._middle_revid = None
113 self._filename = filename113 self._filename = filename
114 self._high_indicator = None
114 self.load()115 self.load()
115116
116 def _open_for_read(self):117 def _open_for_read(self):
@@ -160,10 +161,12 @@
160 continue161 continue
161 if len(matches) > 1:162 if len(matches) > 1:
162 raise RuntimeError("revision %s duplicated" % revision)163 raise RuntimeError("revision %s duplicated" % revision)
163 if matches[0] == "yes":164 if not self._high_indicator:
165 self._high_indicator = matches[0]
166 if matches[0] == self._high_indicator:
164 high_revid = revision167 high_revid = revision
165 between_revs = []168 between_revs = []
166 elif matches[0] == "no":169 else:
167 low_revid = revision170 low_revid = revision
168 del between_revs[0]171 del between_revs[0]
169 break172 break
170173
=== modified file 'tests.py'
--- tests.py 2011-11-24 16:04:11 +0000
+++ tests.py 2012-09-16 15:30:25 +0000
@@ -253,6 +253,38 @@
253 self.run_bzr(['bisect', 'no'])253 self.run_bzr(['bisect', 'no'])
254 self.assertRevno(3)254 self.assertRevno(3)
255255
256 def testWorkflowReverse(self):
257 """Run through a basic usage scenario, except with senses of
258 'yes' and 'no' reversed. The plugin should pick up on that
259 and adjust."""
260
261 # Start up the bisection. When the two ends are set, we should
262 # end up in the middle.
263
264 self.run_bzr(['bisect', 'start'])
265 self.run_bzr(['bisect', 'no'])
266 self.run_bzr(['bisect', 'yes', '-r', '1'])
267 self.assertRevno(3)
268
269 # Mark feature as absent in the middle. Should move us
270 # halfway back between the current middle and the start.
271
272 self.run_bzr(['bisect', 'no'])
273 self.assertRevno(2)
274
275 # Mark feature as present. Since this is only one
276 # rev back from the lowest marked revision without the feature,
277 # the process should end, with the current rev set to the
278 # rev following.
279
280 self.run_bzr(['bisect', 'yes'])
281 self.assertRevno(3)
282
283 # Run again. Since we're done, this should do nothing.
284
285 self.run_bzr(['bisect', 'yes'])
286 self.assertRevno(3)
287
256 def testWorkflowSubtree(self):288 def testWorkflowSubtree(self):
257 """Run through a usage scenario where the offending change289 """Run through a usage scenario where the offending change
258 is in a subtree."""290 is in a subtree."""

Subscribers

People subscribed via source and target branches

to all changes: