Skip to content

Commit da6b9eb

Browse files
committed
Retry entire test_paginate_files() storage system test.
This is not just "taking the easy way out", it's really the most appropriate fix since there are so many assertions that can fail due to eventual consistency. (For example, asking for 2 blobs should have a next page when 3 are in the bucket, but this may break down due to eventual consistency.) Fixes googleapis#2217. Also - restoring test_second_level() to pre-googleapis#2252 (by retrying the entire test case) - restoring test_list_files() to pre-googleapis#2181 (by retrying the entire test case) - adding retries around remaining test cases that call list_blobs(): test_root_level_w_delimiter(), test_first_level() and test_third_level() - adding a helper to empty a bucket in setUpClass() helper (which also uses list_blobs()) in both TestStoragePseudoHierarchy and TestStorageListFiles
1 parent 49b3f1b commit da6b9eb

File tree

1 file changed

+28
-29
lines changed

1 file changed

+28
-29
lines changed

system_tests/storage.py

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
from system_test_utils import unique_resource_id
2828
from retry import RetryErrors
29-
from retry import RetryResult
3029

3130

3231
HTTP = httplib2.Http()
@@ -44,6 +43,19 @@ def _bad_copy(bad_request):
4443
error_predicate=_bad_copy)
4544

4645

46+
def _empty_bucket(bucket):
47+
"""Empty a bucket of all existing blobs.
48+
49+
This accounts (partially) for the eventual consistency of the
50+
list blobs API call.
51+
"""
52+
for blob in bucket.list_blobs():
53+
try:
54+
blob.delete()
55+
except exceptions.NotFound: # eventual consistency
56+
pass
57+
58+
4759
class Config(object):
4860
"""Run-time configuration to be modified at set-up.
4961
@@ -216,8 +228,7 @@ class TestStorageListFiles(TestStorageFiles):
216228
def setUpClass(cls):
217229
super(TestStorageListFiles, cls).setUpClass()
218230
# Make sure bucket empty before beginning.
219-
for blob in cls.bucket.list_blobs():
220-
blob.delete()
231+
_empty_bucket(cls.bucket)
221232

222233
logo_path = cls.FILES['logo']['path']
223234
blob = storage.Blob(cls.FILENAMES[0], bucket=cls.bucket)
@@ -235,18 +246,13 @@ def tearDownClass(cls):
235246
for blob in cls.suite_blobs_to_delete:
236247
blob.delete()
237248

249+
@RetryErrors(unittest.TestCase.failureException)
238250
def test_list_files(self):
239-
def _all_in_list(blobs):
240-
return len(blobs) == len(self.FILENAMES)
241-
242-
def _all_blobs():
243-
return list(self.bucket.list_blobs())
244-
245-
retry = RetryResult(_all_in_list)
246-
all_blobs = retry(_all_blobs)()
251+
all_blobs = list(self.bucket.list_blobs())
247252
self.assertEqual(sorted(blob.name for blob in all_blobs),
248253
sorted(self.FILENAMES))
249254

255+
@RetryErrors(unittest.TestCase.failureException)
250256
def test_paginate_files(self):
251257
truncation_size = 1
252258
count = len(self.FILENAMES) - truncation_size
@@ -277,11 +283,7 @@ class TestStoragePseudoHierarchy(TestStorageFiles):
277283
def setUpClass(cls):
278284
super(TestStoragePseudoHierarchy, cls).setUpClass()
279285
# Make sure bucket empty before beginning.
280-
for blob in cls.bucket.list_blobs():
281-
try:
282-
blob.delete()
283-
except exceptions.NotFound: # eventual consistency
284-
pass
286+
_empty_bucket(cls.bucket)
285287

286288
simple_path = cls.FILES['simple']['path']
287289
blob = storage.Blob(cls.FILENAMES[0], bucket=cls.bucket)
@@ -297,6 +299,7 @@ def tearDownClass(cls):
297299
for blob in cls.suite_blobs_to_delete:
298300
blob.delete()
299301

302+
@RetryErrors(unittest.TestCase.failureException)
300303
def test_root_level_w_delimiter(self):
301304
iterator = self.bucket.list_blobs(delimiter='/')
302305
response = iterator.get_next_page_response()
@@ -306,6 +309,7 @@ def test_root_level_w_delimiter(self):
306309
self.assertTrue(iterator.next_page_token is None)
307310
self.assertEqual(iterator.prefixes, set(['parent/']))
308311

312+
@RetryErrors(unittest.TestCase.failureException)
309313
def test_first_level(self):
310314
iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/')
311315
response = iterator.get_next_page_response()
@@ -315,30 +319,25 @@ def test_first_level(self):
315319
self.assertTrue(iterator.next_page_token is None)
316320
self.assertEqual(iterator.prefixes, set(['parent/child/']))
317321

322+
@RetryErrors(unittest.TestCase.failureException)
318323
def test_second_level(self):
319324
expected_names = [
320325
'parent/child/file21.txt',
321326
'parent/child/file22.txt',
322327
]
323328

324-
def _all_in_list(pair):
325-
_, blobs = pair
326-
return [blob.name for blob in blobs] == expected_names
327-
328-
def _all_blobs():
329-
iterator = self.bucket.list_blobs(delimiter='/',
330-
prefix='parent/child/')
331-
response = iterator.get_next_page_response()
332-
blobs = list(iterator.get_items_from_response(response))
333-
return iterator, blobs
334-
335-
retry = RetryResult(_all_in_list)
336-
iterator, _ = retry(_all_blobs)()
329+
iterator = self.bucket.list_blobs(delimiter='/',
330+
prefix='parent/child/')
331+
response = iterator.get_next_page_response()
332+
blobs = list(iterator.get_items_from_response(response))
333+
self.assertEqual([blob.name for blob in blobs],
334+
expected_names)
337335
self.assertEqual(iterator.page_number, 1)
338336
self.assertTrue(iterator.next_page_token is None)
339337
self.assertEqual(iterator.prefixes,
340338
set(['parent/child/grand/', 'parent/child/other/']))
341339

340+
@RetryErrors(unittest.TestCase.failureException)
342341
def test_third_level(self):
343342
# Pseudo-hierarchy can be arbitrarily deep, subject to the limit
344343
# of 1024 characters in the UTF-8 encoded name:

0 commit comments

Comments
 (0)