Skip to content

Commit 733a5ac

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 733a5ac

File tree

1 file changed

+28
-28
lines changed

1 file changed

+28
-28
lines changed

system_tests/storage.py

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,19 @@ def _bad_copy(bad_request):
4444
error_predicate=_bad_copy)
4545

4646

47+
def _empty_bucket(bucket):
48+
"""Empty a bucket of all existing blobs.
49+
50+
This accounts (partially) for the eventual consistency of the
51+
list blobs API call.
52+
"""
53+
for blob in bucket.list_blobs():
54+
try:
55+
blob.delete()
56+
except exceptions.NotFound: # eventual consistency
57+
pass
58+
59+
4760
class Config(object):
4861
"""Run-time configuration to be modified at set-up.
4962
@@ -216,8 +229,7 @@ class TestStorageListFiles(TestStorageFiles):
216229
def setUpClass(cls):
217230
super(TestStorageListFiles, cls).setUpClass()
218231
# Make sure bucket empty before beginning.
219-
for blob in cls.bucket.list_blobs():
220-
blob.delete()
232+
_empty_bucket(cls.bucket)
221233

222234
logo_path = cls.FILES['logo']['path']
223235
blob = storage.Blob(cls.FILENAMES[0], bucket=cls.bucket)
@@ -235,18 +247,13 @@ def tearDownClass(cls):
235247
for blob in cls.suite_blobs_to_delete:
236248
blob.delete()
237249

250+
@RetryErrors(unittest.TestCase.failureException)
238251
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)()
252+
all_blobs = list(self.bucket.list_blobs())
247253
self.assertEqual(sorted(blob.name for blob in all_blobs),
248254
sorted(self.FILENAMES))
249255

256+
@RetryErrors(unittest.TestCase.failureException)
250257
def test_paginate_files(self):
251258
truncation_size = 1
252259
count = len(self.FILENAMES) - truncation_size
@@ -277,11 +284,7 @@ class TestStoragePseudoHierarchy(TestStorageFiles):
277284
def setUpClass(cls):
278285
super(TestStoragePseudoHierarchy, cls).setUpClass()
279286
# 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
287+
_empty_bucket(cls.bucket)
285288

286289
simple_path = cls.FILES['simple']['path']
287290
blob = storage.Blob(cls.FILENAMES[0], bucket=cls.bucket)
@@ -297,6 +300,7 @@ def tearDownClass(cls):
297300
for blob in cls.suite_blobs_to_delete:
298301
blob.delete()
299302

303+
@RetryErrors(unittest.TestCase.failureException)
300304
def test_root_level_w_delimiter(self):
301305
iterator = self.bucket.list_blobs(delimiter='/')
302306
response = iterator.get_next_page_response()
@@ -306,6 +310,7 @@ def test_root_level_w_delimiter(self):
306310
self.assertTrue(iterator.next_page_token is None)
307311
self.assertEqual(iterator.prefixes, set(['parent/']))
308312

313+
@RetryErrors(unittest.TestCase.failureException)
309314
def test_first_level(self):
310315
iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/')
311316
response = iterator.get_next_page_response()
@@ -315,30 +320,25 @@ def test_first_level(self):
315320
self.assertTrue(iterator.next_page_token is None)
316321
self.assertEqual(iterator.prefixes, set(['parent/child/']))
317322

323+
@RetryErrors(unittest.TestCase.failureException)
318324
def test_second_level(self):
319325
expected_names = [
320326
'parent/child/file21.txt',
321327
'parent/child/file22.txt',
322328
]
323329

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)()
330+
iterator = self.bucket.list_blobs(delimiter='/',
331+
prefix='parent/child/')
332+
response = iterator.get_next_page_response()
333+
blobs = list(iterator.get_items_from_response(response))
334+
self.assertEqual([blob.name for blob in blobs],
335+
expected_names)
337336
self.assertEqual(iterator.page_number, 1)
338337
self.assertTrue(iterator.next_page_token is None)
339338
self.assertEqual(iterator.prefixes,
340339
set(['parent/child/grand/', 'parent/child/other/']))
341340

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

0 commit comments

Comments
 (0)