Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<100% test coverage for generated files #632

Closed
larkee opened this issue Oct 2, 2020 · 8 comments
Closed

<100% test coverage for generated files #632

larkee opened this issue Oct 2, 2020 · 8 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@larkee
Copy link

larkee commented Oct 2, 2020

Running nox -s unit cover gives the following coverage results:

nox > Running session cover
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/cover
nox > pip install coverage pytest-cov
nox > coverage report --show-missing --fail-under=100
Name                                                                                        Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------------------------------------------------------------------
google/cloud/__init__.py                                                                        6      6      0      0     0%   1-8
google/cloud/spanner/__init__.py                                                               14      0      0      0   100%
google/cloud/spanner/_helpers.py                                                              119      0     82      0   100%
google/cloud/spanner/_opentelemetry_tracing.py                                                 29      0      8      0   100%
google/cloud/spanner/backup.py                                                                 90      0     10      0   100%
google/cloud/spanner/batch.py                                                                  47      0      4      0   100%
google/cloud/spanner/client.py                                                                 89      0     18      0   100%
google/cloud/spanner/database.py                                                              280      0     54      0   100%
google/cloud/spanner/instance.py                                                              117      0     10      0   100%
google/cloud/spanner/keyset.py                                                                 78      0     48      0   100%
google/cloud/spanner/param_types.py                                                            18      0      0      0   100%
google/cloud/spanner/pool.py                                                                  172      0     32      0   100%
google/cloud/spanner/session.py                                                               148      0     46      0   100%
google/cloud/spanner/snapshot.py                                                              186      0     80      0   100%
google/cloud/spanner/streamed.py                                                              134      0     36      0   100%
google/cloud/spanner/transaction.py                                                           113      0     32      0   100%
google/cloud/spanner_admin_database_v1/__init__.py                                             32      0      0      0   100%
google/cloud/spanner_admin_database_v1/services/__init__.py                                     0      0      0      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/__init__.py                      3      0      0      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/async_client.py                243      0     96      3    99%   802->803, 953->954, 1056->1057
google/cloud/spanner_admin_database_v1/services/database_admin/client.py                      338      0    154      4    99%   943->948, 1091->1096, 1183->1186, 1186->1191
google/cloud/spanner_admin_database_v1/services/database_admin/pagers.py                      160      0     40      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/transports/__init__.py           9      0      0      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/transports/base.py              73      0      8      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/transports/grpc.py             137      0     46      0   100%
google/cloud/spanner_admin_database_v1/services/database_admin/transports/grpc_asyncio.py     140      0     46      0   100%
google/cloud/spanner_admin_database_v1/types/__init__.py                                        4      0      0      0   100%
google/cloud/spanner_admin_database_v1/types/backup.py                                         61      0      0      0   100%
google/cloud/spanner_admin_database_v1/types/common.py                                          8      0      0      0   100%
google/cloud/spanner_admin_database_v1/types/spanner_database_admin.py                         80      0      0      0   100%
google/cloud/spanner_admin_instance_v1/__init__.py                                             16      0      0      0   100%
google/cloud/spanner_admin_instance_v1/services/__init__.py                                     0      0      0      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/__init__.py                      3      0      0      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/async_client.py                157      0     60      3    99%   956->957, 1103->1104, 1203->1204
google/cloud/spanner_admin_instance_v1/services/instance_admin/client.py                      238      0    104      4    99%   1096->1101, 1240->1245, 1329->1332, 1332->1337
google/cloud/spanner_admin_instance_v1/services/instance_admin/pagers.py                       80      0     20      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/transports/__init__.py           9      0      0      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/transports/base.py              57      0      8      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/transports/grpc.py             100      0     32      0   100%
google/cloud/spanner_admin_instance_v1/services/instance_admin/transports/grpc_asyncio.py     103      0     32      0   100%
google/cloud/spanner_admin_instance_v1/types/__init__.py                                        2      0      0      0   100%
google/cloud/spanner_admin_instance_v1/types/spanner_instance_admin.py                         75      0      0      0   100%
google/cloud/spanner_v1/__init__.py                                                            38      0      0      0   100%
google/cloud/spanner_v1/services/__init__.py                                                    0      0      0      0   100%
google/cloud/spanner_v1/services/spanner/__init__.py                                            3      0      0      0   100%
google/cloud/spanner_v1/services/spanner/async_client.py                                      171      2     44      0    99%   1357-1358
google/cloud/spanner_v1/services/spanner/client.py                                            258      2    104      0    99%   1461-1462
google/cloud/spanner_v1/services/spanner/pagers.py                                             41      0     10      0   100%
google/cloud/spanner_v1/services/spanner/transports/__init__.py                                 9      0      0      0   100%
google/cloud/spanner_v1/services/spanner/transports/base.py                                    63      2      8      0    97%   38-39
google/cloud/spanner_v1/services/spanner/transports/grpc.py                                   118      0     40      0   100%
google/cloud/spanner_v1/services/spanner/transports/grpc_asyncio.py                           121      0     40      0   100%
google/cloud/spanner_v1/types/__init__.py                                                       8      0      0      0   100%
google/cloud/spanner_v1/types/keys.py                                                          13      0      0      0   100%
google/cloud/spanner_v1/types/mutation.py                                                      18      0      0      0   100%
google/cloud/spanner_v1/types/query_plan.py                                                    25      0      0      0   100%
google/cloud/spanner_v1/types/result_set.py                                                    25      0      0      0   100%
google/cloud/spanner_v1/types/spanner.py                                                      115      0      0      0   100%
google/cloud/spanner_v1/types/transaction.py                                                   25      0      0      0   100%
google/cloud/spanner_v1/types/type.py                                                          24      0      0      0   100%
tests/unit/__init__.py                                                                          0      0      0      0   100%
tests/unit/test__helpers.py                                                                   503      0     12      0   100%
tests/unit/test__opentelemetry_tracing.py                                                      85      0      2      0   100%
tests/unit/test_backup.py                                                                     334      0      0      0   100%
tests/unit/test_batch.py                                                                      241      0     12      0   100%
tests/unit/test_client.py                                                                     341      0     10      0   100%
tests/unit/test_database.py                                                                  1191      0     24      0   100%
tests/unit/test_instance.py                                                                   536      0     20      0   100%
tests/unit/test_keyset.py                                                                     286      0      4      0   100%
tests/unit/test_param_types.py                                                                 19      0      0      0   100%
tests/unit/test_pool.py                                                                       688      0     50      0   100%
tests/unit/test_session.py                                                                    774      0     20      0   100%
tests/unit/test_snapshot.py                                                                   770      0     44      0   100%
tests/unit/test_streamed.py                                                                   717      0     78      0   100%
tests/unit/test_transaction.py                                                                452      0     16      0   100%
---------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                       11780     12   1644     14    99%

All the of files with <100% coverage are generated files. Should these files be at 100% coverage or should the fail_under parameter be lowered?

Code can be found at: https://github.com/larkee/python-spanner/tree/generator-migration

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Oct 2, 2020
@software-dov
Copy link
Contributor

I took a look at some of the specific failures, and they all look spurious. I am, in principle, against lowering test coverage rate.

It looks like the failures in client submodules are branch failures, not line failures. The logic there is checking multiple different, independent keyword parameters and is setting something in a local variable if the keyword is set. I'm not quite sure why the not-taken branch isn't getting tested, but I can work on that.

The coverage failures in transport submodules are due to a variable initialization that has some fallback logic triggered by an exception. That whole initialization could reasonably be put under nocover, because the variable is used a little while later on. If that logic really failed, worse things would happen almost immediately.

TL;DR: I don't think this coverage failure is indicative of anything serious. The instances just need to be worked around or new generated unit tests need to be added.

@software-dov software-dov added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Oct 5, 2020
@software-dov
Copy link
Contributor

Some of the recent fixes for other APIs have also removed this issue, or at least, I can't reproduce it when I generate the Spanner GAPIC locally using the most recent generator version. We can reopen the issue if it pops up again.

@tseaver
Copy link
Contributor

tseaver commented Apr 21, 2021

I'm still seeing coverage failures in python-datastore fore the generated async_client and pagers modules:

@tseaver tseaver reopened this Apr 21, 2021
@tseaver
Copy link
Contributor

tseaver commented Apr 21, 2021

Looking in detail at the missing lines: none of the API invocation methods are being tested for the async clients: only the contructors / factories are being tested. None of the methods of the async pager are tested, either.

@software-dov
Copy link
Contributor

software-dov commented Apr 21, 2021

none of the API invocation methods are being tested for the async clients
@tseaver can you elaborate? The test template definitely has definitions for async client and async method variant tests.

I am having trouble recreating the issue as stated. Locally, I see a problem that seems to be related to mocking credentials:

>           raise _InactiveRpcError(state)
E           grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
E           	status = StatusCode.UNAUTHENTICATED
E           	details = "Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project."
E           	debug_error_string = "{"created":"@1619028490.436427028","description":"Error received from peer ipv6:[2607:f8b0:400a:805::200a]:443","file":"src/core/lib/surface/call.cc","file_line":1067,"grpc_message":"Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project.","grpc_status":16}"
E           >

How urgently is a fix needed? Is it just blocking regeneration? Are there any critical features that would be blocked if we deferred regeneration?

@tseaver
Copy link
Contributor

tseaver commented Apr 21, 2021

@software-dov this issue is for unit test coverage, which we expect to be at 100% -- there should be no actual gRPC occurring. Looking at the test_export_entities method in the generated test_datastore_admin.py, it is clear that the async client is not being tested.

@tseaver
Copy link
Contributor

tseaver commented Jun 7, 2021

Coverage is again / finally at 100% for googleapis/python-datastore#170.

@tseaver tseaver closed this as completed Jun 7, 2021
@tseaver
Copy link
Contributor

tseaver commented Jun 25, 2021

And we're back!

googleapis/python-bigtable#335 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants