ResultSet: add tests to cover paging with empty pages #104
Conversation
| @@ -41,6 +41,19 @@ def test_iter_paged(self): | |||
| type(response_future).has_more_pages = PropertyMock(side_effect=(True, True, False)) # after init to avoid side effects being consumed by init | |||
| self.assertListEqual(list(itr), expected) | |||
|
|
|||
| def test_iter_paged_with_empty_pages(self): | |||
| expected = list(range(10)) | |||
| response_future = Mock(has_more_pages=True, _continuous_paging_session=None) | |||
There was a problem hiding this comment.
Why is it ok to always return has_more_pages==True? Shouldn't it return False for the last page?
There was a problem hiding this comment.
because stopIteration will be raised anyway, so in the scope of this test, we do not need to add the complexity of chaging the paging state of the response future to demonstrate and test the fixed behavior
There was a problem hiding this comment.
I must have a bad day because I don't see how it works.
If has_more_pages is True then the condition in this if will be false and the exception won't be raised, no?
That in turn will lead to next being called again recursively over and over gain, no?
or will the StopIteration be thrown from fetch_next_page?
There was a problem hiding this comment.
I understand it's not obvious sorry.
The StopIteration comes from the mocked response_future.result() iterator over the configured side_effects.
In other words, the tests do iterate over the side_effects every time response_future.result() is called in fetch_next_page(), when all the side_effects (pages) have been depleted, the next call to response_future.result() will raise a StopIteration exception.
Does that make more sense?
There was a problem hiding this comment.
yep. That's what I meant by StopIteration being thrown from fetch_next_page. Thanks.
|
#need a pypi release plz |
Related to #102