Merge lp:~naesten/bzr-bisect/683822-bisect-start-range-argument into lp:bzr-bisect

Proposed by Samuel Bronson
Status: Needs review
Proposed branch: lp:~naesten/bzr-bisect/683822-bisect-start-range-argument
Merge into: lp:bzr-bisect
Diff against target: 252 lines (+99/-18)
2 files modified
cmds.py (+62/-18)
tests.py (+37/-0)
To merge this branch: bzr merge lp:~naesten/bzr-bisect/683822-bisect-start-range-argument
Reviewer Review Type Date Requested Status
Bazaar Developers Pending
Review via email: mp+124565@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Samuel Bronson (naesten) wrote :

Unfortunately, it turns out that this doesn't match the convention used by "run" for the common case of finding when a problem was introduced :-(.

87. By Samuel Bronson

Roll back my test changes; later, I'll add some tests just for this.

88. By Samuel Bronson

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

This gives "yes" and "no" equal standing again.

89. By Samuel Bronson

Reverse polarity of "start -r" and "run -r".

Now it matches the "run" commands's return-value handling.

90. By Samuel Bronson

Add a test for "bzr bisect run -r".

91. By Samuel Bronson

Add some debug output (mostly log-only).

Revision history for this message
Samuel Bronson (naesten) wrote :

