Merge lp:~vila/bzr/786980-url-aliases into lp:bzr

Proposed by Vincent Ladeuil on 2011-07-15
Status: Merged
Approved by: John A Meinel on 2011-07-15
Approved revision: 5658
Merged at revision: 6028
Proposed branch: lp:~vila/bzr/786980-url-aliases
Merge into: lp:bzr
Diff against target: 65 lines (+40/-15)
1 file modified
bzrlib/tests/test_remote.py (+40/-15)
To merge this branch: bzr merge lp:~vila/bzr/786980-url-aliases
Reviewer Review Type Date Requested Status
John A Meinel 2011-07-15 Approve on 2011-07-15
Review via email: mp+68069@code.launchpad.net

Commit message

Add more tests for bound_location variations in config files.

Description of the change

This add tests for accepted bound_location variations that are
still pointing to the same branch.

I tried adding variations that led to a different branch to
exercise the fix some more only to realized that this is already
properly handled in higher levels.

This means that the code modified for this fix can be reached
ONLY if the bound_location differs from source_url but is still
valid.

So the added tests should be enough to satisfy the review
comments in
https://code.launchpad.net/~vila/bzr/786980-url-aliases/+merge/67351

To post a comment you must log in.
John A Meinel (jameinel) wrote :

Good enough for now.

review: Approve
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/tests/test_remote.py'
2--- bzrlib/tests/test_remote.py 2011-07-15 08:25:00 +0000
3+++ bzrlib/tests/test_remote.py 2011-07-15 10:42:48 +0000
4@@ -3408,21 +3408,46 @@
5 self.assertTrue(len(self.hpss_calls) > 1)
6
7
8-class TestUpdateBoundBranch(tests.TestCaseWithTransport):
9-
10- def test_bug_786980(self):
11+class TestUpdateBoundBranchWithModifiedBoundLocation(
12+ tests.TestCaseWithTransport):
13+ """Ensure correct handling of bound_location modifications.
14+
15+ This is tested against a smart server as http://pad.lv/786980 was about a
16+ ReadOnlyError (write attempt during a read-only transaction) which can only
17+ happen in this context.
18+ """
19+
20+ def setUp(self):
21+ super(TestUpdateBoundBranchWithModifiedBoundLocation, self).setUp()
22 self.transport_server = test_server.SmartTCPServer_for_testing
23- wt = self.make_branch_and_tree('master')
24- checkout = wt.branch.create_checkout('checkout')
25- wt.commit('add stuff')
26- last_revid = wt.commit('even more stuff')
27- bound_location = checkout.branch.get_bound_location()
28+
29+ def make_master_and_checkout(self, master_name, checkout_name):
30+ # Create the master branch and its associated checkout
31+ self.master = self.make_branch_and_tree(master_name)
32+ self.checkout = self.master.branch.create_checkout(checkout_name)
33+ # Modify the master branch so there is something to update
34+ self.master.commit('add stuff')
35+ self.last_revid = self.master.commit('even more stuff')
36+ self.bound_location = self.checkout.branch.get_bound_location()
37+
38+ def assertUpdateSucceeds(self, new_location):
39+ self.checkout.branch.set_bound_location(new_location)
40+ self.checkout.update()
41+ self.assertEquals(self.last_revid, self.checkout.last_revision())
42+
43+ def test_without_final_slash(self):
44+ self.make_master_and_checkout('master', 'checkout')
45 # For unclear reasons some users have a bound_location without a final
46 # '/', simulate that by forcing such a value
47- self.assertEndsWith(bound_location, '/')
48- new_location = bound_location.rstrip('/')
49- checkout.branch.set_bound_location(new_location)
50- # bug 786980 was raising ReadOnlyError: A write attempt was made in a
51- # read only transaction during the update()
52- checkout.update()
53- self.assertEquals(last_revid, checkout.last_revision())
54+ self.assertEndsWith(self.bound_location, '/')
55+ self.assertUpdateSucceeds(self.bound_location.rstrip('/'))
56+
57+ def test_plus_sign(self):
58+ self.make_master_and_checkout('+master', 'checkout')
59+ self.assertUpdateSucceeds(self.bound_location.replace('%2B', '+', 1))
60+
61+ def test_tilda(self):
62+ # Embed ~ in the middle of the path just to avoid any $HOME
63+ # interpretation
64+ self.make_master_and_checkout('mas~ter', 'checkout')
65+ self.assertUpdateSucceeds(self.bound_location.replace('%2E', '~', 1))