Retry entire test_paginate_files() storage system test.

[dhermes]

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 #2217.

Also

• restoring test_second_level() to pre-Adding a retry for storage list_blobs(). #2252 (by retrying the entire test case)
• restoring test_list_files() to pre-Add eventual consistency check for storage list_blobs() #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

---

@tseaver I would like some input here. I am worried that RetryErrors on the error unittest.TestCase.failureException is insufficient / overly general. This is especially because
unittest.TestCase.failureException is just an alias for AssertionError.

Should we temporarily replace it (via another decorator)?

[daspecster]

@dhermes, one thing I thought about the issue you mentioned with RetryErrors, is that the exception can be passed in if you want a different exception caught instead of AssertionError.

I also think you could stack RetryErrors decorators to capture more than one specific exception if I'm not mistaken.

[dhermes]

@daspecster Thanks for the tip, though I was aware of that behavior already. The point of this is that the failures are quite complex and to really change them to be eventual-consistency-proof would be equivalent to decorating the entire method, so we might as well.

[tseaver]

I'm doubtful that wrapping the entire testcase is going to make us more robust than wrapping the individual list_blobs calls: for testcase methods that call list_blobs more than once, we might need at the minimum to up the retry count, for instance.

[dhermes]

@tseaver None of the wrapped tests call list_blobs(), they fetch a single page and check information in the iterator as well. The tests need to check the state of so many different things, it's equivalent to just wrapping the entire function in an inner function and changing some of the asserts to part of the boolean checked by RetryResult.

[dhermes]

@tseaver Bump. This showed up again in:

https://travis-ci.org/GoogleCloudPlatform/google-cloud-python/builds/160041019

[tseaver]

None of the wrapped tests call list_blobs(), they fetch a single page and check information in the iterator as well

Hmm? The all call list_blobs: I guess maybe you meant "none call it more than once?"

I'm still dubious that we should assume any assertion failure in one of these tests is due to an eventual consistency error.

[dhermes]

@tseaver Regarding your doubt:

• test_list_files only has one assertion and list(self.bucket.list_blobs()) is definitely subject to eventual consistency (it may have to hit multiple pages, but more likely a single page)
• test_paginate_files (using max_results=2): The assertions are

1. 2 items in first page (can fail if backend's list only has 1 element due to eventual consistentcy)
2. iterator is on the first page (can't fail, just a += 1 locally)
3. next_page_token isn't null in the first page / i.e. there is another page (can fail if backend's list only has 2 elements due to eventual consistency)
4. the number of blobs in the last page is 1 (probably can't fail if the previous succeeded, though might if two different machines had different version of state)

• test_root_level_w_delimiter gets all prefixes and files not containing /. The assertions are:

1. the only blobs returned are file01.txt (can fail if backend doesn't have file01.txt due to eventual consistency)
2. iterator is on the first page (can't fail, just a += 1 locally)
3. next_page_token is null / i.e. there is another page (probably can't fail?)
4. the prefixes returned are parent/ (can fail if backend doesn't have any of the parent/ files due to eventual consistency)

• test_first_level gets all prefixes and files not containing / with prefix prefix/. The assertions are:

1. the only blobs returned are parent/file11.txt (can fail if backend doesn't have parent/file11.txt due to eventual consistency)
2. iterator is on the first page (can't fail, just a += 1 locally)
3. next_page_token is null / i.e. there is another page (probably can't fail?)
4. the prefixes returned are parent/child/ (can fail if backend doesn't have any of the parent/child/ files due to eventual consistency)

test_second_level and test_third_level are essentially the same as test_first_level

[tseaver]

I'll acquiesce, but still think we'd be better off repeating the list_blobs API request explicitly until it returned the expected number of files. LGTM

[tseaver]

After merging #2323 and #2248, this was the only remaining failure:

• https://travis-ci.org/GoogleCloudPlatform/google-cloud-python/builds/160327181

[daspecster]

@dhermes @tseaver, I think this was good to go right? I think that @tseaver gave the LGTM?