From e114a628dcaf59e2e4649dff10916b78fead4ba9 Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Tue, 1 Jul 2014 11:23:48 -0700 Subject: [PATCH] Remove fake _get_part and use the real thing The get_part method is fast and stable given a consistent hash path suffix/prefix, so there's no absolute requirement for the fake implementation other than convenience. OTOH, removing the fake implementation and fixing the tests that were relying on it should make it easier to write better tests going forward and harder to hide bugs that don't show up when using the fakes. There may be some overhead when writing new tests that use the ring if you're making assertions on partitions or paths, but with a part power of zero it's normally trivially obvious when a 1 needs to be a 0 or vice versa. Or you can just drop the assertions around the parts you were faking anyway. Change-Id: I8bfc388a04eff6491038991cdfd7686c9d961545 --- test/unit/__init__.py | 7 --- test/unit/account/test_reaper.py | 2 +- test/unit/proxy/test_server.py | 88 ++++++++++++++++---------------- 3 files changed, 46 insertions(+), 51 deletions(-) diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 019b8ffef2..fe8538ebc7 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -139,13 +139,6 @@ class FakeRing(Ring): self._part_shift = 32 - part_power self._reload() - def get_part(self, *args, **kwargs): - # always call the real method, even if the fake ignores the result - real_part = super(FakeRing, self).get_part(*args, **kwargs) - if self._part_shift == 32: - return 1 - return real_part - def _reload(self): self._rtime = time.time() diff --git a/test/unit/account/test_reaper.py b/test/unit/account/test_reaper.py index f929aafe2b..deec2e3301 100644 --- a/test/unit/account/test_reaper.py +++ b/test/unit/account/test_reaper.py @@ -294,7 +294,7 @@ class TestReaper(unittest.TestCase): 'X-Backend-Storage-Policy-Index': policy.idx } ring = r.get_object_ring(policy.idx) - expected = call(ring.devs[i], 1, 'a', 'c', 'o', + expected = call(ring.devs[i], 0, 'a', 'c', 'o', headers=headers, conn_timeout=0.5, response_timeout=10) self.assertEqual(call_args, expected) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 581b940208..149c5b9e7a 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -1728,15 +1728,17 @@ class TestObjectController(unittest.TestCase): req_method, req_path, req_headers = req self.assertEqual(method, req_method) # caller can ignore leading path parts - self.assertTrue(req_path.endswith(path)) + self.assertTrue(req_path.endswith(path), + 'expected path to end with %s, it was %s' % ( + path, req_path)) headers = headers or {} # caller can ignore some headers for k, v in headers.items(): self.assertEqual(req_headers[k], v) account_request = backend_requests.pop(0) - check_request(account_request, method='HEAD', path='/sda/1/a') + check_request(account_request, method='HEAD', path='/sda/0/a') container_request = backend_requests.pop(0) - check_request(container_request, method='HEAD', path='/sda/1/a/c') + check_request(container_request, method='HEAD', path='/sda/0/a/c') # make sure backend requests included expected container headers container_headers = {} for request in backend_requests: @@ -1746,9 +1748,9 @@ class TestObjectController(unittest.TestCase): container_headers[device] = host expectations = { 'method': 'POST', - 'path': '/1/a/c/o', + 'path': '/0/a/c/o', 'headers': { - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'Connection': 'close', 'User-Agent': 'proxy-server %s' % os.getpid(), 'Host': 'localhost:80', @@ -1781,7 +1783,7 @@ class TestObjectController(unittest.TestCase): for request in backend_requests[2:]: expectations = { 'method': 'POST', - 'path': '/1/a/c/o', # ignore device bit + 'path': '/0/a/c/o', # ignore device bit 'headers': { 'X-Object-Meta-Color': 'Blue', 'X-Backend-Storage-Policy-Index': '0', @@ -1808,17 +1810,17 @@ class TestObjectController(unittest.TestCase): policy1 = {'X-Backend-Storage-Policy-Index': '1'} expected = [ # account info - {'method': 'HEAD', 'path': '/1/a'}, + {'method': 'HEAD', 'path': '/0/a'}, # container info - {'method': 'HEAD', 'path': '/1/a/c'}, + {'method': 'HEAD', 'path': '/0/a/c'}, # x-newests - {'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1}, - {'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1}, - {'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1}, + {'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1}, + {'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1}, + {'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1}, # new writes - {'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0}, - {'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0}, - {'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0}, + {'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0}, + {'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0}, + {'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0}, ] for request, expectations in zip(backend_requests, expected): check_request(request, **expectations) @@ -4664,13 +4666,13 @@ class TestObjectController(unittest.TestCase): self.assertEqual( seen_headers, [ {'X-Container-Host': '10.0.0.0:1000', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sda'}, {'X-Container-Host': '10.0.0.1:1001', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdb'}, {'X-Container-Host': '10.0.0.2:1002', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdc'}]) def test_PUT_x_container_headers_with_fewer_container_replicas(self): @@ -4686,10 +4688,10 @@ class TestObjectController(unittest.TestCase): self.assertEqual( seen_headers, [ {'X-Container-Host': '10.0.0.0:1000', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sda'}, {'X-Container-Host': '10.0.0.1:1001', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdb'}, {'X-Container-Host': None, 'X-Container-Partition': None, @@ -4708,13 +4710,13 @@ class TestObjectController(unittest.TestCase): self.assertEqual( seen_headers, [ {'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sda,sdd'}, {'X-Container-Host': '10.0.0.1:1001', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdb'}, {'X-Container-Host': '10.0.0.2:1002', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdc'}]) def test_POST_x_container_headers_with_more_container_replicas(self): @@ -4732,13 +4734,13 @@ class TestObjectController(unittest.TestCase): self.assertEqual( seen_headers, [ {'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sda,sdd'}, {'X-Container-Host': '10.0.0.1:1001', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdb'}, {'X-Container-Host': '10.0.0.2:1002', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdc'}]) def test_DELETE_x_container_headers_with_more_container_replicas(self): @@ -4754,13 +4756,13 @@ class TestObjectController(unittest.TestCase): self.assertEqual(seen_headers, [ {'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sda,sdd'}, {'X-Container-Host': '10.0.0.1:1001', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdb'}, {'X-Container-Host': '10.0.0.2:1002', - 'X-Container-Partition': '1', + 'X-Container-Partition': '0', 'X-Container-Device': 'sdc'} ]) @@ -4787,11 +4789,11 @@ class TestObjectController(unittest.TestCase): self.assertEqual(seen_headers, [ {'X-Delete-At-Host': '10.0.0.0:1000', 'X-Delete-At-Container': delete_at_container, - 'X-Delete-At-Partition': '1', + 'X-Delete-At-Partition': '0', 'X-Delete-At-Device': 'sda'}, {'X-Delete-At-Host': '10.0.0.1:1001', 'X-Delete-At-Container': delete_at_container, - 'X-Delete-At-Partition': '1', + 'X-Delete-At-Partition': '0', 'X-Delete-At-Device': 'sdb'}, {'X-Delete-At-Host': None, 'X-Delete-At-Container': None, @@ -4823,15 +4825,15 @@ class TestObjectController(unittest.TestCase): self.assertEqual(seen_headers, [ {'X-Delete-At-Host': '10.0.0.0:1000,10.0.0.3:1003', 'X-Delete-At-Container': delete_at_container, - 'X-Delete-At-Partition': '1', + 'X-Delete-At-Partition': '0', 'X-Delete-At-Device': 'sda,sdd'}, {'X-Delete-At-Host': '10.0.0.1:1001', 'X-Delete-At-Container': delete_at_container, - 'X-Delete-At-Partition': '1', + 'X-Delete-At-Partition': '0', 'X-Delete-At-Device': 'sdb'}, {'X-Delete-At-Host': '10.0.0.2:1002', 'X-Delete-At-Container': delete_at_container, - 'X-Delete-At-Partition': '1', + 'X-Delete-At-Partition': '0', 'X-Delete-At-Device': 'sdc'} ]) @@ -5827,10 +5829,10 @@ class TestContainerController(unittest.TestCase): 200, 201, 201, 201) # HEAD PUT PUT PUT self.assertEqual(seen_headers, [ {'X-Account-Host': '10.0.0.0:1000', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sda'}, {'X-Account-Host': '10.0.0.1:1001', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdb'}, {'X-Account-Host': None, 'X-Account-Partition': None, @@ -5847,13 +5849,13 @@ class TestContainerController(unittest.TestCase): 200, 201, 201, 201) # HEAD PUT PUT PUT self.assertEqual(seen_headers, [ {'X-Account-Host': '10.0.0.0:1000,10.0.0.3:1003', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sda,sdd'}, {'X-Account-Host': '10.0.0.1:1001', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdb'}, {'X-Account-Host': '10.0.0.2:1002', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdc'} ]) @@ -5867,10 +5869,10 @@ class TestContainerController(unittest.TestCase): 200, 204, 204, 204) # HEAD DELETE DELETE DELETE self.assertEqual(seen_headers, [ {'X-Account-Host': '10.0.0.0:1000', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sda'}, {'X-Account-Host': '10.0.0.1:1001', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdb'}, {'X-Account-Host': None, 'X-Account-Partition': None, @@ -5887,13 +5889,13 @@ class TestContainerController(unittest.TestCase): 200, 204, 204, 204) # HEAD DELETE DELETE DELETE self.assertEqual(seen_headers, [ {'X-Account-Host': '10.0.0.0:1000,10.0.0.3:1003', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sda,sdd'}, {'X-Account-Host': '10.0.0.1:1001', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdb'}, {'X-Account-Host': '10.0.0.2:1002', - 'X-Account-Partition': '1', + 'X-Account-Partition': '0', 'X-Account-Device': 'sdc'} ])