> Unfortunately, it turns out that this doesn't match the convention used by
> "run" for the common case of finding when a problem was introduced :-(.

Okay, as of revision 89, I've matched the polarities here; now, it's the bulk of the tests that don't match ;-).

Unmerged revisions

91. By Samuel Bronson

Add some debug output (mostly log-only).

90. By Samuel Bronson

Add a test for "bzr bisect run -r".

89. By Samuel Bronson

Reverse polarity of "start -r" and "run -r".

Now it matches the "run" commands's return-value handling.

88. By Samuel Bronson

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

This gives "yes" and "no" equal standing again.

87. By Samuel Bronson

Roll back my test changes; later, I'll add some tests just for this.

86. By Samuel Bronson

Implement the "-r revA..revB" syntax for "start" and "run" suggested in bug 683822.

Quite likely this is buggy, and none of the scenareos in the test suite (some of which I converted to this syntax) are complicated enough to show this: they all seem to start by setting the bisect range to cover all revisions on the branch.

More tests would be quite useful!

85. By Samuel Bronson

Cleanup option handling preperatory to implementing bug 683822.

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-21 21:50:23 +0000
4@@ -23,7 +23,7 @@
5 from bzrlib.commands import Command
6 from bzrlib.errors import BzrCommandError
7 from bzrlib.option import Option
8-from bzrlib.trace import note
9+from bzrlib.trace import note, mutter
10
11 bisect_info_path = ".bzr/bisect"
12 bisect_rev_path = ".bzr/bisect_revid"
13@@ -75,6 +75,7 @@
14 """Write the current revision's log entry to a file."""
15 rev = self._bzrbranch.repository.get_revision(self._revid)
16 revno = ".".join([str(x) for x in self.get_current_revno()])
17+ mutter("On revision %s (%s)", revno, rev.revision_id)
18 out.write("On revision %s (%s):\n%s\n" % (revno, rev.revision_id,
19 rev.message))
20
21@@ -111,6 +112,7 @@
22 self._low_revid = None
23 self._middle_revid = None
24 self._filename = filename
25+ self._high_indicator = None
26 self.load()
27
28 def _open_for_read(self):
29@@ -160,10 +162,12 @@
30 continue
31 if len(matches) > 1:
32 raise RuntimeError("revision %s duplicated" % revision)
33- if matches[0] == "yes":
34+ if not self._high_indicator:
35+ self._high_indicator = matches[0]
36+ if matches[0] == self._high_indicator:
37 high_revid = revision
38 between_revs = []
39- elif matches[0] == "no":
40+ else:
41 low_revid = revision
42 del between_revs[0]
43 break
44@@ -201,6 +205,7 @@
45 def _set_status(self, revid, status):
46 """Set the bisect status for the given revid."""
47 if not self.is_done():
48+ note("Marking %s as %s", revid, status)
49 if status != "done" and revid in [x[0] for x in self._items
50 if x[1] in ['yes', 'no']]:
51 raise RuntimeError("attempting to add revid %s twice" % revid)
52@@ -253,6 +258,7 @@
53
54 def bisect(self, outf):
55 """Using the current revision's status, do a bisection."""
56+ mutter("Bisecting")
57 self._find_range_and_middle()
58 # If we've found the "final" revision, check for a
59 # merge point.
60@@ -297,6 +303,10 @@
61 The specified revision (or the current revision, if not given)
62 does not have the characteristic we're looking for,
63
64+ bzr bisect start -r revA..revB
65+ Like: "bzr bisect start; bzr bisect yes revA; bzr bisect no revB".
66+ If without revA, skip the "yes"; without revB, skip the "no".
67+
68 bzr bisect move -r rev
69 Switch to a different revision manually. Use if the bisect
70 algorithm chooses a revision that is not suitable. Try to
71@@ -313,12 +323,14 @@
72 Replay a previously-saved bisect log, forgetting any bisection
73 that might be in progress.
74
75- bzr bisect run <script>
76+ bzr bisect run [-r revA:revB] <script>
77 Bisect automatically using <script> to determine 'yes' or 'no'.
78 <script> should exit with:
79 0 for yes
80 125 for unknown (like build failed so we could not test)
81 anything else for no
82+
83+ "-r revA:revB" is handled as for "start".
84 """
85
86 takes_args = ['subcommand', 'args*']
87@@ -353,23 +365,47 @@
88 """Handle the bisect command."""
89
90 log_fn = None
91+ # Handle args_list
92+ if subcommand == 'replay':
93+ if len(args_list) > 1:
94+ raise BzrCommandError(
95+ "The 'bisect replay' command only accepts one argument.")
96+ elif args_list:
97+ log_fn = args_list[0]
98+ pass
99+ elif subcommand == 'run':
100+ if len(args_list) != 1:
101+ raise BzrCommandError(
102+ "The 'bisect run' command requires a single argument.")
103+ run_script = args_list[0]
104+ elif args_list:
105+ raise BzrCommandError(
106+ "The 'bisect %s' command doesn't take any arguments." %
107+ (subcommand))
108+
109+ # Handle revision
110 if subcommand in ('yes', 'no', 'move') and revision:
111- pass
112- elif subcommand in ('replay', ) and args_list and len(args_list) == 1:
113- log_fn = args_list[0]
114- elif subcommand in ('move', ) and not revision:
115+ if len(revision) > 1:
116+ raise BzrCommandError(
117+ "The 'bisect %s' command doesn't accept revision ranges." %
118+ (subcommand))
119+ elif subcommand == 'move' and not revision:
120 raise BzrCommandError(
121 "The 'bisect move' command requires a revision.")
122- elif subcommand in ('run', ):
123- run_script = args_list[0]
124- elif args_list or revision:
125+ elif subcommand in ('start', 'run') and revision:
126+ if len(revision) != 2:
127+ raise BzrCommandError(
128+ "The 'bisect %s' command only accepts revision ranges." %
129+ (subcommand))
130+ elif revision:
131 raise BzrCommandError(
132- "Improper arguments to bisect " + subcommand)
133+ "The 'bisect %s' command doesn't accept revspecs." %
134+ (subcommand))
135
136 # Dispatch.
137
138 if subcommand == "start":
139- self.start()
140+ self.start(revision)
141 elif subcommand == "yes":
142 self.yes(revision)
143 elif subcommand == "no":
144@@ -383,7 +419,7 @@
145 elif subcommand == "replay":
146 self.replay(log_fn)
147 elif subcommand == "run":
148- self.run_bisect(run_script)
149+ self.run_bisect(run_script, revision)
150 else:
151 raise BzrCommandError(
152 "Unknown bisect command: " + subcommand)
153@@ -394,7 +430,7 @@
154 BisectCurrent().reset()
155 os.unlink(bisect_info_path)
156
157- def start(self):
158+ def start(self, revspec):
159 """Reset the bisect state, then prepare for a new bisection."""
160 if os.path.exists(bisect_info_path):
161 BisectCurrent().reset()
162@@ -402,6 +438,13 @@
163
164 bisect_log = BisectLog()
165 bisect_log.set_current("start")
166+
167+ if revspec:
168+ revA, revB = revspec
169+ if revA: bisect_log.set_status_from_revspec([revA], 'yes')
170+ if revB: bisect_log.set_status_from_revspec([revB], 'no')
171+ bisect_log.bisect(self.outf)
172+
173 bisect_log.save()
174
175 def yes(self, revspec):
176@@ -437,15 +480,16 @@
177
178 bisect_log.bisect(self.outf)
179
180- def run_bisect(self, script):
181+ def run_bisect(self, script, revspec):
182 import subprocess
183 note("Starting bisect.")
184- self.start()
185+ self.start(revspec)
186 while True:
187 try:
188 process = subprocess.Popen(script, shell=True)
189 process.wait()
190 retcode = process.returncode
191+ mutter("Script returned %d", retcode)
192 if retcode == 0:
193 done = self._set_state(None, 'yes')
194 elif retcode == 125:
195@@ -455,4 +499,4 @@
196 if done:
197 break
198 except RuntimeError:
199- break
200+ raise
201
202=== modified file 'tests.py'
203--- tests.py 2011-11-24 16:04:11 +0000
204+++ tests.py 2012-09-21 21:50:23 +0000
205@@ -253,6 +253,38 @@
206 self.run_bzr(['bisect', 'no'])
207 self.assertRevno(3)
208
209+ def testWorkflowReverse(self):
210+ """Run through a basic usage scenario, except with senses of
211+ 'yes' and 'no' reversed. The plugin should pick up on that
212+ and adjust."""
213+
214+ # Start up the bisection. When the two ends are set, we should
215+ # end up in the middle.
216+
217+ self.run_bzr(['bisect', 'start'])
218+ self.run_bzr(['bisect', 'no'])
219+ self.run_bzr(['bisect', 'yes', '-r', '1'])
220+ self.assertRevno(3)
221+
222+ # Mark feature as absent in the middle. Should move us
223+ # halfway back between the current middle and the start.
224+
225+ self.run_bzr(['bisect', 'no'])
226+ self.assertRevno(2)
227+
228+ # Mark feature as present. Since this is only one
229+ # rev back from the lowest marked revision without the feature,
230+ # the process should end, with the current rev set to the
231+ # rev following.
232+
233+ self.run_bzr(['bisect', 'yes'])
234+ self.assertRevno(3)
235+
236+ # Run again. Since we're done, this should do nothing.
237+
238+ self.run_bzr(['bisect', 'yes'])
239+ self.assertRevno(3)
240+
241 def testWorkflowSubtree(self):
242 """Run through a usage scenario where the offending change
243 is in a subtree."""
244@@ -414,3 +446,8 @@
245 raise KnownFailure\
246 ("bisect does not drill down into merge commits: "
247 "https://bugs.launchpad.net/bzr-bisect/+bug/539937")
248+
249+ def testRunRange(self):
250+ out, err = self.run_bzr(['bisect', 'run', '-r', '2..5',
251+ '! grep -q one test_file_append'])
252+ self.assertRevno(3)

Subscribers

People subscribed via source and target branches

to all changes: