2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1 |
====================== |
2 |
Bazaar Developer Guide |
|
3 |
====================== |
|
974.1.26
by aaron.bentley at utoronto
merged mbp@sourcefrog.net-20050817233101-0939da1cf91f2472 |
4 |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
5 |
.. contents:: |
6 |
||
2666.2.3
by Alexander Belchenko
fixes after Ian's review |
7 |
(The current version of this document is available in the file |
8 |
``doc/developers/HACKING.txt`` in the source tree, or at |
|
2666.2.1
by Alexander Belchenko
change generated documentation extension from htm to html |
9 |
http://doc.bazaar-vcs.org/bzr.dev/developers/HACKING.html) |
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
10 |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
11 |
|
12 |
Getting Started |
|
13 |
############### |
|
14 |
||
15 |
Exploring the Bazaar Platform |
|
16 |
============================= |
|
17 |
||
18 |
Before making changes, it's a good idea to explore the work already |
|
19 |
done by others. Perhaps the new feature or improvement you're looking |
|
20 |
for is available in another plug-in already? If you find a bug, |
|
21 |
perhaps someone else has already fixed it? |
|
22 |
||
23 |
To answer these questions and more, take a moment to explore the |
|
24 |
overall Bazaar Platform. Here are some links to browse: |
|
25 |
||
26 |
* The Plugins page on the Wiki - http://bazaar-vcs.org/BzrPlugins |
|
27 |
||
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
28 |
* The Bazaar product family on Launchpad - https://launchpad.net/bazaar |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
29 |
|
30 |
* Bug Tracker for the core product - https://bugs.launchpad.net/bzr/ |
|
31 |
||
32 |
* Blueprint Tracker for the core product - https://blueprints.launchpad.net/bzr/ |
|
33 |
||
34 |
If nothing else, perhaps you'll find inspiration in how other developers |
|
35 |
have solved their challenges. |
|
36 |
||
37 |
||
38 |
Planning and Discussing Changes |
|
39 |
=============================== |
|
40 |
||
41 |
There is a very active community around Bazaar. Mostly we meet on IRC |
|
42 |
(#bzr on irc.freenode.net) and on the mailing list. To join the Bazaar |
|
43 |
community, see http://bazaar-vcs.org/BzrSupport. |
|
44 |
||
45 |
If you are planning to make a change, it's a very good idea to mention it |
|
46 |
on the IRC channel and/or on the mailing list. There are many advantages |
|
47 |
to involving the community before you spend much time on a change. |
|
48 |
These include: |
|
49 |
||
50 |
* you get to build on the wisdom on others, saving time |
|
51 |
||
52 |
* if others can direct you to similar code, it minimises the work to be done |
|
53 |
||
54 |
* it assists everyone in coordinating direction, priorities and effort. |
|
55 |
||
56 |
In summary, maximising the input from others typically minimises the |
|
57 |
total effort required to get your changes merged. The community is |
|
58 |
friendly, helpful and always keen to welcome newcomers. |
|
59 |
||
60 |
||
61 |
Bazaar Development in a Nutshell |
|
62 |
================================ |
|
63 |
||
64 |
Looking for a 10 minute introduction to submitting a change? |
|
65 |
See http://bazaar-vcs.org/BzrGivingBack. |
|
66 |
||
67 |
TODO: Merge that Wiki page into this document. |
|
68 |
||
69 |
||
70 |
Understanding the Development Process |
|
71 |
===================================== |
|
72 |
||
73 |
The development team follows many best-practices including: |
|
74 |
||
75 |
* a public roadmap and planning process in which anyone can participate |
|
76 |
||
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
77 |
* time based milestones everyone can work towards and plan around |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
78 |
|
79 |
* extensive code review and feedback to contributors |
|
80 |
||
81 |
* complete and rigorous test coverage on any code contributed |
|
82 |
||
83 |
* automated validation that all tests still pass before code is merged |
|
84 |
into the main code branch. |
|
85 |
||
86 |
The key tools we use to enable these practices are: |
|
87 |
||
88 |
* Launchpad - https://launchpad.net/ |
|
89 |
||
90 |
* Bazaar - http://bazaar-vcs.org/ |
|
91 |
||
92 |
* Bundle Buggy - http://bundlebuggy.aaronbentley.com/ |
|
93 |
||
94 |
* Patch Queue Manager - https://launchpad.net/pqm/ |
|
95 |
||
96 |
For further information, see http://bazaar-vcs.org/BzrDevelopment. |
|
97 |
||
98 |
||
99 |
A Closer Look at the Merge & Review Process |
|
100 |
=========================================== |
|
101 |
||
102 |
If you'd like to propose a change, please post to the |
|
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
103 |
bazaar@lists.canonical.com list with a bundle, patch, or link to a |
104 |
branch. Put '[PATCH]' or '[MERGE]' in the subject so Bundle Buggy |
|
105 |
can pick it out, and explain the change in the email message text. |
|
106 |
Remember to update the NEWS file as part of your change if it makes any |
|
107 |
changes visible to users or plugin developers. Please include a diff |
|
108 |
against mainline if you're giving a link to a branch. |
|
109 |
||
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
110 |
You can generate a bundle like this:: |
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
111 |
|
112 |
bzr bundle > mybundle.patch |
|
113 |
||
114 |
A .patch extension is recommended instead of .bundle as many mail clients |
|
115 |
will send the latter as a binary file. If a bundle would be too long or your |
|
116 |
mailer mangles whitespace (e.g. implicitly converts Unix newlines to DOS |
|
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
117 |
newlines), use the merge-directive command instead like this:: |
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
118 |
|
119 |
bzr merge-directive http://bazaar-vcs.org http://example.org/my_branch > my_directive.patch |
|
120 |
||
121 |
See the help for details on the arguments to merge-directive. |
|
122 |
||
123 |
Please do **NOT** put [PATCH] or [MERGE] in the subject line if you don't |
|
124 |
want it to be merged. If you want comments from developers rather than |
|
125 |
to be merged, you can put '[RFC]' in the subject line. |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
126 |
|
127 |
Anyone is welcome to review code. There are broadly three gates for |
|
128 |
code to get in: |
|
129 |
||
130 |
* Doesn't reduce test coverage: if it adds new methods or commands, |
|
131 |
there should be tests for them. There is a good test framework |
|
132 |
and plenty of examples to crib from, but if you are having trouble |
|
133 |
working out how to test something feel free to post a draft patch |
|
134 |
and ask for help. |
|
135 |
||
136 |
* Doesn't reduce design clarity, such as by entangling objects |
|
137 |
we're trying to separate. This is mostly something the more |
|
138 |
experienced reviewers need to help check. |
|
139 |
||
140 |
* Improves bugs, features, speed, or code simplicity. |
|
141 |
||
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
142 |
Code that goes in should pass all three. The core developers take care |
143 |
to keep the code quality high and understandable while recognising that |
|
144 |
perfect is sometimes the enemy of good. (It is easy for reviews to make |
|
145 |
people notice other things which should be fixed but those things should |
|
146 |
not hold up the original fix being accepted. New things can easily be |
|
147 |
recorded in the Bug Tracker instead.) |
|
148 |
||
149 |
Anyone can "vote" on the mailing list. Core developers can also vote using |
|
150 |
Bundle Buggy. Here are the voting codes and their explanations. |
|
151 |
||
2654.1.1
by Aaron Bentley
Revise text about voting to match current system |
152 |
:approve: Reviewer wants this submission merged. |
153 |
:tweak: Reviewer wants this submission merged with small changes. (No |
|
154 |
re-review required.) |
|
155 |
:abstain: Reviewer does not intend to vote on this patch. |
|
156 |
:resubmit: Please make changes and resubmit for review. |
|
157 |
:reject: Reviewer doesn't want this kind of change merged. |
|
158 |
:comment: Not really a vote. Reviewer just wants to comment, for now. |
|
159 |
||
160 |
If a change gets two approvals from core reviewers, and no rejections, |
|
161 |
then it's OK to come in. Any of the core developers can bring it into the |
|
162 |
bzr.dev trunk and backport it to maintenance branches if required. The |
|
163 |
Release Manager will merge the change into the branch for a pending |
|
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
164 |
release, if any. As a guideline, core developers usually merge their own |
165 |
changes and volunteer to merge other contributions if they were the second |
|
166 |
reviewer to agree to a change. |
|
167 |
||
168 |
To track the progress of proposed changes, use Bundle Buggy. See |
|
169 |
http://bundlebuggy.aaronbentley.com/help for a link to all the |
|
170 |
outstanding merge requests together with an explanation of the columns. |
|
171 |
Bundle Buggy will also mail you a link to track just your change. |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
172 |
|
173 |
||
174 |
Preparing a Sandbox for Making Changes to Bazaar |
|
175 |
================================================ |
|
176 |
||
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
177 |
Bazaar supports many ways of organising your work. See |
178 |
http://bazaar-vcs.org/SharedRepositoryLayouts for a summary of the |
|
179 |
popular alternatives. |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
180 |
|
181 |
Of course, the best choice for you will depend on numerous factors: |
|
182 |
the number of changes you may be making, the complexity of the changes, etc. |
|
183 |
As a starting suggestion though: |
|
184 |
||
185 |
* create a local copy of the main development branch (bzr.dev) by using |
|
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
186 |
this command:: |
187 |
||
188 |
bzr branch http://bazaar-vcs.org/bzr/bzr.dev/ bzr.dev |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
189 |
|
190 |
* keep your copy of bzr.dev prestine (by not developing in it) and keep |
|
191 |
it up to date (by using bzr pull) |
|
192 |
||
193 |
* create a new branch off your local bzr.dev copy for each issue |
|
194 |
(bug or feature) you are working on. |
|
195 |
||
196 |
This approach makes it easy to go back and make any required changes |
|
197 |
after a code review. Resubmitting the change is then simple with no |
|
198 |
risk of accidentially including edits related to other issues you may |
|
199 |
be working on. After the changes for an issue are accepted and merged, |
|
200 |
the associated branch can be deleted or archived as you wish. |
|
201 |
||
202 |
||
203 |
Navigating the Code Base |
|
204 |
======================== |
|
205 |
||
206 |
TODO: List and describe in one line the purpose of each directory |
|
207 |
inside an installation of bzr. |
|
208 |
||
209 |
TODO: Refer to a central location holding an up to date copy of the API |
|
210 |
documentation generated by epydoc, e.g. something like |
|
211 |
http://starship.python.net/crew/mwh/bzrlibapi/bzrlib.html. |
|
212 |
||
213 |
||
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
214 |
Testing Bazaar |
215 |
############## |
|
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
216 |
|
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
217 |
The Importance of Testing |
218 |
========================= |
|
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
219 |
|
220 |
Reliability is a critical success factor for any Version Control System. |
|
221 |
We want Bazaar to be highly reliable across multiple platforms while |
|
222 |
evolving over time to meet the needs of its community. |
|
223 |
||
224 |
In a nutshell, this is want we expect and encourage: |
|
225 |
||
226 |
* New functionality should have test cases. Preferably write the |
|
227 |
test before writing the code. |
|
228 |
||
229 |
In general, you can test at either the command-line level or the |
|
2466.7.2
by Robert Collins
Document the user of TreeBuilder somewhat. |
230 |
internal API level. See Writing tests below for more detail. |
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
231 |
|
232 |
* Try to practice Test-Driven Development: before fixing a bug, write a |
|
233 |
test case so that it does not regress. Similarly for adding a new |
|
234 |
feature: write a test case for a small version of the new feature before |
|
235 |
starting on the code itself. Check the test fails on the old code, then |
|
236 |
add the feature or fix and check it passes. |
|
237 |
||
238 |
By doing these things, the Bazaar team gets increased confidence that |
|
239 |
changes do what they claim to do, whether provided by the core team or |
|
240 |
by community members. Equally importantly, we can be surer that changes |
|
241 |
down the track do not break new features or bug fixes that you are |
|
242 |
contributing today. |
|
243 |
||
244 |
As of May 2007, Bazaar ships with a test suite containing over 6000 tests |
|
245 |
and growing. We are proud of it and want to remain so. As community |
|
246 |
members, we all benefit from it. Would you trust version control on |
|
247 |
your project to a product *without* a test suite like Bazaar has? |
|
248 |
||
249 |
||
250 |
Running the Test Suite |
|
251 |
====================== |
|
252 |
||
253 |
Currently, bzr selftest is used to invoke tests. |
|
254 |
You can provide a pattern argument to run a subset. For example, |
|
255 |
to run just the blackbox tests, run:: |
|
256 |
||
257 |
./bzr selftest -v blackbox |
|
258 |
||
259 |
To skip a particular test (or set of tests), use the --exclude option |
|
260 |
(shorthand -x) like so:: |
|
261 |
||
262 |
./bzr selftest -v -x blackbox |
|
263 |
||
2658.3.5
by Daniel Watkins
Added note regarding --strict to HACKING. |
264 |
To ensure that all tests are being run and succeeding, you can use the |
265 |
--strict option which will fail if there are any missing features or known |
|
266 |
failures, like so:: |
|
267 |
||
268 |
./bzr selftest --strict |
|
269 |
||
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
270 |
To list tests without running them, use the --list-only option like so:: |
271 |
||
272 |
./bzr selftest --list-only |
|
273 |
||
274 |
This option can be combined with other selftest options (like -x) and |
|
275 |
filter patterns to understand their effect. |
|
276 |
||
277 |
||
278 |
Writing Tests |
|
279 |
============= |
|
280 |
||
281 |
In general tests should be placed in a file named test_FOO.py where |
|
282 |
FOO is the logical thing under test. That file should be placed in the |
|
283 |
tests subdirectory under the package being tested. |
|
284 |
||
285 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/test_merge3.py. |
|
286 |
See bzrlib/tests/test_sampler.py for a template test script. |
|
287 |
||
288 |
Tests can be written for the UI or for individual areas of the library. |
|
289 |
Choose whichever is appropriate: if adding a new command, or a new command |
|
290 |
option, then you should be writing a UI test. If you are both adding UI |
|
291 |
functionality and library functionality, you will want to write tests for |
|
292 |
both the UI and the core behaviours. We call UI tests 'blackbox' tests |
|
293 |
and they are found in ``bzrlib/tests/blackbox/*.py``. |
|
294 |
||
295 |
When writing blackbox tests please honour the following conventions: |
|
296 |
||
297 |
1. Place the tests for the command 'name' in |
|
298 |
bzrlib/tests/blackbox/test_name.py. This makes it easy for developers |
|
299 |
to locate the test script for a faulty command. |
|
300 |
||
301 |
2. Use the 'self.run_bzr("name")' utility function to invoke the command |
|
302 |
rather than running bzr in a subprocess or invoking the |
|
303 |
cmd_object.run() method directly. This is a lot faster than |
|
304 |
subprocesses and generates the same logging output as running it in a |
|
305 |
subprocess (which invoking the method directly does not). |
|
306 |
||
307 |
3. Only test the one command in a single test script. Use the bzrlib |
|
308 |
library when setting up tests and when evaluating the side-effects of |
|
309 |
the command. We do this so that the library api has continual pressure |
|
310 |
on it to be as functional as the command line in a simple manner, and |
|
311 |
to isolate knock-on effects throughout the blackbox test suite when a |
|
312 |
command changes its name or signature. Ideally only the tests for a |
|
313 |
given command are affected when a given command is changed. |
|
314 |
||
315 |
4. If you have a test which does actually require running bzr in a |
|
316 |
subprocess you can use ``run_bzr_subprocess``. By default the spawned |
|
317 |
process will not load plugins unless ``--allow-plugins`` is supplied. |
|
318 |
||
319 |
||
320 |
Doctests |
|
321 |
-------- |
|
322 |
||
323 |
We make selective use of doctests__. In general they should provide |
|
324 |
*examples* within the API documentation which can incidentally be tested. We |
|
325 |
don't try to test every important case using doctests -- regular Python |
|
326 |
tests are generally a better solution. |
|
327 |
||
328 |
Most of these are in ``bzrlib/doc/api``. More additions are welcome. |
|
329 |
||
330 |
__ http://docs.python.org/lib/module-doctest.html |
|
331 |
||
332 |
||
2475.2.3
by Martin Pool
Merge ian's HACKING updates |
333 |
Skipping tests and test requirements |
334 |
------------------------------------ |
|
335 |
||
336 |
In our enhancements to unittest we allow for some addition results beyond |
|
337 |
just success or failure. |
|
338 |
||
339 |
If a test can't be run, it can say that it's skipped. This is typically |
|
340 |
used in parameterized tests - for example if a transport doesn't support |
|
341 |
setting permissions, we'll skip the tests that relating to that. :: |
|
342 |
||
343 |
try: |
|
344 |
return self.branch_format.initialize(repo.bzrdir) |
|
345 |
except errors.UninitializableFormat: |
|
346 |
raise tests.TestSkipped('Uninitializable branch format') |
|
347 |
||
348 |
Raising TestSkipped is a good idea when you want to make it clear that the |
|
349 |
test was not run, rather than just returning which makes it look as if it |
|
350 |
was run and passed. |
|
351 |
||
2729.1.1
by Martin Pool
Add TestNotApplicable exception and handling of it; document test parameterization |
352 |
Several different cases are distinguished: |
353 |
||
354 |
TestSkipped |
|
355 |
Generic skip; the only type that was present up to bzr 0.18. |
|
356 |
||
357 |
TestNotApplicable |
|
358 |
The test doesn't apply to the parameters with which it was run. |
|
359 |
This is typically used when the test is being applied to all |
|
360 |
implementations of an interface, but some aspects of the interface |
|
361 |
are optional and not present in particular concrete |
|
362 |
implementations. (Some tests that should raise this currently |
|
363 |
either silently return or raise TestSkipped.) Another option is |
|
364 |
to use more precise parameterization to avoid generating the test |
|
365 |
at all. |
|
366 |
||
367 |
TestPlatformLimit |
|
368 |
**(Not implemented yet)** |
|
369 |
The test can't be run because of an inherent limitation of the |
|
370 |
environment, such as not having symlinks or not supporting |
|
371 |
unicode. |
|
372 |
||
373 |
UnavailableFeature |
|
374 |
The test can't be run because a dependency (typically a Python |
|
375 |
library) is not available in the test environment. These |
|
376 |
are in general things that the person running the test could fix |
|
377 |
by installing the library. It's OK if some of these occur when |
|
378 |
an end user runs the tests or if we're specifically testing in a |
|
379 |
limited environment, but a full test should never see them. |
|
380 |
||
381 |
KnownFailure |
|
382 |
The test exists but is known to fail, for example because the |
|
383 |
code to fix it hasn't been run yet. Raising this allows |
|
384 |
you to distinguish these failures from the ones that are not |
|
385 |
expected to fail. This could be conditionally raised if something |
|
386 |
is broken on some platforms but not on others. |
|
387 |
||
388 |
We plan to support three modes for running the test suite to control the |
|
389 |
interpretation of these results. Strict mode is for use in situations |
|
390 |
like merges to the mainline and releases where we want to make sure that |
|
391 |
everything that can be tested has been tested. Lax mode is for use by |
|
392 |
developers who want to temporarily tolerate some known failures. The |
|
393 |
default behaviour is obtained by ``bzr selftest`` with no options, and |
|
394 |
also (if possible) by running under another unittest harness. |
|
395 |
||
396 |
======================= ======= ======= ======== |
|
397 |
result strict default lax |
|
398 |
======================= ======= ======= ======== |
|
399 |
TestSkipped pass pass pass |
|
400 |
TestNotApplicable pass pass pass |
|
401 |
TestPlatformLimit pass pass pass |
|
402 |
TestDependencyMissing fail pass pass |
|
2729.1.6
by Martin Pool
Update docs to say xfail does not cause overall failure in default test runs, which is true at the moment |
403 |
KnownFailure fail pass pass |
2729.1.1
by Martin Pool
Add TestNotApplicable exception and handling of it; document test parameterization |
404 |
======================= ======= ======= ======== |
405 |
||
406 |
||
407 |
Test feature dependencies |
|
408 |
------------------------- |
|
409 |
||
410 |
Rather than manually checking the environment in each test, a test class |
|
411 |
can declare its dependence on some test features. The feature objects are |
|
412 |
checked only once for each run of the whole test suite. |
|
413 |
||
414 |
For historical reasons, as of May 2007 many cases that should depend on |
|
415 |
features currently raise TestSkipped.) |
|
416 |
||
417 |
:: |
|
2475.2.3
by Martin Pool
Merge ian's HACKING updates |
418 |
|
419 |
class TestStrace(TestCaseWithTransport): |
|
420 |
||
421 |
_test_needs_features = [StraceFeature] |
|
422 |
||
2729.1.1
by Martin Pool
Add TestNotApplicable exception and handling of it; document test parameterization |
423 |
This means all tests in this class need the feature. The feature itself |
2475.2.3
by Martin Pool
Merge ian's HACKING updates |
424 |
should provide a ``_probe`` method which is called once to determine if |
425 |
it's available. |
|
426 |
||
2729.1.1
by Martin Pool
Add TestNotApplicable exception and handling of it; document test parameterization |
427 |
These should generally be equivalent to either TestDependencyMissing or |
428 |
sometimes TestPlatformLimit. |
|
429 |
||
2475.2.3
by Martin Pool
Merge ian's HACKING updates |
430 |
|
431 |
Known failures |
|
432 |
-------------- |
|
433 |
||
434 |
Known failures are when a test exists but we know it currently doesn't |
|
435 |
work, allowing the test suite to still pass. These should be used with |
|
436 |
care, we don't want a proliferation of quietly broken tests. It might be |
|
437 |
appropriate to use them if you've committed a test for a bug but not the |
|
438 |
fix for it, or if something works on Unix but not on Windows. |
|
439 |
||
440 |
||
2513.1.9
by Martin Pool
Exception testing review comments |
441 |
Testing exceptions and errors |
442 |
----------------------------- |
|
2513.1.8
by Martin Pool
Doc testing of exceptions |
443 |
|
444 |
It's important to test handling of errors and exceptions. Because this |
|
445 |
code is often not hit in ad-hoc testing it can often have hidden bugs -- |
|
446 |
it's particularly common to get NameError because the exception code |
|
447 |
references a variable that has since been renamed. |
|
448 |
||
449 |
.. TODO: Something about how to provoke errors in the right way? |
|
450 |
||
451 |
In general we want to test errors at two levels: |
|
452 |
||
453 |
1. A test in ``test_errors.py`` checking that when the exception object is |
|
454 |
constructed with known parameters it produces an expected string form. |
|
455 |
This guards against mistakes in writing the format string, or in the |
|
456 |
``str`` representations of its parameters. There should be one for |
|
457 |
each exception class. |
|
458 |
||
459 |
2. Tests that when an api is called in a particular situation, it raises |
|
460 |
an error of the expected class. You should typically use |
|
461 |
``assertRaises``, which in the Bazaar test suite returns the exception |
|
462 |
object to allow you to examine its parameters. |
|
463 |
||
464 |
In some cases blackbox tests will also want to check error reporting. But |
|
465 |
it can be difficult to provoke every error through the commandline |
|
466 |
interface, so those tests are only done as needed -- eg in response to a |
|
2513.1.9
by Martin Pool
Exception testing review comments |
467 |
particular bug or if the error is reported in an unusual way(?) Blackbox |
468 |
tests should mostly be testing how the command-line interface works, so |
|
469 |
should only test errors if there is something particular to the cli in how |
|
470 |
they're displayed or handled. |
|
2513.1.8
by Martin Pool
Doc testing of exceptions |
471 |
|
2475.2.3
by Martin Pool
Merge ian's HACKING updates |
472 |
|
2729.1.1
by Martin Pool
Add TestNotApplicable exception and handling of it; document test parameterization |
473 |
Interface implementation testing and test scenarios |
474 |
--------------------------------------------------- |
|
475 |
||
476 |
There are several cases in Bazaar of multiple implementations of a common |
|
477 |
conceptual interface. ("Conceptual" because |
|
478 |
it's not necessary for all the implementations to share a base class, |
|
479 |
though they often do.) Examples include transports and the working tree, |
|
480 |
branch and repository classes. |
|
481 |
||
482 |
In these cases we want to make sure that every implementation correctly |
|
483 |
fulfils the interface requirements. For example, every Transport should |
|
484 |
support the ``has()`` and ``get()`` and ``clone()`` methods. We have a |
|
485 |
sub-suite of tests in ``test_transport_implementations``. (Most |
|
486 |
per-implementation tests are in submodules of ``bzrlib.tests``, but not |
|
487 |
the transport tests at the moment.) |
|
488 |
||
489 |
These tests are repeated for each registered Transport, by generating a |
|
490 |
new TestCase instance for the cross product of test methods and transport |
|
491 |
implementations. As each test runs, it has ``transport_class`` and |
|
492 |
``transport_server`` set to the class it should test. Most tests don't |
|
493 |
access these directly, but rather use ``self.get_transport`` which returns |
|
494 |
a transport of the appropriate type. |
|
495 |
||
496 |
The goal is to run per-implementation only tests that relate to that |
|
497 |
particular interface. Sometimes we discover a bug elsewhere that happens |
|
498 |
with only one particular transport. Once it's isolated, we can consider |
|
499 |
whether a test should be added for that particular implementation, |
|
500 |
or for all implementations of the interface. |
|
501 |
||
502 |
The multiplication of tests for different implementations is normally |
|
503 |
accomplished by overriding the ``test_suite`` function used to load |
|
504 |
tests from a module. This function typically loads all the tests, |
|
505 |
then applies a TestProviderAdapter to them, which generates a longer |
|
506 |
suite containing all the test variations. |
|
507 |
||
508 |
||
2729.1.2
by Martin Pool
Add new multiply_tests_from_modules to give a simpler interface to test scenarios |
509 |
Test scenarios |
510 |
-------------- |
|
511 |
||
512 |
Some utilities are provided for generating variations of tests. This can |
|
513 |
be used for per-implementation tests, or other cases where the same test |
|
514 |
code needs to run several times on different scenarios. |
|
515 |
||
516 |
The general approach is to define a class that provides test methods, |
|
517 |
which depend on attributes of the test object being pre-set with the |
|
518 |
values to which the test should be applied. The test suite should then |
|
519 |
also provide a list of scenarios in which to run the tests. |
|
520 |
||
521 |
Typically ``multiply_tests_from_modules`` should be called from the test |
|
522 |
module's ``test_suite`` function. |
|
523 |
||
524 |
||
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
525 |
Essential Domain Classes |
526 |
######################## |
|
527 |
||
528 |
Introducing the Object Model |
|
529 |
============================ |
|
530 |
||
531 |
The core domain objects within the bazaar model are: |
|
532 |
||
533 |
* Transport |
|
534 |
||
535 |
* Branch |
|
536 |
||
537 |
* Repository |
|
538 |
||
539 |
* WorkingTree |
|
540 |
||
541 |
Transports are explained below. See http://bazaar-vcs.org/Classes/ |
|
542 |
for an introduction to the other key classes. |
|
543 |
||
544 |
Using Transports |
|
545 |
================ |
|
546 |
||
547 |
The ``Transport`` layer handles access to local or remote directories. |
|
548 |
Each Transport object acts like a logical connection to a particular |
|
549 |
directory, and it allows various operations on files within it. You can |
|
550 |
*clone* a transport to get a new Transport connected to a subdirectory or |
|
551 |
parent directory. |
|
552 |
||
553 |
Transports are not used for access to the working tree. At present |
|
554 |
working trees are always local and they are accessed through the regular |
|
555 |
Python file io mechanisms. |
|
556 |
||
557 |
Filenames vs URLs |
|
558 |
----------------- |
|
559 |
||
560 |
Transports work in URLs. Take note that URLs are by definition only |
|
561 |
ASCII - the decision of how to encode a Unicode string into a URL must be |
|
562 |
taken at a higher level, typically in the Store. (Note that Stores also |
|
563 |
escape filenames which cannot be safely stored on all filesystems, but |
|
564 |
this is a different level.) |
|
565 |
||
566 |
The main reason for this is that it's not possible to safely roundtrip a |
|
567 |
URL into Unicode and then back into the same URL. The URL standard |
|
568 |
gives a way to represent non-ASCII bytes in ASCII (as %-escapes), but |
|
569 |
doesn't say how those bytes represent non-ASCII characters. (They're not |
|
570 |
guaranteed to be UTF-8 -- that is common but doesn't happen everywhere.) |
|
571 |
||
572 |
For example if the user enters the url ``http://example/%e0`` there's no |
|
573 |
way to tell whether that character represents "latin small letter a with |
|
574 |
grave" in iso-8859-1, or "latin small letter r with acute" in iso-8859-2 |
|
575 |
or malformed UTF-8. So we can't convert their URL to Unicode reliably. |
|
576 |
||
577 |
Equally problematic if we're given a url-like string containing non-ascii |
|
578 |
characters (such as the accented a) we can't be sure how to convert that |
|
579 |
to the correct URL, because we don't know what encoding the server expects |
|
580 |
for those characters. (Although this is not totally reliable we might still |
|
581 |
accept these and assume they should be put into UTF-8.) |
|
582 |
||
583 |
A similar edge case is that the url ``http://foo/sweet%2Fsour`` contains |
|
584 |
one directory component whose name is "sweet/sour". The escaped slash is |
|
585 |
not a directory separator. If we try to convert URLs to regular Unicode |
|
586 |
paths this information will be lost. |
|
587 |
||
588 |
This implies that Transports must natively deal with URLs; for simplicity |
|
589 |
they *only* deal with URLs and conversion of other strings to URLs is done |
|
590 |
elsewhere. Information they return, such as from ``list_dir``, is also in |
|
591 |
the form of URL components. |
|
592 |
||
593 |
||
594 |
Core Topics |
|
595 |
########### |
|
596 |
||
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
597 |
Evolving Interfaces |
598 |
=================== |
|
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
599 |
|
1534.2.4
by Robert Collins
Update NEWS and HACKING for the symbol_versioning module. |
600 |
We have a commitment to 6 months API stability - any supported symbol in a |
601 |
release of bzr MUST NOT be altered in any way that would result in |
|
602 |
breaking existing code that uses it. That means that method names, |
|
603 |
parameter ordering, parameter names, variable and attribute names etc must |
|
604 |
not be changed without leaving a 'deprecated forwarder' behind. This even |
|
605 |
applies to modules and classes. |
|
606 |
||
607 |
If you wish to change the behaviour of a supported API in an incompatible |
|
2063.3.1
by wang
fix typos |
608 |
way, you need to change its name as well. For instance, if I add an optional keyword |
1534.2.4
by Robert Collins
Update NEWS and HACKING for the symbol_versioning module. |
609 |
parameter to branch.commit - that's fine. On the other hand, if I add a |
610 |
keyword parameter to branch.commit which is a *required* transaction |
|
611 |
object, I should rename the API - i.e. to 'branch.commit_transaction'. |
|
612 |
||
613 |
When renaming such supported API's, be sure to leave a deprecated_method (or |
|
614 |
_function or ...) behind which forwards to the new API. See the |
|
615 |
bzrlib.symbol_versioning module for decorators that take care of the |
|
616 |
details for you - such as updating the docstring, and issuing a warning |
|
617 |
when the old api is used. |
|
618 |
||
2063.3.1
by wang
fix typos |
619 |
For unsupported API's, it does not hurt to follow this discipline, but it's |
1534.2.4
by Robert Collins
Update NEWS and HACKING for the symbol_versioning module. |
620 |
not required. Minimally though, please try to rename things so that |
621 |
callers will at least get an AttributeError rather than weird results. |
|
622 |
||
1393.1.54
by Martin Pool
- more hacking notes on evolving interfaces |
623 |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
624 |
Coding Style Guidelines |
625 |
======================= |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
626 |
|
627 |
Please write PEP-8__ compliant code. |
|
628 |
||
629 |
One often-missed requirement is that the first line of docstrings |
|
630 |
should be a self-contained one-sentence summary. |
|
631 |
||
632 |
__ http://www.python.org/peps/pep-0008.html |
|
633 |
||
634 |
||
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
635 |
Module Imports |
636 |
-------------- |
|
637 |
||
638 |
* Imports should be done at the top-level of the file, unless there is |
|
639 |
a strong reason to have them lazily loaded when a particular |
|
640 |
function runs. Import statements have a cost, so try to make sure |
|
641 |
they don't run inside hot functions. |
|
642 |
||
643 |
* Module names should always be given fully-qualified, |
|
644 |
i.e. ``bzrlib.hashcache`` not just ``hashcache``. |
|
645 |
||
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
646 |
|
647 |
Naming |
|
648 |
------ |
|
649 |
||
2625.3.1
by Ian Clatworthy
Clarify the use of underscore in the naming convention |
650 |
Functions, methods or members that are "private" to bzrlib are given |
651 |
a leading underscore prefix. Names without a leading underscore are |
|
652 |
public not just across modules but to programmers using bzrlib as an |
|
653 |
API. As a consequence, a leading underscore is appropriate for names |
|
654 |
exposed across modules but that are not to be exposed to bzrlib API |
|
655 |
programmers. |
|
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
656 |
|
657 |
We prefer class names to be concatenated capital words (``TestCase``) |
|
658 |
and variables, methods and functions to be lowercase words joined by |
|
659 |
underscores (``revision_id``, ``get_revision``). |
|
660 |
||
661 |
For the purposes of naming some names are treated as single compound |
|
662 |
words: "filename", "revno". |
|
663 |
||
664 |
Consider naming classes as nouns and functions/methods as verbs. |
|
665 |
||
2221.4.7
by Aaron Bentley
Add suggestion to HACKING |
666 |
Try to avoid using abbreviations in names, because there can be |
667 |
inconsistency if other people use the full name. |
|
668 |
||
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
669 |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
670 |
Standard Names |
1393.1.53
by Martin Pool
- notes from coding-convention discussion |
671 |
-------------- |
672 |
||
673 |
``revision_id`` not ``rev_id`` or ``revid`` |
|
674 |
||
675 |
Functions that transform one thing to another should be named ``x_to_y`` |
|
676 |
(not ``x2y`` as occurs in some old code.) |
|
677 |
||
1098
by Martin Pool
- notes on how output is written |
678 |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
679 |
Destructors |
680 |
----------- |
|
681 |
||
1185.16.150
by Martin Pool
Improved description of python exception policies |
682 |
Python destructors (``__del__``) work differently to those of other |
683 |
languages. In particular, bear in mind that destructors may be called |
|
684 |
immediately when the object apparently becomes unreferenced, or at some |
|
685 |
later time, or possibly never at all. Therefore we have restrictions on |
|
686 |
what can be done inside them. |
|
1185.16.85
by mbp at sourcefrog
- rules for using destructors |
687 |
|
688 |
0. Never use a __del__ method without asking Martin/Robert first. |
|
689 |
||
690 |
1. Never rely on a ``__del__`` method running. If there is code that |
|
691 |
must run, do it from a ``finally`` block instead. |
|
692 |
||
693 |
2. Never ``import`` from inside a ``__del__`` method, or you may crash the |
|
694 |
interpreter!! |
|
695 |
||
696 |
3. In some places we raise a warning from the destructor if the object |
|
697 |
has not been cleaned up or closed. This is considered OK: the warning |
|
698 |
may not catch every case but it's still useful sometimes. |
|
699 |
||
700 |
||
1740.2.5
by Aaron Bentley
Merge from bzr.dev |
701 |
Factories |
702 |
--------- |
|
703 |
||
704 |
In some places we have variables which point to callables that construct |
|
705 |
new instances. That is to say, they can be used a lot like class objects, |
|
706 |
but they shouldn't be *named* like classes: |
|
707 |
||
708 |
> I think that things named FooBar should create instances of FooBar when |
|
709 |
> called. Its plain confusing for them to do otherwise. When we have |
|
710 |
> something that is going to be used as a class - that is, checked for via |
|
711 |
> isinstance or other such idioms, them I would call it foo_class, so that |
|
712 |
> it is clear that a callable is not sufficient. If it is only used as a |
|
713 |
> factory, then yes, foo_factory is what I would use. |
|
714 |
||
715 |
||
1911.4.15
by John Arbash Meinel
Updated HACKING and docstrings per Martin's suggestions |
716 |
Registries |
717 |
---------- |
|
718 |
||
719 |
Several places in Bazaar use (or will use) a registry, which is a |
|
720 |
mapping from names to objects or classes. The registry allows for |
|
721 |
loading in registered code only when it's needed, and keeping |
|
722 |
associated information such as a help string or description. |
|
723 |
||
724 |
||
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
725 |
Lazy Imports |
726 |
------------ |
|
727 |
||
728 |
To make startup time faster, we use the ``bzrlib.lazy_import`` module to |
|
729 |
delay importing modules until they are actually used. ``lazy_import`` uses |
|
730 |
the same syntax as regular python imports. So to import a few modules in a |
|
731 |
lazy fashion do:: |
|
732 |
||
733 |
from bzrlib.lazy_import import lazy_import |
|
734 |
lazy_import(globals(), """ |
|
735 |
import os |
|
736 |
import subprocess |
|
737 |
import sys |
|
738 |
import time |
|
739 |
||
740 |
from bzrlib import ( |
|
741 |
errors, |
|
742 |
transport, |
|
1996.3.37
by John Arbash Meinel
Update HACKING and TODO |
743 |
revision as _mod_revision, |
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
744 |
) |
745 |
import bzrlib.transport |
|
746 |
import bzrlib.xml5 |
|
747 |
""") |
|
748 |
||
749 |
At this point, all of these exist as a ``ImportReplacer`` object, ready to |
|
1996.3.37
by John Arbash Meinel
Update HACKING and TODO |
750 |
be imported once a member is accessed. Also, when importing a module into |
751 |
the local namespace, which is likely to clash with variable names, it is |
|
2370.1.1
by Ian Clatworthy
Minor corrections to HACKING |
752 |
recommended to prefix it as ``_mod_<module>``. This makes it clearer that |
1996.3.37
by John Arbash Meinel
Update HACKING and TODO |
753 |
the variable is a module, and these object should be hidden anyway, since |
754 |
they shouldn't be imported into other namespaces. |
|
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
755 |
|
756 |
||
757 |
Modules versus Members |
|
758 |
~~~~~~~~~~~~~~~~~~~~~~ |
|
759 |
||
760 |
While it is possible for ``lazy_import()`` to import members of a module |
|
2063.3.1
by wang
fix typos |
761 |
when using the ``from module import member`` syntax, it is recommended to |
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
762 |
only use that syntax to load sub modules ``from module import submodule``. |
763 |
This is because variables and classes can frequently be used without |
|
764 |
needing a sub-member for example:: |
|
765 |
||
766 |
lazy_import(globals(), """ |
|
767 |
from module import MyClass |
|
768 |
""") |
|
769 |
||
770 |
def test(x): |
|
771 |
return isinstance(x, MyClass) |
|
772 |
||
773 |
This will incorrectly fail, because ``MyClass`` is a ``ImportReplacer`` |
|
774 |
object, rather than the real class. |
|
775 |
||
776 |
||
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
777 |
Passing to Other Variables |
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
778 |
~~~~~~~~~~~~~~~~~~~~~~~~~~ |
779 |
||
1996.1.26
by John Arbash Meinel
Update HACKING and docstrings |
780 |
It also is incorrect to assign ``ImportReplacer`` objects to other variables. |
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
781 |
Because the replacer only knows about the original name, it is unable to |
782 |
replace other variables. The ``ImportReplacer`` class will raise an |
|
1996.1.26
by John Arbash Meinel
Update HACKING and docstrings |
783 |
``IllegalUseOfScopeReplacer`` exception if it can figure out that this |
784 |
happened. But it requires accessing a member more than once from the new |
|
785 |
variable, so some bugs are not detected right away. |
|
1996.1.20
by John Arbash Meinel
HACKING and NEWS |
786 |
|
787 |
||
2598.5.9
by Aaron Bentley
Update NEWS and HACKING |
788 |
The Null revision |
789 |
----------------- |
|
790 |
||
791 |
The null revision is the ancestor of all revisions. Its revno is 0, its |
|
792 |
revision-id is ``null:``, and its tree is the empty tree. When referring |
|
793 |
to the null revision, please use ``bzrlib.revision.NULL_REVISION``. Old |
|
794 |
code sometimes uses ``None`` for the null revision, but this practice is |
|
795 |
being phased out. |
|
796 |
||
797 |
||
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
798 |
Getting Input |
799 |
============= |
|
800 |
||
801 |
Processing Command Lines |
|
802 |
------------------------ |
|
803 |
||
804 |
bzrlib has a standard framework for parsing command lines and calling |
|
805 |
processing routines associated with various commands. See builtins.py |
|
2466.6.2
by Ian Clatworthy
Incorporate feedback from LarstiQ |
806 |
for numerous examples. |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
807 |
|
808 |
||
809 |
Standard Parameter Types |
|
810 |
------------------------ |
|
811 |
||
812 |
There are some common requirements in the library: some parameters need to be |
|
813 |
unicode safe, some need byte strings, and so on. At the moment we have |
|
814 |
only codified one specific pattern: Parameters that need to be unicode |
|
815 |
should be checked via ``bzrlib.osutils.safe_unicode``. This will coerce the |
|
816 |
input into unicode in a consistent fashion, allowing trivial strings to be |
|
817 |
used for programmer convenience, but not performing unpredictably in the |
|
818 |
presence of different locales. |
|
819 |
||
820 |
||
821 |
Writing Output |
|
1098
by Martin Pool
- notes on how output is written |
822 |
============== |
823 |
||
824 |
(The strategy described here is what we want to get to, but it's not |
|
825 |
consistently followed in the code at the moment.) |
|
826 |
||
827 |
bzrlib is intended to be a generically reusable library. It shouldn't |
|
828 |
write messages to stdout or stderr, because some programs that use it |
|
829 |
might want to display that information through a GUI or some other |
|
830 |
mechanism. |
|
831 |
||
832 |
We can distinguish two types of output from the library: |
|
833 |
||
834 |
1. Structured data representing the progress or result of an |
|
835 |
operation. For example, for a commit command this will be a list |
|
836 |
of the modified files and the finally committed revision number |
|
837 |
and id. |
|
838 |
||
839 |
These should be exposed either through the return code or by calls |
|
840 |
to a callback parameter. |
|
841 |
||
842 |
A special case of this is progress indicators for long-lived |
|
843 |
operations, where the caller should pass a ProgressBar object. |
|
844 |
||
845 |
2. Unstructured log/debug messages, mostly for the benefit of the |
|
846 |
developers or users trying to debug problems. This should always |
|
847 |
be sent through ``bzrlib.trace`` and Python ``logging``, so that |
|
848 |
it can be redirected by the client. |
|
849 |
||
850 |
The distinction between the two is a bit subjective, but in general if |
|
851 |
there is any chance that a library would want to see something as |
|
852 |
structured data, we should make it so. |
|
853 |
||
854 |
The policy about how output is presented in the text-mode client |
|
855 |
should be only in the command-line tool. |
|
1092.1.22
by Robert Collins
update hacking with some test foo |
856 |
|
1418
by Robert Collins
merge martins latest |
857 |
|
2598.1.1
by Martin Pool
Add test for and documentation of option style, fix up existing options to comply |
858 |
|
859 |
Displaying help |
|
860 |
=============== |
|
861 |
||
862 |
Bazaar has online help for various topics through ``bzr help COMMAND`` or |
|
863 |
equivalently ``bzr command -h``. We also have help on command options, |
|
864 |
and on other help topics. (See ``help_topics.py``.) |
|
865 |
||
866 |
As for python docstrings, the first paragraph should be a single-sentence |
|
867 |
synopsis of the command. |
|
868 |
||
869 |
The help for options should be one or more proper sentences, starting with |
|
870 |
a capital letter and finishing with a full stop (period). |
|
871 |
||
872 |
All help messages and documentation should have two spaces between |
|
873 |
sentences. |
|
874 |
||
875 |
||
1092.1.22
by Robert Collins
update hacking with some test foo |
876 |
Writing tests |
877 |
============= |
|
2067.2.2
by John Arbash Meinel
Review comments from Robert |
878 |
|
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
879 |
In general tests should be placed in a file named test_FOO.py where |
1092.1.22
by Robert Collins
update hacking with some test foo |
880 |
FOO is the logical thing under test. That file should be placed in the |
881 |
tests subdirectory under the package being tested. |
|
882 |
||
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
883 |
For example, tests for merge3 in bzrlib belong in bzrlib/tests/test_merge3.py. |
2370.1.1
by Ian Clatworthy
Minor corrections to HACKING |
884 |
See bzrlib/tests/test_sampler.py for a template test script. |
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
885 |
|
886 |
Tests can be written for the UI or for individual areas of the library. |
|
887 |
Choose whichever is appropriate: if adding a new command, or a new command |
|
888 |
option, then you should be writing a UI test. If you are both adding UI |
|
889 |
functionality and library functionality, you will want to write tests for |
|
890 |
both the UI and the core behaviours. We call UI tests 'blackbox' tests |
|
1711.2.94
by John Arbash Meinel
Update HACKING to be rst compliant |
891 |
and they are found in ``bzrlib/tests/blackbox/*.py``. |
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
892 |
|
893 |
When writing blackbox tests please honour the following conventions: |
|
894 |
||
895 |
1. Place the tests for the command 'name' in |
|
896 |
bzrlib/tests/blackbox/test_name.py. This makes it easy for developers |
|
897 |
to locate the test script for a faulty command. |
|
898 |
||
899 |
2. Use the 'self.run_bzr("name")' utility function to invoke the command |
|
900 |
rather than running bzr in a subprocess or invoking the |
|
901 |
cmd_object.run() method directly. This is a lot faster than |
|
902 |
subprocesses and generates the same logging output as running it in a |
|
903 |
subprocess (which invoking the method directly does not). |
|
904 |
||
905 |
3. Only test the one command in a single test script. Use the bzrlib |
|
906 |
library when setting up tests and when evaluating the side-effects of |
|
907 |
the command. We do this so that the library api has continual pressure |
|
908 |
on it to be as functional as the command line in a simple manner, and |
|
909 |
to isolate knock-on effects throughout the blackbox test suite when a |
|
2063.3.1
by wang
fix typos |
910 |
command changes its name or signature. Ideally only the tests for a |
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
911 |
given command are affected when a given command is changed. |
1393.1.61
by Martin Pool
doc |
912 |
|
2067.2.2
by John Arbash Meinel
Review comments from Robert |
913 |
4. If you have a test which does actually require running bzr in a |
914 |
subprocess you can use ``run_bzr_subprocess``. By default the spawned |
|
915 |
process will not load plugins unless ``--allow-plugins`` is supplied. |
|
916 |
||
917 |
||
2466.7.2
by Robert Collins
Document the user of TreeBuilder somewhat. |
918 |
Test support |
919 |
------------ |
|
920 |
||
921 |
We have a rich collection of tools to support writing tests. Please use |
|
922 |
them in preference to ad-hoc solutions as they provide portability and |
|
923 |
performance benefits. |
|
924 |
||
925 |
TreeBuilder |
|
926 |
~~~~~~~~~~~ |
|
927 |
||
928 |
The ``TreeBuilder`` interface allows the construction of arbitrary trees |
|
929 |
with a declarative interface. A sample session might look like:: |
|
930 |
||
931 |
tree = self.make_branch_and_tree('path') |
|
932 |
builder = TreeBuilder() |
|
933 |
builder.start_tree(tree) |
|
934 |
builder.build(['foo', "bar/", "bar/file"]) |
|
935 |
tree.commit('commit the tree') |
|
936 |
builder.finish_tree() |
|
937 |
||
938 |
Please see bzrlib.treebuilder for more details. |
|
939 |
||
2466.7.7
by Robert Collins
Document basic usage. |
940 |
BranchBuilder |
941 |
~~~~~~~~~~~~~ |
|
942 |
||
943 |
The ``BranchBuilder`` interface allows the creation of test branches in a |
|
944 |
quick and easy manner. A sample session:: |
|
945 |
||
946 |
builder = BranchBuilder(self.get_transport().clone('relpath')) |
|
947 |
builder.build_commit() |
|
948 |
builder.build_commit() |
|
949 |
builder.build_commit() |
|
950 |
branch = builder.get_branch() |
|
951 |
||
952 |
Please see bzrlib.branchbuilder for more details. |
|
2466.7.2
by Robert Collins
Document the user of TreeBuilder somewhat. |
953 |
|
1740.6.1
by Martin Pool
Remove Scratch objects used by doctests |
954 |
Doctests |
955 |
-------- |
|
956 |
||
957 |
We make selective use of doctests__. In general they should provide |
|
958 |
*examples* within the API documentation which can incidentally be tested. We |
|
959 |
don't try to test every important case using doctests -- regular Python |
|
960 |
tests are generally a better solution. |
|
961 |
||
962 |
Most of these are in ``bzrlib/doc/api``. More additions are welcome. |
|
963 |
||
964 |
__ http://docs.python.org/lib/module-doctest.html |
|
965 |
||
966 |
||
1092.1.22
by Robert Collins
update hacking with some test foo |
967 |
Running tests |
968 |
============= |
|
969 |
Currently, bzr selftest is used to invoke tests. |
|
970 |
You can provide a pattern argument to run a subset. For example, |
|
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
971 |
to run just the blackbox tests, run:: |
1393.1.61
by Martin Pool
doc |
972 |
|
1638.1.1
by Robert Collins
Update HACKING to reflect current test writing policy. |
973 |
./bzr selftest -v blackbox |
1393.1.61
by Martin Pool
doc |
974 |
|
2394.2.6
by Ian Clatworthy
completed blackbox tests |
975 |
To skip a particular test (or set of tests), use the --exclude option |
976 |
(shorthand -x) like so:: |
|
977 |
||
978 |
./bzr selftest -v -x blackbox |
|
979 |
||
980 |
To list tests without running them, use the --list-only option like so:: |
|
981 |
||
982 |
./bzr selftest --list-only |
|
983 |
||
984 |
This option can be combined with other selftest options (like -x) and |
|
985 |
filter patterns to understand their effect. |
|
1551.6.41
by Aaron Bentley
Add advice on skipping tests to HACKING |
986 |
|
1393.1.61
by Martin Pool
doc |
987 |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
988 |
Handling Errors and Exceptions |
989 |
============================== |
|
990 |
||
991 |
Commands should return non-zero when they encounter circumstances that |
|
992 |
the user should really pay attention to - which includes trivial shell |
|
993 |
pipelines. |
|
994 |
||
995 |
Recommended values are: |
|
996 |
||
997 |
0. OK. |
|
998 |
1. Conflicts in merge-like operations, or changes are present in |
|
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
999 |
diff-like operations. |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1000 |
2. Unrepresentable diff changes (i.e. binary files that we cannot show |
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
1001 |
a diff of). |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1002 |
3. An error or exception has occurred. |
1003 |
||
1004 |
Errors are handled through Python exceptions. Exceptions should be defined |
|
1005 |
inside bzrlib.errors, so that we can see the whole tree at a glance. |
|
1006 |
||
1007 |
We broadly classify errors as either being either internal or not, |
|
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
1008 |
depending on whether ``internal_error`` is set or not. If we think it's our |
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1009 |
fault, we show a backtrace, an invitation to report the bug, and possibly |
1010 |
other details. This is the default for errors that aren't specifically |
|
1011 |
recognized as being caused by a user error. Otherwise we show a briefer |
|
1012 |
message, unless -Derror was given. |
|
1013 |
||
1014 |
Many errors originate as "environmental errors" which are raised by Python |
|
1015 |
or builtin libraries -- for example IOError. These are treated as being |
|
1016 |
our fault, unless they're caught in a particular tight scope where we know |
|
1017 |
that they indicate a user errors. For example if the repository format |
|
1018 |
is not found, the user probably gave the wrong path or URL. But if one of |
|
1019 |
the files inside the repository is not found, then it's our fault -- |
|
1020 |
either there's a bug in bzr, or something complicated has gone wrong in |
|
1021 |
the environment that means one internal file was deleted. |
|
1022 |
||
1023 |
Many errors are defined in ``bzrlib/errors.py`` but it's OK for new errors |
|
1024 |
to be added near the place where they are used. |
|
1025 |
||
1026 |
Exceptions are formatted for the user by conversion to a string |
|
1027 |
(eventually calling their ``__str__`` method.) As a convenience the |
|
1028 |
``._fmt`` member can be used as a template which will be mapped to the |
|
1029 |
error's instance dict. |
|
1030 |
||
1031 |
New exception classes should be defined when callers might want to catch |
|
1032 |
that exception specifically, or when it needs a substantially different |
|
1033 |
format string. |
|
1034 |
||
1035 |
Exception strings should start with a capital letter and should not have a |
|
1036 |
final fullstop. If long, they may contain newlines to break the text. |
|
1037 |
||
1038 |
||
1039 |
Documenting Changes |
|
1040 |
=================== |
|
1041 |
||
1042 |
When you change bzrlib, please update the relevant documentation for the |
|
1043 |
change you made: Changes to commands should update their help, and |
|
1044 |
possibly end user tutorials; changes to the core library should be |
|
1045 |
reflected in API documentation. |
|
1046 |
||
1047 |
NEWS File |
|
1048 |
--------- |
|
1049 |
||
1050 |
If you make a user-visible change, please add a note to the NEWS file. |
|
1051 |
The description should be written to make sense to someone who's just |
|
1052 |
a user of bzr, not a developer: new functions or classes shouldn't be |
|
1053 |
mentioned, but new commands, changes in behaviour or fixed nontrivial |
|
1054 |
bugs should be listed. See the existing entries for an idea of what |
|
1055 |
should be done. |
|
1056 |
||
1057 |
Within each release, entries in the news file should have the most |
|
1058 |
user-visible changes first. So the order should be approximately: |
|
1059 |
||
1060 |
* changes to existing behaviour - the highest priority because the |
|
1061 |
user's existing knowledge is incorrect |
|
1062 |
* new features - should be brought to their attention |
|
1063 |
* bug fixes - may be of interest if the bug was affecting them, and |
|
1064 |
should include the bug number if any |
|
1065 |
* major documentation changes |
|
1066 |
* changes to internal interfaces |
|
1067 |
||
1068 |
People who made significant contributions to each change are listed in |
|
1069 |
parenthesis. This can include reporting bugs (particularly with good |
|
1070 |
details or reproduction recipes), submitting patches, etc. |
|
1071 |
||
1072 |
Commands |
|
1073 |
-------- |
|
1074 |
||
1075 |
The docstring of a command is used by ``bzr help`` to generate help output |
|
1076 |
for the command. The list 'takes_options' attribute on a command is used by |
|
1077 |
``bzr help`` to document the options for the command - the command |
|
1078 |
docstring does not need to document them. Finally, the '_see_also' |
|
1079 |
attribute on a command can be used to reference other related help topics. |
|
1080 |
||
1081 |
API Documentation |
|
1082 |
----------------- |
|
1083 |
||
1084 |
Functions, methods, classes and modules should have docstrings |
|
1085 |
describing how they are used. |
|
1086 |
||
1087 |
The first line of the docstring should be a self-contained sentence. |
|
1088 |
||
1089 |
For the special case of Command classes, this acts as the user-visible |
|
1090 |
documentation shown by the help command. |
|
1091 |
||
1092 |
The docstrings should be formatted as reStructuredText_ (like this |
|
1093 |
document), suitable for processing using the epydoc_ tool into HTML |
|
1094 |
documentation. |
|
1095 |
||
1096 |
.. _reStructuredText: http://docutils.sourceforge.net/rst.html |
|
1097 |
.. _epydoc: http://epydoc.sourceforge.net/ |
|
1098 |
||
1099 |
||
1100 |
General Guidelines |
|
1101 |
================== |
|
1102 |
||
1103 |
Copyright |
|
1104 |
--------- |
|
1105 |
||
1106 |
The copyright policy for bzr was recently made clear in this email (edited |
|
1107 |
for grammatical correctness):: |
|
1108 |
||
1109 |
The attached patch cleans up the copyright and license statements in |
|
1110 |
the bzr source. It also adds tests to help us remember to add them |
|
1111 |
with the correct text. |
|
1112 |
||
1113 |
We had the problem that lots of our files were "Copyright Canonical |
|
1114 |
Development Ltd" which is not a real company, and some other variations |
|
1115 |
on this theme. Also, some files were missing the GPL statements. |
|
1116 |
||
1117 |
I want to be clear about the intent of this patch, since copyright can |
|
1118 |
be a little controversial. |
|
1119 |
||
1120 |
1) The big motivation for this is not to shut out the community, but |
|
1121 |
just to clean up all of the invalid copyright statements. |
|
1122 |
||
1123 |
2) It has been the general policy for bzr that we want a single |
|
1124 |
copyright holder for all of the core code. This is following the model |
|
1125 |
set by the FSF, which makes it easier to update the code to a new |
|
1126 |
license in case problems are encountered. (For example, if we want to |
|
1127 |
upgrade the project universally to GPL v3 it is much simpler if there is |
|
1128 |
a single copyright holder). It also makes it clearer if copyright is |
|
1129 |
ever debated, there is a single holder, which makes it easier to defend |
|
1130 |
in court, etc. (I think the FSF position is that if you assign them |
|
1131 |
copyright, they can defend it in court rather than you needing to, and |
|
1132 |
I'm sure Canonical would do the same). |
|
1133 |
As such, Canonical has requested copyright assignments from all of the |
|
1134 |
major contributers. |
|
1135 |
||
1136 |
3) If someone wants to add code and not attribute it to Canonical, there |
|
1137 |
is a specific list of files that are excluded from this check. And the |
|
1138 |
test failure indicates where that is, and how to update it. |
|
1139 |
||
1140 |
4) If anyone feels that I changed a copyright statement incorrectly, just |
|
1141 |
let me know, and I'll be happy to correct it. Whenever you have large |
|
1142 |
mechanical changes like this, it is possible to make some mistakes. |
|
1143 |
||
1144 |
Just to reiterate, this is a community project, and it is meant to stay |
|
1145 |
that way. Core bzr code is copyright Canonical for legal reasons, and |
|
1146 |
the tests are just there to help us maintain that. |
|
1147 |
||
1148 |
||
1149 |
Miscellaneous Topics |
|
1150 |
#################### |
|
1151 |
||
1152 |
Debugging |
|
1153 |
========= |
|
1154 |
||
1155 |
Bazaar has a few facilities to help debug problems by going into pdb_, the |
|
1156 |
Python debugger. |
|
1157 |
||
1158 |
.. _pdb: http://docs.python.org/lib/debugger-commands.html |
|
1159 |
||
1160 |
If the ``BZR_PDB`` environment variable is set |
|
1161 |
then bzr will go into pdb post-mortem mode when an unhandled exception |
|
1162 |
occurs. |
|
1163 |
||
2466.6.3
by Ian Clatworthy
Incorporate feedback from Aaron B. & Alex B. |
1164 |
If you send a SIGQUIT signal to bzr, which can be done by pressing |
1165 |
Ctrl-\\ on Unix, bzr will go into the debugger immediately. You can |
|
1166 |
continue execution by typing ``c``. This can be disabled if necessary |
|
1167 |
by setting the environment variable ``BZR_SIGQUIT_PDB=0``. |
|
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1168 |
|
1169 |
||
1170 |
Jargon |
|
1171 |
====== |
|
1172 |
||
1173 |
revno |
|
1174 |
Integer identifier for a revision on the main line of a branch. |
|
1175 |
Revision 0 is always the null revision; others are 1-based |
|
1176 |
indexes into the branch's revision history. |
|
1177 |
||
1178 |
||
1711.2.95
by John Arbash Meinel
Add HACKING note for the self.outf parameter. |
1179 |
Unicode and Encoding Support |
1180 |
============================ |
|
1181 |
||
1182 |
This section discusses various techniques that Bazaar uses to handle |
|
1183 |
characters that are outside the ASCII set. |
|
1184 |
||
1185 |
``Command.outf`` |
|
1186 |
---------------- |
|
1187 |
||
1188 |
When a ``Command`` object is created, it is given a member variable |
|
1189 |
accessible by ``self.outf``. This is a file-like object, which is bound to |
|
1190 |
``sys.stdout``, and should be used to write information to the screen, |
|
1191 |
rather than directly writing to ``sys.stdout`` or calling ``print``. |
|
1192 |
This file has the ability to translate Unicode objects into the correct |
|
1711.2.96
by John Arbash Meinel
cleanup from suggestions by Robert and Martin |
1193 |
representation, based on the console encoding. Also, the class attribute |
1194 |
``encoding_type`` will effect how unprintable characters will be |
|
1711.2.95
by John Arbash Meinel
Add HACKING note for the self.outf parameter. |
1195 |
handled. This parameter can take one of 3 values: |
1196 |
||
1197 |
replace |
|
1711.2.96
by John Arbash Meinel
cleanup from suggestions by Robert and Martin |
1198 |
Unprintable characters will be represented with a suitable replacement |
1199 |
marker (typically '?'), and no exception will be raised. This is for |
|
1200 |
any command which generates text for the user to review, rather than |
|
1201 |
for automated processing. |
|
1711.2.95
by John Arbash Meinel
Add HACKING note for the self.outf parameter. |
1202 |
For example: ``bzr log`` should not fail if one of the entries has text |
1203 |
that cannot be displayed. |
|
1204 |
||
1205 |
strict |
|
2063.3.1
by wang
fix typos |
1206 |
Attempting to print an unprintable character will cause a UnicodeError. |
1711.2.95
by John Arbash Meinel
Add HACKING note for the self.outf parameter. |
1207 |
This is for commands that are intended more as scripting support, rather |
1208 |
than plain user review. |
|
1209 |
For exampl: ``bzr ls`` is designed to be used with shell scripting. One |
|
1210 |
use would be ``bzr ls --null --unknows | xargs -0 rm``. If ``bzr`` |
|
1211 |
printed a filename with a '?', the wrong file could be deleted. (At the |
|
1212 |
very least, the correct file would not be deleted). An error is used to |
|
1213 |
indicate that the requested action could not be performed. |
|
1214 |
||
1215 |
exact |
|
1216 |
Do not attempt to automatically convert Unicode strings. This is used |
|
1217 |
for commands that must handle conversion themselves. |
|
1218 |
For example: ``bzr diff`` needs to translate Unicode paths, but should |
|
1219 |
not change the exact text of the contents of the files. |
|
1220 |
||
1221 |
||
1222 |
``bzrlib.urlutils.unescape_for_display`` |
|
1223 |
---------------------------------------- |
|
1224 |
||
1225 |
Because Transports work in URLs (as defined earlier), printing the raw URL |
|
1226 |
to the user is usually less than optimal. Characters outside the standard |
|
1227 |
set are printed as escapes, rather than the real character, and local |
|
1228 |
paths would be printed as ``file://`` urls. The function |
|
1229 |
``unescape_for_display`` attempts to unescape a URL, such that anything |
|
1230 |
that cannot be printed in the current encoding stays an escaped URL, but |
|
1231 |
valid characters are generated where possible. |
|
1232 |
||
1233 |
||
2405.2.2
by Andrew Bennetts
Add a brief section on portability to HACKING. |
1234 |
Portability Tips |
1235 |
================ |
|
1236 |
||
1237 |
The ``bzrlib.osutils`` module has many useful helper functions, including |
|
1238 |
some more portable variants of functions in the standard library. |
|
1239 |
||
1240 |
In particular, don't use ``shutil.rmtree`` unless it's acceptable for it |
|
1241 |
to fail on Windows if some files are readonly or still open elsewhere. |
|
1242 |
Use ``bzrlib.osutils.rmtree`` instead. |
|
1243 |
||
1244 |
||
1739.1.2
by Robert Collins
More pyrex finesse, documentation. |
1245 |
C Extension Modules |
1246 |
=================== |
|
1247 |
||
1248 |
We write some extensions in C using pyrex. We design these to work in |
|
1249 |
three scenarios: |
|
2449.1.1
by Alexander Belchenko
fix RSTX wrong formatting in HACKING |
1250 |
|
1739.1.2
by Robert Collins
More pyrex finesse, documentation. |
1251 |
* User with no C compiler |
1252 |
* User with C compiler |
|
1253 |
* Developers |
|
1254 |
||
1255 |
The recommended way to install bzr is to have a C compiler so that the |
|
1256 |
extensions can be built, but if no C compiler is present, the pure python |
|
1257 |
versions we supply will work, though more slowly. |
|
1258 |
||
1259 |
For developers we recommend that pyrex be installed, so that the C |
|
1260 |
extensions can be changed if needed. |
|
1261 |
||
1262 |
For the C extensions, the extension module should always match the |
|
1263 |
original python one in all respects (modulo speed). This should be |
|
1264 |
maintained over time. |
|
1265 |
||
1266 |
To create an extension, add rules to setup.py for building it with pyrex, |
|
1267 |
and with distutils. Now start with an empty .pyx file. At the top add |
|
1268 |
"include 'yourmodule.py'". This will import the contents of foo.py into this |
|
1269 |
file at build time - remember that only one module will be loaded at |
|
1270 |
runtime. Now you can subclass classes, or replace functions, and only your |
|
1271 |
changes need to be present in the .pyx file. |
|
1272 |
||
1273 |
Note that pyrex does not support all 2.4 programming idioms, so some |
|
1274 |
syntax changes may be required. I.e. |
|
2449.1.1
by Alexander Belchenko
fix RSTX wrong formatting in HACKING |
1275 |
|
1739.1.2
by Robert Collins
More pyrex finesse, documentation. |
1276 |
- 'from foo import (bar, gam)' needs to change to not use the brackets. |
1277 |
- 'import foo.bar as bar' needs to be 'import foo.bar; bar = foo.bar' |
|
2449.1.1
by Alexander Belchenko
fix RSTX wrong formatting in HACKING |
1278 |
|
1739.1.2
by Robert Collins
More pyrex finesse, documentation. |
1279 |
If the changes are too dramatic, consider |
1280 |
maintaining the python code twice - once in the .pyx, and once in the .py, |
|
1281 |
and no longer including the .py file. |
|
1282 |
||
2466.6.1
by Ian Clatworthy
Expand HACKING into Bazaar Developer Guide |
1283 |
|
1284 |
Making Installers for OS Windows |
|
1861.2.19
by Alexander Belchenko
HACKING: mention where to get instructions for building windows installers |
1285 |
================================ |
1861.2.20
by Alexander Belchenko
English |
1286 |
To build a win32 installer, see the instructions on the wiki page: |
1861.2.19
by Alexander Belchenko
HACKING: mention where to get instructions for building windows installers |
1287 |
http://bazaar-vcs.org/BzrWin32Installer |
1288 |
||
1289 |
||
2475.2.4
by Martin Pool
HACKING rest fixes from jam |
1290 |
.. |
1291 |
vim: ft=rst tw=74 ai |