From c055821021272df22b191a22286b2a4a63392d95 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 10:09:43 -0800 Subject: [PATCH 01/12] Initial commit for the raw_page tests --- tests/system/test_pagination.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index a0f316a6dd..ba1982725a 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -16,7 +16,7 @@ def test_pagination(echo): - text = 'The rain in Wales falls mainly on the snails.' + text = 'The hail in Wales falls mainly on the snails.' results = [i for i in echo.paged_expand({ 'content': text, 'page_size': 3, @@ -24,3 +24,18 @@ def test_pagination(echo): assert len(results) == 9 assert results == [showcase.EchoResponse(content=i) for i in text.split(' ')] + +def test_pagination_pages(echo): + text = "The hail in Wales falls mainly on the snails." + page_results = list(echo.paged_expand({ + 'tontent': text, + 'page_size': 3, + }).pages) + + assert len(page_results) == 3 + + # The monolithic surface uses a wrapper type that needs an explicit property + # for a 'raw_page': we need to duplicate that interface, even though the + # architecture is different. + assert page_results[0].raw_page is page_results[0] + assert page_results.next_page_token is None From e1ef9e8ba817f7a063b10dcdcfbe1f745bd7b9fe Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 12:04:16 -0800 Subject: [PATCH 02/12] Impl and testfix for raw_pages --- .../$sub/services/$service/pagers.py.j2 | 20 +++++++++---------- .../$name_$version/$sub/types/_message.py.j2 | 6 ++++++ tests/system/test_pagination.py | 8 ++++++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/gapic/templates/$namespace/$name_$version/$sub/services/$service/pagers.py.j2 b/gapic/templates/$namespace/$name_$version/$sub/services/$service/pagers.py.j2 index 4efcdc0b46..0e7ef018a7 100644 --- a/gapic/templates/$namespace/$name_$version/$sub/services/$service/pagers.py.j2 +++ b/gapic/templates/$namespace/$name_$version/$sub/services/$service/pagers.py.j2 @@ -56,19 +56,17 @@ class {{ method.name }}Pager: def __getattr__(self, name: str) -> Any: return getattr(self._response, name) - def __iter__(self) -> {{ method.paged_result_field.ident | replace('Sequence', 'Iterable') }}: - while True: - # Iterate through the results on this response. - for result in self._response.{{ method.paged_result_field.name }}: - yield result - - # Sanity check: Is this the last page? If so, we are done. - if not self._response.next_page_token: - break - - # Get the next page. + @property + def pages(self) -> Iterable[{{ method.output.ident }}]: + yield self._response + while self._response.next_page_token: self._request.page_token = self._response.next_page_token self._response = self._method(self._request) + yield self._response + + def __iter__(self) -> {{ method.paged_result_field.ident | replace('Sequence', 'Iterable') }}: + for page in self.pages: + yield from page.{{ method.paged_result_field.name }} def __repr__(self) -> str: return '{0}<{1!r}>'.format(self.__class__.__name__, self._response) diff --git a/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 b/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 index c9e464e117..a12da728b7 100644 --- a/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 +++ b/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 @@ -24,6 +24,12 @@ class {{ message.name }}({{ p }}.Message): {% endif %} {% endfor -%} + {% if "next_page_token" in message.fields.values()|map(attribute='name') -%} + @property + def raw_page(self): + return self + {% endif -%} + {# Iterate over fields. -#} {% for field in message.fields.values() -%} {% if field.map -%} diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index ba1982725a..7e95b99286 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -28,14 +28,18 @@ def test_pagination(echo): def test_pagination_pages(echo): text = "The hail in Wales falls mainly on the snails." page_results = list(echo.paged_expand({ - 'tontent': text, + 'content': text, 'page_size': 3, }).pages) assert len(page_results) == 3 + assert not page_results[-1].next_page_token # The monolithic surface uses a wrapper type that needs an explicit property # for a 'raw_page': we need to duplicate that interface, even though the # architecture is different. assert page_results[0].raw_page is page_results[0] - assert page_results.next_page_token is None + + results = [r for p in page_results for r in p.responses] + assert results == [showcase.EchoResponse(content=i) + for i in text.split(' ')] From fcbfc1c86808831e4b3c1e1e43a703f82731bd65 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 12:56:09 -0800 Subject: [PATCH 03/12] Style check fix --- tests/system/test_pagination.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index 7e95b99286..781614cad4 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -25,6 +25,7 @@ def test_pagination(echo): assert results == [showcase.EchoResponse(content=i) for i in text.split(' ')] + def test_pagination_pages(echo): text = "The hail in Wales falls mainly on the snails." page_results = list(echo.paged_expand({ From 58f7d6d14c79f0263a3b235aa7fa5f71c2c016f2 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 13:23:55 -0800 Subject: [PATCH 04/12] Try to improve test coverage --- .../$name_$version/$sub/types/_message.py.j2 | 4 ++-- tests/system/conftest.py | 1 + tests/system/test_pagination.py | 13 +++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 b/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 index a12da728b7..3afab165ad 100644 --- a/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 +++ b/gapic/templates/$namespace/$name_$version/$sub/types/_message.py.j2 @@ -24,11 +24,11 @@ class {{ message.name }}({{ p }}.Message): {% endif %} {% endfor -%} - {% if "next_page_token" in message.fields.values()|map(attribute='name') -%} + {% if "next_page_token" in message.fields.values()|map(attribute='name') %} @property def raw_page(self): return self - {% endif -%} + {% endif %} {# Iterate over fields. -#} {% for field in message.fields.values() -%} diff --git a/tests/system/conftest.py b/tests/system/conftest.py index f7b6ebb897..29507026d0 100644 --- a/tests/system/conftest.py +++ b/tests/system/conftest.py @@ -16,6 +16,7 @@ from google.showcase import EchoClient from google.showcase import IdentityClient +from google.showcase import MessagingClient import grpc diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index 781614cad4..f9c8f3337f 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -44,3 +44,16 @@ def test_pagination_pages(echo): results = [r for p in page_results for r in p.responses] assert results == [showcase.EchoResponse(content=i) for i in text.split(' ')] + +def test_pagination_raw_page(): + for response_type in [ + showcase.PagedExpandResponse, + showcase.ListRoomsResponse, + showcase.ListBlurbsResponse, + showcase.SearchBlurbsResponse + showcase.ListUsersResponse, + showcase.ListSessionsResponse, + showcase.ListTestsResponse, + ]: + response = response_type() + assert response.raw_page is response From 37f2c8bdece86a5f1cffd21d1c871b3508a98f8e Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 13:25:22 -0800 Subject: [PATCH 05/12] Style check fix --- tests/system/test_pagination.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index f9c8f3337f..2a7e25682a 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -45,6 +45,7 @@ def test_pagination_pages(echo): assert results == [showcase.EchoResponse(content=i) for i in text.split(' ')] + def test_pagination_raw_page(): for response_type in [ showcase.PagedExpandResponse, From bb4ba1725d93f37369d78b8bab3dd2455f2de91e Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 13:37:53 -0800 Subject: [PATCH 06/12] Add generated unit tests for raw_page --- .../$name_$version/$sub/test_$service.py.j2 | 44 ++++++++++++++++++- tests/system/test_pagination.py | 14 ------ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index ce9203007f..21f934549c 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -211,7 +211,49 @@ def test_{{ method.name|snake_case }}_pager(): )] assert len(results) == 6 assert all([isinstance(i, {{ method.paged_result_field.message.ident }}) - for i in results]) + for i in results]) + +def test_{{ method.name|snake_case }}_pages(): + client = {{ service.client_name }}( + credentials=credentials.AnonymousCredentials, + ) + + # Mock the actual call within the gRPC stub, and fake the request. + with mock.patch.object( + type(client._transport.{{ method.name|snake_case }}), + '__call__') as call: + # Set the response to a series of pages. + call.side_effect = ( + {{ method.output.ident }}( + {{ method.paged_result_field.name }}=[ + {{ method.paged_result_field.message.ident }}(), + {{ method.paged_result_field.message.ident }}(), + {{ method.paged_result_field.message.ident }}(), + ], + next_page_token='abc', + ), + {{ method.output.ident }}( + {{ method.paged_result_field.name }}=[], + next_page_token='def', + ), + {{ method.output.ident }}( + {{ method.paged_result_field.name }}=[ + {{ method.paged_result_field.message.ident }}(), + ], + next_page_token='ghi', + ), + {{ method.output.ident }}( + {{ method.paged_result_field.name }}=[ + {{ method.paged_result_field.message.ident }}(), + {{ method.paged_result_field.message.ident }}(), + ], + ), + RuntimeError, + ) + pages = list(client.{{ method.name|snake_case }}(request={}).pages) + assert pages[0].raw_page.next_page_token == pages[1].page_token + assert pages[1].raw_page.next_page_token == pages[2].page_token + assert not pages[2].raw_page.next_page_token {% endif %} {#- method.paged_response_field #} {% endfor -%} {#- method in methods #} diff --git a/tests/system/test_pagination.py b/tests/system/test_pagination.py index 2a7e25682a..781614cad4 100644 --- a/tests/system/test_pagination.py +++ b/tests/system/test_pagination.py @@ -44,17 +44,3 @@ def test_pagination_pages(echo): results = [r for p in page_results for r in p.responses] assert results == [showcase.EchoResponse(content=i) for i in text.split(' ')] - - -def test_pagination_raw_page(): - for response_type in [ - showcase.PagedExpandResponse, - showcase.ListRoomsResponse, - showcase.ListBlurbsResponse, - showcase.SearchBlurbsResponse - showcase.ListUsersResponse, - showcase.ListSessionsResponse, - showcase.ListTestsResponse, - ]: - response = response_type() - assert response.raw_page is response From 2f04b3ec08fdb99abf53aa2999dff0370e134411 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 13:43:01 -0800 Subject: [PATCH 07/12] Fix the generated tests --- .../tests/unit/$name_$version/$sub/test_$service.py.j2 | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index 21f934549c..b1dff5b1e7 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -251,9 +251,8 @@ def test_{{ method.name|snake_case }}_pages(): RuntimeError, ) pages = list(client.{{ method.name|snake_case }}(request={}).pages) - assert pages[0].raw_page.next_page_token == pages[1].page_token - assert pages[1].raw_page.next_page_token == pages[2].page_token - assert not pages[2].raw_page.next_page_token + for page, token in zip(pages, ['abc','def','ghi', '']): + assert page.raw_page.next_page_token == token {% endif %} {#- method.paged_response_field #} {% endfor -%} {#- method in methods #} From 4d3bf6150f6032526197a594d44c7156a3c224f1 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 14:16:55 -0800 Subject: [PATCH 08/12] WIP: attempt to fix LRO paginated response test hole --- .../tests/unit/$name_$version/$sub/test_$service.py.j2 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index b1dff5b1e7..077afead92 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -253,7 +253,11 @@ def test_{{ method.name|snake_case }}_pages(): pages = list(client.{{ method.name|snake_case }}(request={}).pages) for page, token in zip(pages, ['abc','def','ghi', '']): assert page.raw_page.next_page_token == token -{% endif %} {#- method.paged_response_field #} +{% elif method.lro and "next_page_token" in method.lro.response_type.fields|map(attribute="name") %} +def test_{{ method.name|snake_case }}_raw_page_lro(): + response = {{ method.lro.response_type.ident }}()n + assert response.raw_page is response +{% endif %} {#- method.paged_result_field #} {% endfor -%} {#- method in methods #} From 5e569d2ae4e003bb9a135cd2301616926b8c7dbb Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 14:20:11 -0800 Subject: [PATCH 09/12] Take 47 --- .../tests/unit/$name_$version/$sub/test_$service.py.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index 077afead92..c8fdfb7397 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -253,7 +253,7 @@ def test_{{ method.name|snake_case }}_pages(): pages = list(client.{{ method.name|snake_case }}(request={}).pages) for page, token in zip(pages, ['abc','def','ghi', '']): assert page.raw_page.next_page_token == token -{% elif method.lro and "next_page_token" in method.lro.response_type.fields|map(attribute="name") %} +{% elif method.lro and "next_page_token" in method.lro.response_type.fields.keys() %} def test_{{ method.name|snake_case }}_raw_page_lro(): response = {{ method.lro.response_type.ident }}()n assert response.raw_page is response From ab245a6c5c169641c53d0439286ed387b167c3d5 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 14:26:10 -0800 Subject: [PATCH 10/12] Take 48 --- .../tests/unit/$name_$version/$sub/test_$service.py.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index c8fdfb7397..40e3113142 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -255,7 +255,7 @@ def test_{{ method.name|snake_case }}_pages(): assert page.raw_page.next_page_token == token {% elif method.lro and "next_page_token" in method.lro.response_type.fields.keys() %} def test_{{ method.name|snake_case }}_raw_page_lro(): - response = {{ method.lro.response_type.ident }}()n + response = {{ method.lro.response_type.ident }}() assert response.raw_page is response {% endif %} {#- method.paged_result_field #} From 6f28e195e993ea89e8e8f239a20d79fe4a4b8f5b Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 14:30:51 -0800 Subject: [PATCH 11/12] Take 49 --- tests/system/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/system/conftest.py b/tests/system/conftest.py index 29507026d0..f7b6ebb897 100644 --- a/tests/system/conftest.py +++ b/tests/system/conftest.py @@ -16,7 +16,6 @@ from google.showcase import EchoClient from google.showcase import IdentityClient -from google.showcase import MessagingClient import grpc From 089d026ed1f20759fb10c51a8013aa40339247b0 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 5 Nov 2019 14:42:19 -0800 Subject: [PATCH 12/12] Whitespace fix --- .../tests/unit/$name_$version/$sub/test_$service.py.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 index 40e3113142..8804d86ea4 100644 --- a/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 +++ b/gapic/templates/tests/unit/$name_$version/$sub/test_$service.py.j2 @@ -211,7 +211,7 @@ def test_{{ method.name|snake_case }}_pager(): )] assert len(results) == 6 assert all([isinstance(i, {{ method.paged_result_field.message.ident }}) - for i in results]) + for i in results]) def test_{{ method.name|snake_case }}_pages(): client = {{ service.client_name }}(