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

Proposed by Samuel Bronson on 2012-09-16
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 2012-09-16 Pending
Review via email: mp+124565@code.launchpad.net
To post a comment you must log in.
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 on 2012-09-18

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

88. By Samuel Bronson on 2012-09-18

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

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

89. By Samuel Bronson on 2012-09-19

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

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

90. By Samuel Bronson on 2012-09-19

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

91. By Samuel Bronson on 2012-09-21

Add some debug output (mostly log-only).

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 on 2012-09-21

Add some debug output (mostly log-only).

90. By Samuel Bronson on 2012-09-19

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

89. By Samuel Bronson on 2012-09-19

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

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

88. By Samuel Bronson on 2012-09-18

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

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

87. By Samuel Bronson on 2012-09-18

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

86. By Samuel Bronson on 2012-09-16

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 on 2012-09-15

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: