Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .librarian/generator-input/librarian.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,4 +250,4 @@

# Use a python runtime which is available in the owlbot post processor here
# https://github.com/googleapis/synthtool/blob/master/docker/owlbot/python/Dockerfile
s.shell.run(["nox", "-s", "blacken-3.14"], hide_output=False)
s.shell.run(["nox", "-s", "format-3.14"], hide_output=False)
83 changes: 73 additions & 10 deletions .librarian/generator-input/noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.


# Generated by synthtool. DO NOT EDIT!

from __future__ import absolute_import
Expand Down Expand Up @@ -64,6 +65,7 @@
SYSTEM_TEST_STANDARD_DEPENDENCIES: List[str] = [
"mock",
"pytest",
"pytest-asyncio",
"google-cloud-testutils",
]
SYSTEM_TEST_EXTERNAL_DEPENDENCIES: List[str] = []
Expand Down Expand Up @@ -217,9 +219,8 @@ def unit(session, protobuf_implementation):
session.install("protobuf<4")

# Run py.test against the unit tests.
session.run(
args = [
"py.test",
"--quiet",
"-s",
f"--junitxml=unit_{session.python}_sponge_log.xml",
"--cov=google",
Expand All @@ -228,8 +229,13 @@ def unit(session, protobuf_implementation):
"--cov-config=.coveragerc",
"--cov-report=",
"--cov-fail-under=0",
os.path.join("tests", "unit"),
*session.posargs,
]
if not session.posargs:
args.append(os.path.join("tests", "unit"))
args.extend(session.posargs)

session.run(
*args,
env={
"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": protobuf_implementation,
},
Expand Down Expand Up @@ -362,25 +368,67 @@ def system(session, protobuf_implementation, database_dialect):

# Run py.test against the system tests.
if system_test_exists:
session.run(
args = [
"py.test",
"--quiet",
"-o",
"asyncio_mode=auto",
f"--junitxml=system_{session.python}_sponge_log.xml",
system_test_path,
*session.posargs,
]
if not session.posargs:
args.append(system_test_path)
args.extend(session.posargs)

session.run(
*args,
env={
"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": protobuf_implementation,
"SPANNER_DATABASE_DIALECT": database_dialect,
"SKIP_BACKUP_TESTS": "true",
},
)
elif system_test_folder_exists:
# Run sync tests
sync_args = [
"py.test",
"--quiet",
"-o",
"asyncio_mode=auto",
f"--junitxml=system_{session.python}_sync_sponge_log.xml",
]
if not session.posargs:
sync_args.append(os.path.join("tests", "system"))
sync_args.append("--ignore=tests/system/_async")
else:
sync_args.extend(session.posargs)

session.run(
*sync_args,
env={
"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": protobuf_implementation,
"SPANNER_DATABASE_DIALECT": database_dialect,
"SKIP_BACKUP_TESTS": "true",
},
)

# Run async tests
async_args = [
"py.test",
"--quiet",
f"--junitxml=system_{session.python}_sponge_log.xml",
system_test_folder_path,
*session.posargs,
"-o",
"asyncio_mode=auto",
f"--junitxml=system_{session.python}_async_sponge_log.xml",
]
if not session.posargs:
async_args.append(os.path.join("tests", "system", "_async"))
else:
# If posargs are provided, only run if they match async tests
# or just skip if they were already run in sync.
# For simplicity, we only run async folder if no posargs.
return
Comment on lines +424 to +428
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current logic for handling session.posargs in system tests is flawed. If positional arguments are provided (e.g., to run a specific test file), they are passed to the sync test runner, and the async test run is skipped entirely due to the return statement. This prevents developers from running specific async tests.

For example, running nox -s system -- tests/system/_async/test_it.py would execute the sync test command with the async test path, and then exit before the async test command is even constructed. This is incorrect.

A better approach would be to pass the posargs to the async test run as well, allowing pytest to correctly select and run the specified tests.

Suggested change
else:
# If posargs are provided, only run if they match async tests
# or just skip if they were already run in sync.
# For simplicity, we only run async folder if no posargs.
return
else:
# If posargs are provided, pass them to the async test run as well.
# pytest will filter and run only the relevant tests.
async_args.extend(session.posargs)


session.run(
*async_args,
env={
"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": protobuf_implementation,
"SPANNER_DATABASE_DIALECT": database_dialect,
Expand Down Expand Up @@ -551,6 +599,10 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
"google-cloud-testutils",
# dependencies of google-cloud-testutils"
"click",
# dependency of google-auth
"cffi",
"cryptography",
"cachetools",
]

for dep in prerel_deps:
Expand Down Expand Up @@ -589,6 +641,8 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
session.run(
"py.test",
"--verbose",
"-o",
"asyncio_mode=auto",
f"--junitxml=system_{session.python}_sponge_log.xml",
system_test_path,
*session.posargs,
Expand All @@ -602,6 +656,8 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
session.run(
"py.test",
"--verbose",
"-o",
"asyncio_mode=auto",
f"--junitxml=system_{session.python}_sponge_log.xml",
system_test_folder_path,
*session.posargs,
Expand All @@ -611,3 +667,10 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
"SKIP_BACKUP_TESTS": "true",
},
)


@nox.session(python=DEFAULT_PYTHON_VERSION)
def generate(session):
"""Regenerate synchronous code from asynchronous code."""
session.install("black", "autoflake")
session.run("python", ".cross_sync/generate.py", "google/cloud/spanner_v1")
3 changes: 2 additions & 1 deletion .librarian/state.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ libraries:
- ^google/cloud/spanner_v1/types
- ^google/cloud/spanner_admin_database_v1
- ^google/cloud/spanner_admin_instance_v1
- ^tests/unit/gapic
- ^tests/unit/gapic/spanner
- ^tests/unit/gapic/__init__.py
- ^tests/__init__.py
- ^tests/unit/__init__.py
- ^.pre-commit-config.yaml
Expand Down
2 changes: 0 additions & 2 deletions google/cloud/spanner_v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
BatchWriteRequest,
BatchWriteResponse,
BeginTransactionRequest,
ClientContext,
CommitRequest,
CreateSessionRequest,
DeleteSessionRequest,
Expand Down Expand Up @@ -124,7 +123,6 @@
"BatchWriteRequest",
"BatchWriteResponse",
"BeginTransactionRequest",
"ClientContext",
"CommitRequest",
"CommitResponse",
"CreateSessionRequest",
Expand Down
33 changes: 20 additions & 13 deletions google/cloud/spanner_v1/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
from google.rpc.error_details_pb2 import RetryInfo

from google.cloud._helpers import _date_from_iso8601_date
from google.cloud.spanner_v1.types import ClientContext
from google.cloud.spanner_v1.types import RequestOptions
from google.cloud.spanner_v1.data_types import JsonObject, Interval
from google.cloud.spanner_v1.exceptions import wrap_with_request_id
Expand Down Expand Up @@ -196,17 +195,17 @@ def _merge_query_options(base, merge):
def _merge_client_context(base, merge):
"""Merge higher precedence ClientContext with current ClientContext.

:type base: :class:`~google.cloud.spanner_v1.types.ClientContext`
:type base: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
or :class:`dict` or None
:param base: The current ClientContext that is intended for use.

:type merge: :class:`~google.cloud.spanner_v1.types.ClientContext`
:type merge: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
or :class:`dict` or None
:param merge:
The ClientContext that has a higher priority than base. These options
should overwrite the fields in base.

:rtype: :class:`~google.cloud.spanner_v1.types.ClientContext`
:rtype: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
or None
:returns:
ClientContext object formed by merging the two given ClientContexts.
Expand All @@ -215,15 +214,23 @@ def _merge_client_context(base, merge):
return None

# Avoid in-place modification of base
combined_pb = ClientContext()._pb
combined_pb = RequestOptions.ClientContext()._pb
if base:
base_pb = ClientContext(base)._pb if isinstance(base, dict) else base._pb
base_pb = (
RequestOptions.ClientContext(base)._pb
if isinstance(base, dict)
else base._pb
)
combined_pb.MergeFrom(base_pb)
if merge:
merge_pb = ClientContext(merge)._pb if isinstance(merge, dict) else merge._pb
merge_pb = (
RequestOptions.ClientContext(merge)._pb
if isinstance(merge, dict)
else merge._pb
)
combined_pb.MergeFrom(merge_pb)

combined = ClientContext(combined_pb)
combined = RequestOptions.ClientContext(combined_pb)

if not combined.secure_context:
return None
Expand All @@ -233,18 +240,18 @@ def _merge_client_context(base, merge):
def _validate_client_context(client_context):
"""Validate and convert client_context.

:type client_context: :class:`~google.cloud.spanner_v1.types.ClientContext`
:type client_context: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
or :class:`dict`
:param client_context: (Optional) Client context to use.

:rtype: :class:`~google.cloud.spanner_v1.types.ClientContext`
:rtype: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
:returns: Validated ClientContext object or None.
:raises TypeError: if client_context is not a ClientContext or a dict.
"""
if client_context is not None:
if isinstance(client_context, dict):
client_context = ClientContext(client_context)
elif not isinstance(client_context, ClientContext):
client_context = RequestOptions.ClientContext(client_context)
elif not isinstance(client_context, RequestOptions.ClientContext):
raise TypeError("client_context must be a ClientContext or a dict")
return client_context

Expand All @@ -256,7 +263,7 @@ def _merge_request_options(request_options, client_context):
or :class:`dict` or None
:param request_options: The current RequestOptions that is intended for use.

:type client_context: :class:`~google.cloud.spanner_v1.types.ClientContext`
:type client_context: :class:`~google.cloud.spanner_v1.types.RequestOptions.ClientContext`
or :class:`dict` or None
:param client_context:
The ClientContext to merge into request_options.
Expand Down
3 changes: 0 additions & 3 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# DO NOT EDIT THIS FILE OUTSIDE OF `.librarian/generator-input`
# The source of truth for this file is `.librarian/generator-input`


# Generated by synthtool. DO NOT EDIT!

Expand Down
Loading
Loading