diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 67eeb0cd5..d5fafbbb0 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -211,7 +211,7 @@ jobs: pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml \ --html=integration-test-report.html --self-contained-html \ --junit-xml=test-results.xml \ - -n 8 --dist loadscope \ + -n 4 --dist loadscope \ $(cat failed_tests.txt | tr '\n' ' ') else echo "::notice::First attempt or no previous failures - running full integration test suite" @@ -220,7 +220,7 @@ jobs: pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml \ --html=integration-test-report.html --self-contained-html \ --junit-xml=test-results.xml \ - tests/integration -n 8 $IGNORE_FLAGS --dist loadscope + tests/integration -n 4 $IGNORE_FLAGS --dist loadscope fi # Execute the CLI tests in a non-dist way because they were causing some test instability when being run concurrently diff --git a/.github/workflows/validate-release.yml b/.github/workflows/validate-release.yml index 40fdea282..04f4492d2 100644 --- a/.github/workflows/validate-release.yml +++ b/.github/workflows/validate-release.yml @@ -58,4 +58,4 @@ jobs: export EXTERNAL_S3_BUCKET_AWS_ACCESS_KEY_ID="${{secrets.EXTERNAL_S3_BUCKET_AWS_ACCESS_KEY_ID}}" export EXTERNAL_S3_BUCKET_AWS_SECRET_ACCESS_KEY="${{secrets.EXTERNAL_S3_BUCKET_AWS_SECRET_ACCESS_KEY}}" - pytest -sv --reruns 3 tests/integration -n 8 --ignore=tests/integration/synapseclient/test_command_line_client.py --dist loadscope + pytest -sv --reruns 3 tests/integration -n 4 --ignore=tests/integration/synapseclient/test_command_line_client.py --dist loadscope diff --git a/CLAUDE.md b/CLAUDE.md index ab390513d..514776837 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,7 +25,7 @@ pip install -e ".[boto3,pandas,pysftp,tests,curator,dev]" pytest -sv tests/unit # Integration tests (requires Synapse credentials, runs in parallel) -pytest -sv --reruns 3 tests/integration -n 8 --dist loadscope +pytest -sv --reruns 3 tests/integration -n 4 --dist loadscope # Pre-commit checks (ruff, black, isort, bandit) pre-commit run --all-files @@ -122,7 +122,7 @@ For type annotations referencing pandas types, use `DATA_FRAME_TYPE` and `SERIES - `asyncio_mode = auto` in pytest.ini — no need for `@pytest.mark.asyncio` - `asyncio_default_fixture_loop_scope = session` — all async tests share one event loop - Unit test client fixture: session-scoped, `skip_checks=True`, `cache_client=False` -- Integration tests use `--reruns 3` for flaky retries and `-n 8 --dist loadscope` for parallelism +- Integration tests use `--reruns 3` for flaky retries and `-n 4 --dist loadscope` for parallelism - Integration fixtures create per-worker Synapse projects; use `schedule_for_cleanup()` for teardown - Auth env vars: `SYNAPSE_AUTH_TOKEN` (bearer token), `SYNAPSE_PROFILE` (config file profile, default: `"default"`), `SYNAPSE_TOKEN_AWS_SSM_PARAMETER_NAME` (AWS SSM path) - CI runs integration tests only on Python 3.10 and 3.14 (oldest + newest) to limit Synapse server load diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 461b54122..6f09a69e7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -190,10 +190,10 @@ pytest -sv tests/unit # Integration tests (requires Synapse credentials, ~30-60 minutes) # Uses pytest-xdist for parallel execution with fixture-aware distribution -pytest -sv tests/integration -n 8 --dist loadscope +pytest -sv tests/integration -n 4 --dist loadscope # Integration tests excluding CLI tests (which must run serially) -pytest -sv tests/integration -n 8 --dist loadscope \ +pytest -sv tests/integration -n 4 --dist loadscope \ --ignore=tests/integration/synapseclient/test_command_line_client.py ``` @@ -218,9 +218,48 @@ fileHandleEndpoint=https://repo-dev.dev.sagebase.org/file/v1 ``` #### Running OpenTelemetry in Integration Tests -`tests/integration/conftest.py` is where we defining which trace exporter to use. Set the `SYNAPSE_OTEL_INTEGRATION_TEST_EXPORTER` environment variable to `otlp` or `console` depending on your use case. +`tests/integration/conftest.py`'s `setup_otel` fixture decides whether to enable OpenTelemetry +for the run. Set `SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED=true` to turn it on; only the values +`1`, `true`, `yes`, and `on` (case-insensitive) count, everything else is treated as off. -When integration tests are ran in the Github CI/CD pipeline it will upload the trace data automatically using OLTP. +``` +export SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED=true +export OTEL_EXPORTER_OTLP_ENDPOINT= +export OTEL_EXPORTER_OTLP_HEADERS= +``` + +To verify locally without a real collector, set `OTEL_DEBUG_CONSOLE=true` to print spans and +metrics to stdout **instead of** exporting them. This only produces output for a **serial** run +with `-s`: +``` +pytest -s tests/integration/ +``` +It does not work under pytest's default output capture (the console exporters flush after +pytest has already closed the captured stream) and it does not work under `-n` (each +pytest-xdist worker's stdout goes through `execnet` and is discarded, even with `-s`) — so it +cannot be used to verify the `-n 4 --dist loadscope` recipe below. For parallel verification, use +a real collector (`OTEL_EXPORTER_OTLP_ENDPOINT` / `OTEL_EXPORTER_OTLP_HEADERS`, no +`OTEL_DEBUG_CONSOLE`) and check the data landed there. + +A rejected export (e.g. malformed `OTEL_EXPORTER_OTLP_HEADERS`) does not fail the run on its +own — the exporter logs the error and pytest exits 0 regardless, so a run can finish "green" +having recorded nothing. `tests/integration/conftest.py` guards against this: it captures OTLP +export-rejection log records for the session (forwarding a worker's via `workeroutput` under +`-n` so the controller sees them too) and fails the session (non-zero exit, plus a terminal +summary line naming the rejection) if any export was rejected. Trust that exit code, not the +`passed`/`failed` count, to know whether telemetry was actually accepted. + +For a measurement run, set `SYNAPSE_TEST_RUN_LABEL` to a value that identifies the run (e.g. a +ticket number or date) so its data points can be grouped and compared against other runs; each +pytest-xdist worker gets its own `service.instance.id` so counts are not double-counted across +workers. Keep `--reruns 3` for measurement runs, same as any other run. + +The two metric instruments to query are `synapse.async_job.submissions` / +`synapse.async_job.duration` (async-job load, with `request_type` and `outcome` attributes) and +`synapse.file_handle.uploads` / `synapse.file_handle.upload.duration` (file-handle upload load). + +When integration tests are run in the GitHub CI/CD pipeline it will upload the trace and metric +data automatically using OTLP. #### Integration testing for external collaborators @@ -443,7 +482,8 @@ following set of guidelines should be followed: - `function` scope: Use for entities that tests **mutate** (e.g., files with changed names, datasets with added/removed items, submission statuses being updated). Each test gets a fresh entity. - All fixtures that create Synapse entities **must** call `schedule_for_cleanup()` to register them for cleanup at session end. - **Polling and retries:** For eventual-consistency scenarios (e.g., waiting for permission propagation, schema binding, attachment preview generation), use `wait_for_condition()` from `tests/integration/helpers.py` instead of hardcoded `asyncio.sleep()` calls. This uses exponential backoff and returns as soon as the condition is met. -- **Parallel execution:** Tests run with `pytest -n 8 --dist loadscope`, which ensures all tests in a class execute on the same worker sequentially. Session-scoped fixtures are shared within each worker. +- **Avoiding real uploads:** A `File(external_url=..., synapse_store=False)` file handle creates a real FileEntity without a real upload, and is the right default when a test only needs the entity to exist (e.g. as a parent, a copy/walk target, or a structure check). **Do not** use it for a test that reads the file back — `sync_from_synapse_async`, a real download, an md5 comparison, or a manifest/annotation/provenance round-trip all require real content on disk. +- **Parallel execution:** Tests run with `pytest -n 4 --dist loadscope`, which ensures all tests in a class execute on the same worker sequentially. Session-scoped fixtures are shared within each worker. ### Repository Admins diff --git a/synapseclient/core/otel_config.py b/synapseclient/core/otel_config.py index 57e4efc1b..cb276f164 100644 --- a/synapseclient/core/otel_config.py +++ b/synapseclient/core/otel_config.py @@ -1,6 +1,7 @@ """OpenTelemetry configuration for Synapse Python Client.""" import os +import platform import sys from typing import Any, Dict, List, Optional @@ -85,19 +86,19 @@ def force_flush(self, timeout_millis: int = 30000) -> None: """No-op method that does nothing when the span processor is forced to flush.""" -def configure_traces( +def _build_resource_attributes( resource_attributes: Optional[Dict[str, Any]] = None, include_context: bool = True, -) -> TracerProvider: +) -> Dict[str, Any]: """ - Configure OpenTelemetry tracing for the Synapse Python Client. + Build the resource attributes shared by the trace and metric providers. Args: resource_attributes: Additional resource attributes to include include_context: Whether to include contextual information about the runtime environment Returns: - The configured TracerProvider + The resource attributes to pass to `Resource.create` """ resource_attrs = { SERVICE_NAME: os.environ.get("OTEL_SERVICE_NAME", DEFAULT_SERVICE_NAME), @@ -112,7 +113,7 @@ def configure_traces( str(v) for v in sys.version_info[:3] ) - resource_attrs["os.type"] = os.name + resource_attrs["os.type"] = platform.system().lower() try: from synapseclient import __version__ as client_version @@ -124,7 +125,26 @@ def configure_traces( if resource_attributes: resource_attrs.update(resource_attributes) - resource = Resource.create(resource_attrs) + return resource_attrs + + +def configure_traces( + resource_attributes: Optional[Dict[str, Any]] = None, + include_context: bool = True, +) -> TracerProvider: + """ + Configure OpenTelemetry tracing for the Synapse Python Client. + + Args: + resource_attributes: Additional resource attributes to include + include_context: Whether to include contextual information about the runtime environment + + Returns: + The configured TracerProvider + """ + resource = Resource.create( + _build_resource_attributes(resource_attributes, include_context) + ) provider = TracerProvider(resource=resource) @@ -165,29 +185,9 @@ def configure_metrics( Returns: The configured MeterProvider """ - resource_attrs = { - SERVICE_NAME: os.environ.get("OTEL_SERVICE_NAME", DEFAULT_SERVICE_NAME), - SYNAPSE_SERVICE_VERSION: CLIENT_VERSION, - } - - if include_context: - resource_attrs["python.version"] = ".".join( - str(v) for v in sys.version_info[:3] - ) - - resource_attrs["os.type"] = os.name - - try: - from synapseclient import __version__ as client_version - - resource_attrs[SYNAPSE_SERVICE_VERSION] = client_version - except ImportError: - pass - - if resource_attributes: - resource_attrs.update(resource_attributes) - - resource = Resource.create(resource_attrs) + resource = Resource.create( + _build_resource_attributes(resource_attributes, include_context) + ) readers = [] diff --git a/synapseclient/core/upload/upload_functions_async.py b/synapseclient/core/upload/upload_functions_async.py index 92f636be4..56616bbe4 100644 --- a/synapseclient/core/upload/upload_functions_async.py +++ b/synapseclient/core/upload/upload_functions_async.py @@ -2,6 +2,7 @@ import asyncio import os +import time import urllib.parse as urllib_parse import uuid from typing import TYPE_CHECKING, Dict, Optional, Union @@ -19,7 +20,7 @@ from synapseclient.core import sts_transfer, utils from synapseclient.core.constants import concrete_types from synapseclient.core.exceptions import SynapseMd5MismatchError -from synapseclient.core.otel_config import get_tracer +from synapseclient.core.otel_config import get_meter, get_tracer from synapseclient.core.remote_file_storage_wrappers import S3ClientWrapper, SFTPWrapper from synapseclient.core.upload.multipart_upload_async import multipart_upload_file_async from synapseclient.core.utils import as_url, file_url_to_path, id_of, is_url @@ -28,6 +29,11 @@ from synapseclient import Synapse tracer = get_tracer() +meter = get_meter() +_upload_counter = meter.create_counter("synapse.file_handle.uploads") +_upload_duration = meter.create_histogram( + "synapse.file_handle.upload.duration", unit="s" +) @tracer.start_as_current_span("synapse.transfer.upload") @@ -66,125 +72,136 @@ async def upload_file_handle( if path is None: raise ValueError("path can not be None") - span = trace.get_current_span() - span.set_attribute("synapse.transfer.direction", "upload") - span.set_attribute("synapse.operation.category", "file_transfer") - - # if doing a external file handle with no actual upload - if not synapse_store: - file_handle = await create_external_file_handle( - syn, path, mimetype=mimetype, md5=md5, file_size=file_size - ) - span.set_attribute("synapse.file_handle_id", file_handle.get("id")) - return file_handle - - # expand the path because past this point an upload is required and some upload functions require an absolute path - expanded_upload_path = os.path.expandvars(os.path.expanduser(path)) + attributes = {"external_file_handle": not synapse_store} + _upload_counter.add(1, attributes) + started = time.monotonic() + try: + span = trace.get_current_span() + span.set_attribute("synapse.transfer.direction", "upload") + span.set_attribute("synapse.operation.category", "file_transfer") + span.set_attribute("synapse.file_handle.external", not synapse_store) + + # if doing a external file handle with no actual upload + if not synapse_store: + file_handle = await create_external_file_handle( + syn, path, mimetype=mimetype, md5=md5, file_size=file_size + ) + span.set_attribute("synapse.file_handle_id", file_handle.get("id")) + return file_handle - if md5 is None and os.path.isfile(expanded_upload_path): - md5 = utils.md5_for_file_hex(filename=expanded_upload_path) + # expand the path because past this point an upload is required and some upload functions require an absolute path + expanded_upload_path = os.path.expandvars(os.path.expanduser(path)) - entity_parent_id = id_of(parent_entity_id) + if md5 is None and os.path.isfile(expanded_upload_path): + md5 = utils.md5_for_file_hex(filename=expanded_upload_path) - # determine the upload function based on the UploadDestination - location = await get_upload_destination( - entity_id=entity_parent_id, synapse_client=syn - ) - upload_destination_type = location.get("concreteType", None) if location else None + entity_parent_id = id_of(parent_entity_id) - if ( - sts_transfer.is_boto_sts_transfer_enabled(syn) - and await sts_transfer.is_storage_location_sts_enabled_async( - syn, entity_parent_id, location + # determine the upload function based on the UploadDestination + location = await get_upload_destination( + entity_id=entity_parent_id, synapse_client=syn ) - and upload_destination_type == concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION - ): - file_handle = await upload_synapse_sts_boto_s3( - syn=syn, - parent_id=entity_parent_id, - upload_destination=location, - local_path=expanded_upload_path, - mimetype=mimetype, - md5=md5, - storage_str="Uploading file to external S3 storage using boto3", + upload_destination_type = ( + location.get("concreteType", None) if location else None ) - elif upload_destination_type in ( - concrete_types.SYNAPSE_S3_UPLOAD_DESTINATION, - concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION, - concrete_types.EXTERNAL_GCP_UPLOAD_DESTINATION, - ): - if upload_destination_type == concrete_types.SYNAPSE_S3_UPLOAD_DESTINATION: - storage_str = "Uploading to Synapse storage" - span.set_attribute("synapse.storage.provider", "s3") - elif upload_destination_type == concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION: - storage_str = "Uploading to your external S3 storage" - span.set_attribute("synapse.storage.provider", "s3") - else: - storage_str = "Uploading to your external Google Bucket storage" - span.set_attribute("synapse.storage.provider", "gcs") - file_handle = await upload_synapse_s3( - syn=syn, - file_path=expanded_upload_path, - storage_location_id=location["storageLocationId"], - mimetype=mimetype, - md5=md5, - storage_str=storage_str, - ) - # external file handle (sftp) - elif upload_destination_type == concrete_types.EXTERNAL_UPLOAD_DESTINATION: - if location["uploadType"] == "SFTP": + + if ( + sts_transfer.is_boto_sts_transfer_enabled(syn) + and await sts_transfer.is_storage_location_sts_enabled_async( + syn, entity_parent_id, location + ) + and upload_destination_type == concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION + ): + file_handle = await upload_synapse_sts_boto_s3( + syn=syn, + parent_id=entity_parent_id, + upload_destination=location, + local_path=expanded_upload_path, + mimetype=mimetype, + md5=md5, + storage_str="Uploading file to external S3 storage using boto3", + ) + elif upload_destination_type in ( + concrete_types.SYNAPSE_S3_UPLOAD_DESTINATION, + concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION, + concrete_types.EXTERNAL_GCP_UPLOAD_DESTINATION, + ): + if upload_destination_type == concrete_types.SYNAPSE_S3_UPLOAD_DESTINATION: + storage_str = "Uploading to Synapse storage" + span.set_attribute("synapse.storage.provider", "s3") + elif ( + upload_destination_type == concrete_types.EXTERNAL_S3_UPLOAD_DESTINATION + ): + storage_str = "Uploading to your external S3 storage" + span.set_attribute("synapse.storage.provider", "s3") + else: + storage_str = "Uploading to your external Google Bucket storage" + span.set_attribute("synapse.storage.provider", "gcs") + file_handle = await upload_synapse_s3( + syn=syn, + file_path=expanded_upload_path, + storage_location_id=location["storageLocationId"], + mimetype=mimetype, + md5=md5, + storage_str=storage_str, + ) + # external file handle (sftp) + elif upload_destination_type == concrete_types.EXTERNAL_UPLOAD_DESTINATION: + if location["uploadType"] == "SFTP": + storage_str = ( + f"Uploading to: {urllib_parse.urlparse(location['url']).netloc}" + ) + banner = location.get("banner", None) + if banner: + syn.logger.info(banner) + file_handle = await upload_external_file_handle_sftp( + syn=syn, + file_path=expanded_upload_path, + sftp_url=location["url"], + mimetype=mimetype, + md5=md5, + storage_str=storage_str, + ) + else: + raise NotImplementedError("Can only handle SFTP upload locations.") + # client authenticated S3 + elif ( + upload_destination_type + == concrete_types.EXTERNAL_OBJECT_STORE_UPLOAD_DESTINATION + ): storage_str = ( - f"Uploading to: {urllib_parse.urlparse(location['url']).netloc}" + f"Uploading to endpoint: [{location.get('endpointUrl')}] " + f"bucket: [{location.get('bucket')}]" ) banner = location.get("banner", None) if banner: syn.logger.info(banner) - file_handle = await upload_external_file_handle_sftp( + file_handle = await upload_client_auth_s3( syn=syn, file_path=expanded_upload_path, - sftp_url=location["url"], + bucket=location["bucket"], + endpoint_url=location["endpointUrl"], + key_prefix=location["keyPrefixUUID"], + storage_location_id=location["storageLocationId"], mimetype=mimetype, md5=md5, storage_str=storage_str, ) - else: - raise NotImplementedError("Can only handle SFTP upload locations.") - # client authenticated S3 - elif ( - upload_destination_type - == concrete_types.EXTERNAL_OBJECT_STORE_UPLOAD_DESTINATION - ): - storage_str = ( - f"Uploading to endpoint: [{location.get('endpointUrl')}] " - f"bucket: [{location.get('bucket')}]" - ) - banner = location.get("banner", None) - if banner: - syn.logger.info(banner) - file_handle = await upload_client_auth_s3( - syn=syn, - file_path=expanded_upload_path, - bucket=location["bucket"], - endpoint_url=location["endpointUrl"], - key_prefix=location["keyPrefixUUID"], - storage_location_id=location["storageLocationId"], - mimetype=mimetype, - md5=md5, - storage_str=storage_str, - ) - else: # unknown storage location - span.set_attribute("synapse.storage.provider", "s3") - file_handle = await upload_synapse_s3( - syn=syn, - file_path=expanded_upload_path, - storage_location_id=None, - mimetype=mimetype, - md5=md5, - storage_str="Uploading to Synapse storage", - ) + else: # unknown storage location + span.set_attribute("synapse.storage.provider", "s3") + file_handle = await upload_synapse_s3( + syn=syn, + file_path=expanded_upload_path, + storage_location_id=None, + mimetype=mimetype, + md5=md5, + storage_str="Uploading to Synapse storage", + ) - span.set_attribute("synapse.file_handle_id", file_handle.get("id")) - return file_handle + span.set_attribute("synapse.file_handle_id", file_handle.get("id")) + return file_handle + finally: + _upload_duration.record(time.monotonic() - started, attributes) async def create_external_file_handle( diff --git a/synapseclient/models/mixins/asynchronous_job.py b/synapseclient/models/mixins/asynchronous_job.py index 31ed18182..bda6dcdc4 100644 --- a/synapseclient/models/mixins/asynchronous_job.py +++ b/synapseclient/models/mixins/asynchronous_job.py @@ -31,8 +31,14 @@ SynapseHTTPError, SynapseTimeoutError, ) +from synapseclient.core.otel_config import get_meter, get_tracer from synapseclient.core.transfer_bar import create_progress_bar +tracer = get_tracer() +meter = get_meter() +_async_job_counter = meter.create_counter("synapse.async_job.submissions") +_async_job_duration = meter.create_histogram("synapse.async_job.duration", unit="s") + ASYNC_JOB_URIS = { AGENT_CHAT_REQUEST: "/agent/chat/async", CREATE_GRID_REQUEST: "/grid/session/async", @@ -338,40 +344,56 @@ async def send_job_and_wait_async( SynapseError: If the job fails. SynapseTimeoutError: If the job does not complete within the timeout. """ - start_time = time.time() - retry_interval = 5 # Retry every 5 seconds - max_wait_time = timeout * 5 # Maximum total wait time of 5 minutes - - while time.time() - start_time < max_wait_time: + attributes = {"request_type": request_type, "outcome": "success"} + with tracer.start_as_current_span("synapse.async_job") as span: + span.set_attribute("synapse.async_job.request_type", request_type) + started = time.monotonic() try: - job_id = await send_job_async( - request=request, synapse_client=synapse_client + start_time = time.time() + retry_interval = 5 # Retry every 5 seconds + max_wait_time = timeout * 5 # Maximum total wait time of 5x the timeout + + while time.time() - start_time < max_wait_time: + try: + job_id = await send_job_async( + request=request, synapse_client=synapse_client + ) + result = { + "jobId": job_id, + **await get_job_async( + job_id=job_id, + request_type=request_type, + synapse_client=synapse_client, + endpoint=endpoint, + timeout=timeout, + request=request, + ), + } + return result + except SynapseHTTPError as e: + if ( + "You cannot create a version of a view that is not available (Status: PROCESSING)" + in str(e) + ): + if time.time() - start_time < max_wait_time: + await asyncio.sleep(retry_interval) + continue + raise # Re-raise any other SynapseHTTPError or if max wait time reached + except Exception: + raise # Re-raise any other exceptions + + raise SynapseError( + f"Failed to create view version after {max_wait_time} seconds" ) - result = { - "jobId": job_id, - **await get_job_async( - job_id=job_id, - request_type=request_type, - synapse_client=synapse_client, - endpoint=endpoint, - timeout=timeout, - request=request, - ), - } - return result - except SynapseHTTPError as e: - if ( - "You cannot create a version of a view that is not available (Status: PROCESSING)" - in str(e) - ): - if time.time() - start_time < max_wait_time: - await asyncio.sleep(retry_interval) - continue - raise # Re-raise any other SynapseHTTPError or if max wait time reached + except SynapseTimeoutError: + attributes["outcome"] = "timeout" + raise except Exception: - raise # Re-raise any other exceptions - - raise SynapseError(f"Failed to create view version after {max_wait_time} seconds") + attributes["outcome"] = "error" + raise + finally: + _async_job_counter.add(1, attributes) + _async_job_duration.record(time.monotonic() - started, attributes) async def send_job_async( diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 39e0e459e..ad1ae36fb 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -1,4 +1,4 @@ - + ## Project @@ -26,14 +26,14 @@ Use `pytest.mark.parametrize` when possible to merge similar tests into one test - `schedule_for_cleanup(item)` — defer entity/file cleanup to session teardown. Always use this instead of inline deletion. Cleanup list is reversed before execution for dependency ordering (children deleted before parents). - Use shared resources when possible via fixtures in `conftest.py` files (e.g., `project_model`, `project`). Refer to existing integration tests for the pattern. - Per-worker project fixtures (`project_model`, `project`) created during session setup -- `--reruns 3` for flaky retry, `-n 8 --dist loadscope` for parallelism -- OpenTelemetry tracing opt-in via `SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED` env var +- `--reruns 3` for flaky retry, `-n 4 --dist loadscope` for parallelism +- OpenTelemetry tracing and metrics opt-in via `SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED` (strict truthiness: only `1`/`true`/`yes`/`on`, case-insensitive). Each pytest-xdist worker gets a distinct `service.instance.id` so per-worker counters don't collide. - Two client fixtures: `syn` (silent logger) and `syn_with_logger` (verbose) - conftest.py locations: `tests/unit/conftest.py` (session client, socket blocking, UTC timezone), `tests/integration/conftest.py` (logged-in client, per-worker projects, cleanup fixture) ### Test utilities - `tests/test_utils.py`: `spy_for_async_function(original_func)` — wraps async function for pytest-mock spying while preserving async behavior. `spy_for_function(original_func)` — sync variant. -- `tests/integration/helpers.py`: `wait_for_condition(condition_fn, timeout_seconds=60)` — async polling helper with exponential backoff. Accepts sync or async condition functions. +- `tests/integration/helpers.py`: `wait_for_condition(condition_fn, timeout_seconds=60)` — async polling helper with exponential backoff. Accepts sync or async condition functions. `telemetry_enabled(env)` and `worker_telemetry_env(env)` — pure helpers behind `setup_otel` in `conftest.py` for the OTel truthiness check and per-worker `OTEL_SERVICE_INSTANCE_ID`/`OTEL_RESOURCE_ATTRIBUTES` values. - `tests/integration/__init__.py`: `QUERY_TIMEOUT_SEC = 600`, `ASYNC_JOB_TIMEOUT_SEC = 600` - Test data generators in production code: `core/utils.py` has `make_bogus_data_file()`, `make_bogus_binary_file(n)`, `make_bogus_uuid_file()` diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 25b466792..ec2348789 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -11,7 +11,7 @@ import pytest import pytest_asyncio -from opentelemetry import trace +from opentelemetry import metrics, trace from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.sampling import ALWAYS_OFF from pytest_asyncio import is_async_test @@ -31,6 +31,13 @@ WikiPage, ) from synapseclient.operations import delete_async +from tests.integration.helpers import ( + OTLP_EXPORTER_LOGGER, + ExportFailureRecorder, + export_failure_summary, + telemetry_enabled, + worker_telemetry_env, +) tracer = trace.get_tracer("synapseclient") working_directory = tempfile.mkdtemp(prefix="someTestFolder") @@ -194,20 +201,74 @@ async def _cleanup(syn: Synapse, items): ) -active_span_processors = [] - - @pytest.fixture(scope="session", autouse=True) -def setup_otel(): +def setup_otel(request): """ Handles setting up the OpenTelemetry tracer provider for integration tests. + + When telemetry is enabled, also attaches an `ExportFailureRecorder` to the + OTLP exporters' logger so a rejected export (e.g. a 401 from a malformed + `OTEL_EXPORTER_OTLP_HEADERS`) is captured. `pytest_sessionfinish` below turns + a captured failure into a non-zero exit code, since the exporters otherwise + just log and swallow the error, leaving pytest itself green (feedback 0010). """ - # Setup - tests_enabled = os.environ.get("SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED", False) - if tests_enabled: - Synapse.enable_open_telemetry() + if telemetry_enabled(os.environ): + os.environ.update(worker_telemetry_env(os.environ)) + Synapse.enable_open_telemetry(enable_open_telemetry_metrics=True) + + recorder = ExportFailureRecorder() + exporter_logger = logging.getLogger(OTLP_EXPORTER_LOGGER) + exporter_logger.addHandler(recorder) + + yield + + tracer_provider = trace.get_tracer_provider() + if hasattr(tracer_provider, "force_flush"): + tracer_provider.force_flush(timeout_millis=30_000) + meter_provider = metrics.get_meter_provider() + if hasattr(meter_provider, "force_flush"): + meter_provider.force_flush(timeout_millis=30_000) + + exporter_logger.removeHandler(recorder) + request.session.config._otel_export_failures = recorder.messages else: trace.set_tracer_provider(TracerProvider(sampler=ALWAYS_OFF)) + yield + + +@pytest.hookimpl(tryfirst=True) +def pytest_sessionfinish(session, exitstatus): + """A rejected OTLP export must fail the run even though pytest itself exits + 0 for it (feedback 0010). + + On an xdist worker, forward the captured failures to the controller via + `workeroutput` (picked up by `pytest_testnodedown` below). On the + controller or in a serial run, fail the session if any failures were + captured directly or forwarded from a worker. + """ + messages = getattr(session.config, "_otel_export_failures", []) + workeroutput = getattr(session.config, "workeroutput", None) + if workeroutput is not None: + workeroutput["otel_export_failures"] = messages + return + if messages: + session.exitstatus = 1 + + +def pytest_testnodedown(node, error): + """Collect a worker's forwarded OTLP export failures back on the controller.""" + messages = (getattr(node, "workeroutput", None) or {}).get("otel_export_failures") + if messages: + node.config._otel_export_failures = ( + getattr(node.config, "_otel_export_failures", []) + messages + ) + + +def pytest_terminal_summary(terminalreporter, exitstatus, config): + """Print a summary line naming any captured OTLP export failures.""" + summary = export_failure_summary(getattr(config, "_otel_export_failures", [])) + if summary: + terminalreporter.write_line(f"OTEL export rejected: {summary}", red=True) @pytest.fixture(autouse=True) @@ -220,9 +281,5 @@ def set_timezone(): @pytest.fixture(autouse=True, scope="function") def wrap_with_otel(request): """Start a new OTEL Span for each test function.""" - with tracer.start_as_current_span(request.node.name) as span: - try: - yield - finally: - for processor in active_span_processors: - processor.force_flush() + with tracer.start_as_current_span(request.node.nodeid): + yield diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 53cab0b31..9df66d997 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -2,12 +2,106 @@ import asyncio import logging -from typing import Any, Awaitable, Callable, Optional, TypeVar, Union +from typing import Awaitable, Callable, Dict, List, Mapping, Optional, TypeVar, Union logger = logging.getLogger(__name__) T = TypeVar("T") +_TELEMETRY_TRUTHY_VALUES = ("1", "true", "yes", "on") + +# Logger name used by the OTLP exporters to report rejected exports (e.g. a 401 +# from a malformed `OTEL_EXPORTER_OTLP_HEADERS`). +OTLP_EXPORTER_LOGGER = "opentelemetry.exporter.otlp.proto.http" + + +class ExportFailureRecorder(logging.Handler): + """Captures ERROR-level log records emitted by the OTLP exporters, so a + rejected export can fail the run instead of leaving it silently green. + """ + + def __init__(self) -> None: + super().__init__(level=logging.ERROR) + self.messages: List[str] = [] + + def emit(self, record: logging.LogRecord) -> None: + self.messages.append(record.getMessage()) + + +def export_failure_summary(messages: List[str]) -> Optional[str]: + """Build a one-line summary of captured OTLP export failures. + + Args: + messages: The messages captured by `ExportFailureRecorder`. + + Returns: + None if messages is empty, otherwise a string naming the count and the + first message. + """ + if not messages: + return None + return f"{len(messages)} OTLP export failure(s); first: {messages[0]}" + + +def telemetry_enabled(env: Mapping[str, str]) -> bool: + """Whether integration-test OpenTelemetry export is enabled. + + Args: + env: The environment to check, typically `os.environ`. + + Returns: + True if `SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED` is `1`/`true`/`yes`/`on` + (case-insensitive), False otherwise. + """ + value = env.get("SYNAPSE_INTEGRATION_TEST_OTEL_ENABLED", "") + return value.strip().lower() in _TELEMETRY_TRUTHY_VALUES + + +def worker_telemetry_env(env: Mapping[str, str]) -> Dict[str, str]: + """Build the env-var deltas that give each pytest-xdist worker a distinct + OpenTelemetry resource identity. + + Args: + env: The environment to derive the deltas from, typically `os.environ`. + + Returns: + A dict of env vars to apply via `os.environ.update(...)`: + `OTEL_SERVICE_INSTANCE_ID` (only present if there is a worker id or an + existing base value to preserve) and `OTEL_RESOURCE_ATTRIBUTES` (always + present, appended to any existing value rather than overwriting it). + """ + worker_id = env.get("PYTEST_XDIST_WORKER") + base_instance_id = env.get("OTEL_SERVICE_INSTANCE_ID") + if worker_id: + service_instance_id = ( + f"{base_instance_id}-{worker_id}" if base_instance_id else worker_id + ) + else: + service_instance_id = base_instance_id + + resource_attribute_parts = [ + f"run.label={env.get('SYNAPSE_TEST_RUN_LABEL') or 'unlabeled'}" + ] + git_sha = env.get("GITHUB_SHA") + if git_sha: + resource_attribute_parts.append(f"git.sha={git_sha}") + worker_count = env.get("PYTEST_XDIST_WORKER_COUNT") + if worker_count: + resource_attribute_parts.append(f"xdist.workers={worker_count}") + + new_resource_attributes = ",".join(resource_attribute_parts) + existing_resource_attributes = env.get("OTEL_RESOURCE_ATTRIBUTES") + resource_attributes = ( + f"{existing_resource_attributes},{new_resource_attributes}" + if existing_resource_attributes + else new_resource_attributes + ) + + result = {"OTEL_RESOURCE_ATTRIBUTES": resource_attributes} + if service_instance_id: + result["OTEL_SERVICE_INSTANCE_ID"] = service_instance_id + return result + async def wait_for_condition( condition_fn: Callable[[], Union[Awaitable[T], T]], diff --git a/tests/integration/synapseclient/models/async/test_activity_async.py b/tests/integration/synapseclient/models/async/test_activity_async.py index 7e3429b9e..64efcffb8 100644 --- a/tests/integration/synapseclient/models/async/test_activity_async.py +++ b/tests/integration/synapseclient/models/async/test_activity_async.py @@ -6,7 +6,6 @@ import pytest -import synapseclient.core.utils as utils from synapseclient import Synapse from synapseclient.models import Activity, File, Project, UsedEntity, UsedURL @@ -28,14 +27,15 @@ async def create_file_with_activity( store_file: bool = True, ) -> File: """Helper to create a file with optional activity""" - path = utils.make_bogus_uuid_file() + # Only the entity's existence matters here, not its content, so an + # external_url file handle avoids a real upload. file = File( parent_id=project_model.id, - path=path, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name=f"bogus_file_{str(uuid.uuid4())}", activity=activity, ) - self.schedule_for_cleanup(file.path) if store_file: await file.store_async(synapse_client=self.syn) diff --git a/tests/integration/synapseclient/models/async/test_agent_async.py b/tests/integration/synapseclient/models/async/test_agent_async.py index d95203f3d..355087d2b 100644 --- a/tests/integration/synapseclient/models/async/test_agent_async.py +++ b/tests/integration/synapseclient/models/async/test_agent_async.py @@ -1,5 +1,8 @@ """Integration tests for the asynchronous methods of the AgentPrompt, AgentSession, and Agent classes.""" +import asyncio +from typing import Any, Awaitable + import pytest from synapseclient import Synapse @@ -10,7 +13,6 @@ AgentSession, AgentSessionAccessLevel, ) -from tests.integration import ASYNC_JOB_TIMEOUT_SEC # These are the ID values for a "Hello World" agent registered on Synapse. # The Bedrock agent is hosted on Sage Bionetworks AWS infrastructure. @@ -19,6 +21,22 @@ AGENT_AWS_ID = "QOTV3KQM1X" +# The client's job timeout is a no-progress budget rather than a total one: +# `get_job_async` resets its clock on every progress message, so a job that keeps +# reporting progress never times out. The agent answering these prompts is hosted +# outside this repository, so each prompt also carries a hard wall-clock deadline. +AGENT_PROMPT_TIMEOUT_SEC = 120 +AGENT_PROMPT_DEADLINE_SEC = 300 + +# An agent that will not answer is an outage rather than flake: a retry cannot make +# it respond, and each one costs another full deadline. +pytestmark = pytest.mark.flaky(reruns=0) + + +async def within_deadline(prompt: Awaitable[Any]) -> Any: + """Bound a prompt's total wall clock, not only the stalls between progress.""" + return await asyncio.wait_for(prompt, timeout=AGENT_PROMPT_DEADLINE_SEC) + class TestAgentPrompt: """Integration tests for the synchronous methods of the AgentPrompt class.""" @@ -44,9 +62,12 @@ async def test_send_job_and_wait_async_with_post_exchange_args(self) -> None: ).start_async(synapse_client=self.syn) test_prompt.session_id = test_session.id # WHEN I send the job and wait for it to complete - await test_prompt.send_job_and_wait_async( - post_exchange_args={"newer_than": 0}, - synapse_client=self.syn, + await within_deadline( + test_prompt.send_job_and_wait_async( + post_exchange_args={"newer_than": 0}, + timeout=AGENT_PROMPT_TIMEOUT_SEC, + synapse_client=self.syn, + ) ) # THEN I expect the AgentPrompt to be updated with the response and trace assert test_prompt.response is not None @@ -122,11 +143,13 @@ async def test_prompt(self) -> None: # WHEN I start a session await agent_session.start_async(synapse_client=self.syn) # THEN I expect to be able to prompt the agent - await agent_session.prompt_async( - prompt="hello", - enable_trace=True, - timeout=ASYNC_JOB_TIMEOUT_SEC, - synapse_client=self.syn, + await within_deadline( + agent_session.prompt_async( + prompt="hello", + enable_trace=True, + timeout=AGENT_PROMPT_TIMEOUT_SEC, + synapse_client=self.syn, + ) ) # AND I expect the chat history to be updated with the prompt and response assert len(agent_session.chat_history) == 1 @@ -216,12 +239,14 @@ async def test_prompt_with_session(self) -> None: agent_registration_id=self.AGENT_REGISTRATION_ID ).start_async(synapse_client=self.syn) # WHEN I prompt the agent with a session - await agent.prompt_async( - prompt="hello", - enable_trace=True, - session=session, - timeout=ASYNC_JOB_TIMEOUT_SEC, - synapse_client=self.syn, + await within_deadline( + agent.prompt_async( + prompt="hello", + enable_trace=True, + session=session, + timeout=AGENT_PROMPT_TIMEOUT_SEC, + synapse_client=self.syn, + ) ) test_session = agent.sessions[session.id] # THEN I expect the chat history to be updated with the prompt and response @@ -239,11 +264,13 @@ async def test_prompt_no_session(self) -> None: ) # WHEN I prompt the agent without a current session set # and no session provided - await agent.prompt_async( - prompt="hello", - enable_trace=True, - timeout=ASYNC_JOB_TIMEOUT_SEC, - synapse_client=self.syn, + await within_deadline( + agent.prompt_async( + prompt="hello", + enable_trace=True, + timeout=AGENT_PROMPT_TIMEOUT_SEC, + synapse_client=self.syn, + ) ) # THEN I expect a new session to be started and set as the current session assert agent.current_session is not None diff --git a/tests/integration/synapseclient/models/async/test_curation_async.py b/tests/integration/synapseclient/models/async/test_curation_async.py index 77bbc7934..61070f0c4 100644 --- a/tests/integration/synapseclient/models/async/test_curation_async.py +++ b/tests/integration/synapseclient/models/async/test_curation_async.py @@ -788,14 +788,42 @@ async def test_create_grid_session_async( class TestCurationTaskSetActiveGridSessionAsync: """Tests for the CurationTask.set_active_grid_session_async method.""" - @pytest.fixture(scope="function") + @pytest.fixture(scope="class") + async def folder_with_view( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> tuple[Folder, EntityView]: + """Create a folder with an associated EntityView, shared by every test in + this class. Every consumer only reads folder.id/entity_view.id; none of them + mutates the folder or entity view.""" + folder = await Folder( + name=str(uuid.uuid4()), + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) + + entity_view = await EntityView( + name=str(uuid.uuid4()), + parent_id=project_model.id, + scope_ids=[folder.id], + view_type_mask=ViewTypeMask.FILE.value, + ).store_async(synapse_client=syn) + schedule_for_cleanup(entity_view.id) + + return folder, entity_view + + @pytest.fixture(scope="class") async def grid( self, syn: Synapse, folder_with_view: tuple[Folder, EntityView], request: pytest.FixtureRequest, ) -> Grid: - """Create a Grid backed by the entity view; delete it after the test.""" + """Create a Grid backed by the entity view, shared by every test in this + class. Every consumer only reads grid.session_id; none of them mutates the + grid itself.""" _, entity_view = folder_with_view grid = await Grid( initial_query=Query(sql=f"SELECT * FROM {entity_view.id}") diff --git a/tests/integration/synapseclient/models/async/test_dataset_async.py b/tests/integration/synapseclient/models/async/test_dataset_async.py index 44f90896b..139a1d74f 100644 --- a/tests/integration/synapseclient/models/async/test_dataset_async.py +++ b/tests/integration/synapseclient/models/async/test_dataset_async.py @@ -5,7 +5,6 @@ import pytest from synapseclient import Synapse -from synapseclient.core import utils from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import ( Column, @@ -59,10 +58,11 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: def create_file_instance(self) -> File: """Helper to create a file instance""" - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) + # Only the file's existence as a dataset item matters here, not its + # content, so an external_url file handle avoids a real upload. return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description=DESCRIPTION_FILE, content_type=CONTENT_TYPE, ) @@ -426,42 +426,72 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - def create_file_instance(self) -> File: - """Helper to create a file instance""" - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - return File( - path=filename, - description=DESCRIPTION_FILE, - content_type=CONTENT_TYPE, - ) - - async def create_dataset( - self, project_model: Project, has_file: bool = False - ) -> Dataset: - """Helper to create a dataset""" - dataset = Dataset( - name=str(uuid.uuid4()), - description="Test dataset", - parent_id=project_model.id, - ) - - if has_file: - file = self.create_file_instance() + @pytest.fixture(scope="class") + async def shared_datasets_with_file( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> List[Dataset]: + """Two datasets, each with one item, built once for the class. + test_dataset_collection_lifecycle and test_dataset_collection_queries only + read id/version_number off these datasets to build EntityRefs and collection + rows -- neither mutates the dataset entities themselves. Note: + test_dataset_collection_queries writes a 'my_annotation' annotation onto + shared_datasets_with_file[0] via its collection's view update, but no other + test's collection defines that column, so the write is unobservable + elsewhere.""" + datasets = [] + for _ in range(2): + file = File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + description=DESCRIPTION_FILE, + content_type=CONTENT_TYPE, + ) stored_file = await file.store_async( - parent=project_model, synapse_client=self.syn + parent=project_model, synapse_client=syn ) - await dataset.add_item_async(stored_file, synapse_client=self.syn) - - dataset = await dataset.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(dataset.id) - return dataset - async def test_dataset_collection_lifecycle(self, project_model: Project) -> None: + dataset = Dataset( + name=str(uuid.uuid4()), + description="Test dataset", + parent_id=project_model.id, + ) + await dataset.add_item_async(stored_file, synapse_client=syn) + dataset = await dataset.store_async(synapse_client=syn) + schedule_for_cleanup(dataset.id) + datasets.append(dataset) + return datasets + + @pytest.fixture(scope="class") + async def shared_datasets_without_file( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> List[Dataset]: + """Two empty datasets built once for the class; used only by + test_dataset_collection_versioning as items in its per-test collection. + Read-only -- only id/version_number are read off them.""" + datasets = [] + for _ in range(2): + dataset = Dataset( + name=str(uuid.uuid4()), + description="Test dataset", + parent_id=project_model.id, + ) + dataset = await dataset.store_async(synapse_client=syn) + schedule_for_cleanup(dataset.id) + datasets.append(dataset) + return datasets + + async def test_dataset_collection_lifecycle( + self, project_model: Project, shared_datasets_with_file: List[Dataset] + ) -> None: """Test creating, updating, and deleting a DatasetCollection""" # GIVEN two datasets - dataset1 = await self.create_dataset(project_model, has_file=True) - dataset2 = await self.create_dataset(project_model, has_file=True) + dataset1, dataset2 = shared_datasets_with_file # WHEN I create a DatasetCollection with the first dataset collection = DatasetCollection( @@ -522,10 +552,12 @@ async def test_dataset_collection_lifecycle(self, project_model: Project) -> Non ): await DatasetCollection(id=collection.id).get_async(synapse_client=self.syn) - async def test_dataset_collection_queries(self, project_model: Project) -> None: + async def test_dataset_collection_queries( + self, project_model: Project, shared_datasets_with_file: List[Dataset] + ) -> None: """Test querying DatasetCollections with various part masks""" # GIVEN a dataset and a collection with that dataset - dataset = await self.create_dataset(project_model=project_model, has_file=True) + dataset = shared_datasets_with_file[0] collection = DatasetCollection( name=str(uuid.uuid4()), @@ -665,11 +697,12 @@ async def test_dataset_collection_columns(self, project_model: Project) -> None: assert second_col not in updated.columns assert new_name in updated.columns - async def test_dataset_collection_versioning(self, project_model: Project) -> None: + async def test_dataset_collection_versioning( + self, project_model: Project, shared_datasets_without_file: List[Dataset] + ) -> None: """Test versioning of DatasetCollections""" # GIVEN a DatasetCollection and datasets - dataset1 = await self.create_dataset(project_model) - dataset2 = await self.create_dataset(project_model) + dataset1, dataset2 = shared_datasets_without_file collection = DatasetCollection( name=str(uuid.uuid4()), diff --git a/tests/integration/synapseclient/models/async/test_docker_async.py b/tests/integration/synapseclient/models/async/test_docker_async.py index e61c0e70c..a6249d1aa 100644 --- a/tests/integration/synapseclient/models/async/test_docker_async.py +++ b/tests/integration/synapseclient/models/async/test_docker_async.py @@ -16,16 +16,15 @@ class TestDockerRepositoryAsync: async def readonly_docker_repo( self, schedule_for_cleanup: Callable[..., None], + project_model: Project, syn: Synapse, ) -> DockerRepository: - """Class-scoped fixture for read-only tests. Do not modify or delete.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - + """Class-scoped fixture for read-only tests. Do not modify or delete. + Shares the session-scoped project_model instead of creating its own + Project.""" docker_repo = DockerRepository( - parent_id=project.id, repository_name="username/test-async-readonly" + parent_id=project_model.id, + repository_name=f"username/test-async-readonly-{uuid.uuid4().hex[:8]}", ) await docker_repo.store_async(synapse_client=syn) schedule_for_cleanup(docker_repo.id) @@ -35,16 +34,16 @@ async def readonly_docker_repo( async def mutable_docker_repo( self, schedule_for_cleanup: Callable[..., None], + project_model: Project, syn: Synapse, ) -> DockerRepository: - """Function-scoped fixture for tests that modify or delete the repo.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - + """Function-scoped fixture for tests that modify or delete the repo. + Shares the session-scoped project_model instead of creating its own + Project; the repository_name is unique per invocation since this + fixture runs once per consuming test.""" docker_repo = DockerRepository( - parent_id=project.id, repository_name="username/test-async-mutable" + parent_id=project_model.id, + repository_name=f"username/test-async-mutable-{uuid.uuid4().hex[:8]}", ) await docker_repo.store_async(synapse_client=syn) schedule_for_cleanup(docker_repo.id) @@ -93,18 +92,16 @@ async def test_get_docker_repo_missing_id_raises_error(self, syn: Synapse) -> No await docker_repo.get_async(synapse_client=syn) async def test_get_docker_repo_with_optional_fields( - self, schedule_for_cleanup: Callable[..., None], syn: Synapse + self, + schedule_for_cleanup: Callable[..., None], + project_model: Project, + syn: Synapse, ) -> None: """Test retrieving a Docker repository with all optional fields set (async).""" - # GIVEN a project and DockerRepository with all fields - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - + # GIVEN a DockerRepository with all fields, under the shared project_model docker_repo = await DockerRepository( - parent_id=project.id, - repository_name="username/test-async-optional", + parent_id=project_model.id, + repository_name=f"username/test-async-optional-{uuid.uuid4().hex[:8]}", name="My Test Repo Async", description="A test repository with all fields (async)", ).store_async(synapse_client=syn) diff --git a/tests/integration/synapseclient/models/async/test_entityview_async.py b/tests/integration/synapseclient/models/async/test_entityview_async.py index d609c4f6e..cdb3df376 100644 --- a/tests/integration/synapseclient/models/async/test_entityview_async.py +++ b/tests/integration/synapseclient/models/async/test_entityview_async.py @@ -9,7 +9,6 @@ import synapseclient.models.mixins.table_components as table_module from synapseclient import Synapse from synapseclient.api import get_default_columns -from synapseclient.core import utils from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import ( Activity, @@ -47,13 +46,15 @@ async def setup_files_in_folder( # Create files files = [] - filename = utils.make_bogus_uuid_file() - # First file has a real path + # First file gets its own file handle. Only the file's existence and + # metadata matter for these tests, not its content, so an + # external_url file handle avoids a real upload. file1 = await File( parent_id=folder.id, name="file1", - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description="file1_description", ).store_async(synapse_client=self.syn) self.schedule_for_cleanup(file1.id) @@ -72,6 +73,48 @@ async def setup_files_in_folder( return folder, files + @pytest.fixture(scope="class") + async def shared_folder_with_files( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> "tuple[Folder, List[File]]": + """Folder with 4 files, built once for the class and shared by every + read-only scope consumer below (test_entityview_with_files_in_scope, + test_update_rows_without_id_column, test_query_with_part_mask, + test_snapshot_functionality) -- none of them mutates the folder or its + files. test_update_rows_and_annotations keeps its own dedicated folder + and files since it writes annotations onto the files, which the other + tests' views (scoped to the same folder) would otherwise observe.""" + folder = await Folder( + name=str(uuid.uuid4()), parent_id=project_model.id + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) + + files = [] + file1 = await File( + parent_id=folder.id, + name="file1", + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + description="file1_description", + ).store_async(synapse_client=syn) + schedule_for_cleanup(file1.id) + files.append(file1) + + for i in range(2, 5): + file = await File( + parent_id=folder.id, + name=f"file{i}", + data_file_handle_id=file1.data_file_handle_id, + description=f"file{i}_description", + ).store_async(synapse_client=syn) + schedule_for_cleanup(file.id) + files.append(file) + + return folder, files + async def test_entityview_creation_with_columns( self, project_model: Project ) -> None: @@ -189,10 +232,14 @@ async def test_entityview_invalid_column(self, project_model: Project) -> None: in str(e.value) ) - async def test_entityview_with_files_in_scope(self, project_model: Project) -> None: + async def test_entityview_with_files_in_scope( + self, + project_model: Project, + shared_folder_with_files: "tuple[Folder, List[File]]", + ) -> None: """Test creating entity view with files in scope and querying it""" # GIVEN a folder with files - folder, files = await self.setup_files_in_folder(project_model) + folder, files = shared_folder_with_files # WHEN I create an entity view with that folder in its scope entityview = EntityView( @@ -405,10 +452,14 @@ def csv_wrapper(*args, **kwargs): else: assert "float_column" not in file_copy.annotations.keys() - async def test_update_rows_without_id_column(self, project_model: Project) -> None: + async def test_update_rows_without_id_column( + self, + project_model: Project, + shared_folder_with_files: "tuple[Folder, List[File]]", + ) -> None: """Test that updating rows requires the id column""" # GIVEN a folder with files and an entity view - folder, _ = await self.setup_files_in_folder(project_model, num_files=1) + folder, _ = shared_folder_with_files entityview = EntityView( name=str(uuid.uuid4()), @@ -490,10 +541,14 @@ async def test_column_modifications(self, project_model: Project) -> None: assert new_column_name not in retrieved_view.columns assert column_to_keep in retrieved_view.columns - async def test_query_with_part_mask(self, project_model: Project) -> None: + async def test_query_with_part_mask( + self, + project_model: Project, + shared_folder_with_files: "tuple[Folder, List[File]]", + ) -> None: """Test querying an entity view with different part masks""" # GIVEN a folder with files - folder, files = await self.setup_files_in_folder(project_model, num_files=2) + folder, files = shared_folder_with_files # AND an entity view with the folder in scope entityview = EntityView( @@ -543,10 +598,14 @@ async def test_query_with_part_mask(self, project_model: Project) -> None: assert results_only.last_updated_on is None assert results_only.result["name"].tolist() == [file.name for file in files] - async def test_snapshot_functionality(self, project_model: Project) -> None: + async def test_snapshot_functionality( + self, + project_model: Project, + shared_folder_with_files: "tuple[Folder, List[File]]", + ) -> None: """Test creating snapshots of entity views with different activity configurations""" # GIVEN a folder with a file - folder, [file] = await self.setup_files_in_folder(project_model, num_files=1) + folder, _ = shared_folder_with_files # AND an entity view with an activity entityview = EntityView( diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index d2a702e19..c188263c2 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -82,21 +82,10 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for evaluation tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - @pytest.fixture(scope="function") async def test_evaluation( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: @@ -104,7 +93,7 @@ async def test_evaluation( evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for get tests", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results", submission_receipt_message="Thank you!", ) @@ -115,7 +104,7 @@ async def test_evaluation( @pytest.fixture(scope="function") async def multiple_evaluations( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> list[Evaluation]: @@ -125,7 +114,7 @@ async def multiple_evaluations( evaluation = Evaluation( name=f"test_evaluation_{i}_{uuid.uuid4()}", description=f"Test evaluation {i}", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results", submission_receipt_message="Thank you!", ) @@ -135,7 +124,7 @@ async def multiple_evaluations( return evaluations async def test_get_evaluation_by_id( - self, test_evaluation: Evaluation, test_project: Project + self, test_evaluation: Evaluation, project_model: Project ): # WHEN I get an evaluation by id using the dataclass method retrieved_evaluation = await Evaluation(id=test_evaluation.id).get_async( @@ -147,14 +136,14 @@ async def test_get_evaluation_by_id( assert retrieved_evaluation.etag is not None # Check that etag is set assert retrieved_evaluation.name == test_evaluation.name assert retrieved_evaluation.description == test_evaluation.description - assert retrieved_evaluation.content_source == test_project.id + assert retrieved_evaluation.content_source == project_model.id assert retrieved_evaluation.owner_id is not None # Check that owner_id is set assert ( retrieved_evaluation.created_on is not None ) # Check that created_on is set async def test_get_evaluation_by_name( - self, test_evaluation: Evaluation, test_project: Project + self, test_evaluation: Evaluation, project_model: Project ): # WHEN I get an evaluation by name using the dataclass method retrieved_evaluation = await Evaluation(name=test_evaluation.name).get_async( @@ -166,7 +155,7 @@ async def test_get_evaluation_by_name( assert retrieved_evaluation.etag is not None # Check that etag is set assert retrieved_evaluation.name == test_evaluation.name assert retrieved_evaluation.description == test_evaluation.description - assert retrieved_evaluation.content_source == test_project.id + assert retrieved_evaluation.content_source == project_model.id assert retrieved_evaluation.owner_id is not None # Check that owner_id is set assert ( retrieved_evaluation.created_on is not None @@ -216,11 +205,11 @@ async def test_get_available_evaluations( assert len(evaluations) >= len(multiple_evaluations) async def test_get_evaluations_by_project( - self, test_project: Project, multiple_evaluations: list[Evaluation] + self, project_model: Project, multiple_evaluations: list[Evaluation] ): # WHEN a call is made to get evaluations by project evaluations = await Evaluation.get_evaluations_by_project_async( - project_id=test_project.id, synapse_client=self.syn + project_id=project_model.id, synapse_client=self.syn ) # THEN the evaluations should be retrieved @@ -229,7 +218,7 @@ async def test_get_evaluations_by_project( # AND all returned evaluations belong to the test project for evaluation in evaluations: - assert evaluation.content_source == test_project.id + assert evaluation.content_source == project_model.id class TestStoreEvaluation: @@ -238,21 +227,10 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for evaluation tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - @pytest.fixture(scope="function") async def test_evaluation( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: @@ -260,7 +238,7 @@ async def test_evaluation( evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for update tests", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results", submission_receipt_message="Thank you!", ) @@ -369,21 +347,10 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for evaluation tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - @pytest.fixture(scope="function") async def test_evaluation( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: @@ -391,7 +358,7 @@ async def test_evaluation( evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for delete tests", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results", submission_receipt_message="Thank you!", ) @@ -414,21 +381,10 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for evaluation tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - @pytest.fixture(scope="function") async def test_evaluation( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: @@ -436,7 +392,7 @@ async def test_evaluation( evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for access tests", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results", submission_receipt_message="Thank you!", ) diff --git a/tests/integration/synapseclient/models/async/test_folder_async.py b/tests/integration/synapseclient/models/async/test_folder_async.py index 067ceb989..30850a69a 100644 --- a/tests/integration/synapseclient/models/async/test_folder_async.py +++ b/tests/integration/synapseclient/models/async/test_folder_async.py @@ -321,20 +321,19 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - def create_file_instance(self, schedule_for_cleanup: Callable[..., None]) -> File: - filename = utils.make_bogus_uuid_file() - schedule_for_cleanup(filename) + def create_file_instance(self) -> File: + # Only the entity's existence matters for copy structure verification, + # not its content, so an external_url file handle avoids a real upload. return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description=DESCRIPTION_FILE, content_type=CONTENT_TYPE, ) def create_files(self, count: int) -> List[File]: """Helper method to create multiple file instances""" - return [ - self.create_file_instance(self.schedule_for_cleanup) for _ in range(count) - ] + return [self.create_file_instance() for _ in range(count)] @pytest.fixture(autouse=True, scope="function") def folder(self) -> Folder: @@ -557,10 +556,13 @@ async def test_sync_all_entity_types(self, project_model: Project) -> None: self.schedule_for_cleanup(folder.id) # Create and store a File + # Only the entity's existence matters for entity-type sync verification, + # not its content, so an external_url file handle avoids a real upload. file = File( name=f"test_file_{str(uuid.uuid4())}.txt", parent_id=folder.id, - path=utils.make_bogus_uuid_file(), + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, ) file = await file.store_async(synapse_client=self.syn) self.schedule_for_cleanup(file.id) @@ -696,11 +698,12 @@ def init( self.syn = syn_with_logger self.schedule_for_cleanup = schedule_for_cleanup - def create_file_instance(self, schedule_for_cleanup: Callable[..., None]) -> File: - filename = utils.make_bogus_uuid_file() - schedule_for_cleanup(filename) + def create_file_instance(self) -> File: + # Only the entity's existence matters for walk structure verification, + # not its content, so an external_url file handle avoids a real upload. return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description=DESCRIPTION_FILE, content_type=CONTENT_TYPE, ) @@ -715,7 +718,7 @@ async def create_test_hierarchy(self, project_model: Project) -> dict: self.schedule_for_cleanup(folder.id) # Create a file in the root folder - root_file = self.create_file_instance(self.schedule_for_cleanup) + root_file = self.create_file_instance() root_file.parent_id = folder.id root_file = await root_file.store_async(synapse_client=self.syn) self.schedule_for_cleanup(root_file.id) @@ -726,7 +729,7 @@ async def create_test_hierarchy(self, project_model: Project) -> dict: nested_folder = await nested_folder.store_async(synapse_client=self.syn) self.schedule_for_cleanup(nested_folder.id) - nested_file = self.create_file_instance(self.schedule_for_cleanup) + nested_file = self.create_file_instance() nested_file.parent_id = nested_folder.id nested_file = await nested_file.store_async(synapse_client=self.syn) self.schedule_for_cleanup(nested_file.id) @@ -830,10 +833,12 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.schedule_for_cleanup = schedule_for_cleanup def create_file_instance(self) -> File: - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) + # Only the entity's existence and metadata matter for manifest CSV + # verification, not its content, so an external_url file handle avoids + # a real upload. return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, content_type="text/plain", ) @@ -967,11 +972,15 @@ async def test_manifest_suppress_creates_no_csv( async def test_manifest_includes_annotations(self, project_model: Project) -> None: # GIVEN a file with mixed-type annotations + # Annotations aren't populated on sync_from_synapse_async for an + # external_url file handle, so this test needs a real upload. folder = Folder(name=str(uuid.uuid4()), parent_id=project_model.id) folder = await folder.store_async(synapse_client=self.syn) self.schedule_for_cleanup(folder.id) - f = self.create_file_instance() + filename = utils.make_bogus_uuid_file() + self.schedule_for_cleanup(filename) + f = File(path=filename, content_type="text/plain") f.parent_id = folder.id f.annotations = { "single_str": ["hello"], @@ -1015,11 +1024,15 @@ async def test_manifest_includes_annotations(self, project_model: Project) -> No async def test_manifest_includes_provenance(self, project_model: Project) -> None: # GIVEN a file with activity (provenance) + # Activity isn't populated on sync_from_synapse_async for an + # external_url file handle, so this test needs a real upload. folder = Folder(name=str(uuid.uuid4()), parent_id=project_model.id) folder = await folder.store_async(synapse_client=self.syn) self.schedule_for_cleanup(folder.id) - f = self.create_file_instance() + filename = utils.make_bogus_uuid_file() + self.schedule_for_cleanup(filename) + f = File(path=filename, content_type="text/plain") f.parent_id = folder.id f.activity = Activity( name="my_activity", diff --git a/tests/integration/synapseclient/models/async/test_form_async.py b/tests/integration/synapseclient/models/async/test_form_async.py index d4c9bfce7..127a055ef 100644 --- a/tests/integration/synapseclient/models/async/test_form_async.py +++ b/tests/integration/synapseclient/models/async/test_form_async.py @@ -10,7 +10,7 @@ import synapseclient.core.utils as utils from synapseclient import Synapse -from synapseclient.models import File, FormData, FormGroup, Project +from synapseclient.models import File, Folder, FormData, FormGroup, Project class TestFormGroup: @@ -56,22 +56,29 @@ async def test_form_group( @pytest.fixture(autouse=True, scope="class") async def test_file( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> File: - """Create a test file for use in form data tests.""" - # Create a test project and a test file to get a file handle ID - project_name = str(uuid.uuid4()) - project = Project(name=project_name) - project = await project.store_async(synapse_client=syn) + """Create a test file for use in form data tests. + + FormData.download_async downloads the real file content, so the file + must be a real upload. Its parent only needs to exist, so a Folder in + the session-shared project is used instead of a dedicated Project. + """ + folder = await Folder( + name=str(uuid.uuid4()), parent_id=project_model.id + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) file_path = utils.make_bogus_data_file() - file = await File(path=file_path, parent_id=project.id).store_async( + file = await File(path=file_path, parent_id=folder.id).store_async( synapse_client=syn ) schedule_for_cleanup(file.id) schedule_for_cleanup(file_path) - schedule_for_cleanup(project.id) return file diff --git a/tests/integration/synapseclient/models/async/test_grid_async.py b/tests/integration/synapseclient/models/async/test_grid_async.py index 502b86ce8..f0d28e056 100644 --- a/tests/integration/synapseclient/models/async/test_grid_async.py +++ b/tests/integration/synapseclient/models/async/test_grid_async.py @@ -10,7 +10,6 @@ import pytest from synapseclient import Synapse -from synapseclient.core.utils import make_bogus_data_file from synapseclient.models import ( AuthorizationMode, EntityView, @@ -297,10 +296,12 @@ async def test_synchronize_grid_async( self.schedule_for_cleanup(created_grid) # AND: A file uploaded into the scoped folder - bogus_file = make_bogus_data_file() - self.schedule_for_cleanup(bogus_file) + # Only the file's existence in the EntityView's scope matters here, + # not its content, so an external_url file handle avoids a real + # upload. uploaded_file = await File( - path=bogus_file, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=folder.id, ).store_async(synapse_client=self.syn) self.schedule_for_cleanup(uploaded_file.id) diff --git a/tests/integration/synapseclient/models/async/test_json_schema_async.py b/tests/integration/synapseclient/models/async/test_json_schema_async.py index 36164d565..c889e7266 100644 --- a/tests/integration/synapseclient/models/async/test_json_schema_async.py +++ b/tests/integration/synapseclient/models/async/test_json_schema_async.py @@ -4,7 +4,6 @@ import pytest from synapseclient import Synapse -from synapseclient.core import utils from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import ( Column, @@ -120,8 +119,12 @@ def create_test_organization_with_schema( @pytest.fixture(scope="function") def file(self) -> File: - filename = utils.make_bogus_uuid_file() - return File(path=filename) + # Only the entity's existence matters here, not its content, so an + # external_url file handle avoids a real upload. + return File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + ) @pytest.fixture(scope="function") def table(self, project_model: Project) -> Table: @@ -466,10 +469,11 @@ async def test_get_validation_statistics_async( ) # Create two files under the folder - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) + # Only the entities' existence matters here, not their content, so + # external_url file handles avoid real uploads. file_1 = await File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name="test_file_1", description=DESCRIPTION_FILE, content_type=CONTENT_TYPE_FILE, @@ -479,10 +483,9 @@ async def test_get_validation_statistics_async( ).store_async(synapse_client=self.syn) self.schedule_for_cleanup(file_1.id) - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) file_2 = await File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name="test_file_2", description=DESCRIPTION_FILE, content_type=CONTENT_TYPE_FILE, @@ -558,10 +561,11 @@ async def test_get_invalid_validation_async( test_org, test_product_schema_uri = create_test_organization_with_schema # Create two files under the folder - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) + # Only the entities' existence matters here, not their content, so + # external_url file handles avoid real uploads. file_1 = await File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name="test_file_1", description=DESCRIPTION_FILE, content_type=CONTENT_TYPE_FILE, @@ -571,10 +575,9 @@ async def test_get_invalid_validation_async( ).store_async(synapse_client=self.syn) self.schedule_for_cleanup(file_1.id) - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) file_2 = await File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name="test_file_2", description=DESCRIPTION_FILE, content_type=CONTENT_TYPE_FILE, diff --git a/tests/integration/synapseclient/models/async/test_materializedview_async.py b/tests/integration/synapseclient/models/async/test_materializedview_async.py index 4ddb797ee..6fff68da7 100644 --- a/tests/integration/synapseclient/models/async/test_materializedview_async.py +++ b/tests/integration/synapseclient/models/async/test_materializedview_async.py @@ -1,6 +1,6 @@ import asyncio import uuid -from typing import Callable +from typing import Callable, Tuple import pandas as pd import pytest @@ -212,14 +212,20 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="function") + @pytest.fixture(scope="class") async def base_table_with_data( self, project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Table: - """Create a table with data for use in a single test.""" + """ + Create a table with data once for the whole class: every test that uses + this fixture only queries through a materialized view or changes the + view's own defining SQL, never the source table's rows, so a shared + table costs nothing in correctness and saves a create-plus-store job + per additional test. + """ table_name = str(uuid.uuid4()) table = Table( name=table_name, @@ -447,10 +453,19 @@ async def test_query_part_mask_async( assert query_result.count == 2 assert query_result.last_updated_on is not None - async def test_materialized_view_with_left_join( - self, project_model: Project - ) -> None: - # GIVEN two tables with related data + @pytest.fixture(scope="class") + async def join_source_tables( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> Tuple[Table, Table]: + """ + Two tables with the identical data used by the left/right/inner join + tests below, built once for the class: none of those tests mutates + either table, they only build a differently-joined `MaterializedView` + over the same pair and query that view. + """ table1 = Table( name=str(uuid.uuid4()), parent_id=project_model.id, @@ -459,8 +474,8 @@ async def test_materialized_view_with_left_join( Column(name="name", column_type=ColumnType.STRING), ], ) - table1 = await table1.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table1.id) + table1 = await table1.store_async(synapse_client=syn) + schedule_for_cleanup(table1.id) table2 = Table( name=str(uuid.uuid4()), @@ -470,14 +485,22 @@ async def test_materialized_view_with_left_join( Column(name="age", column_type=ColumnType.INTEGER), ], ) - table2 = await table2.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table2.id) + table2 = await table2.store_async(synapse_client=syn) + schedule_for_cleanup(table2.id) data1 = pd.DataFrame({"unique_identifier": [1, 2], "name": ["Alice", "Bob"]}) - await table1.store_rows_async(data1, synapse_client=self.syn) + await table1.store_rows_async(data1, synapse_client=syn) data2 = pd.DataFrame({"unique_identifier": [1, 3], "age": [30, 40]}) - await table2.store_rows_async(data2, synapse_client=self.syn) + await table2.store_rows_async(data2, synapse_client=syn) + + return table1, table2 + + async def test_materialized_view_with_left_join( + self, project_model: Project, join_source_tables: Tuple[Table, Table] + ) -> None: + # GIVEN two tables with related data + table1, table2 = join_source_tables # WHEN creating a materialized view with a LEFT JOIN left_join_view = MaterializedView( @@ -507,36 +530,10 @@ async def test_materialized_view_with_left_join( assert pd.isna(result["age"][1]) async def test_materialized_view_with_right_join( - self, project_model: Project + self, project_model: Project, join_source_tables: Tuple[Table, Table] ) -> None: # GIVEN two tables with related data - table1 = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="unique_identifier", column_type=ColumnType.INTEGER), - Column(name="name", column_type=ColumnType.STRING), - ], - ) - table1 = await table1.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table1.id) - - table2 = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="unique_identifier", column_type=ColumnType.INTEGER), - Column(name="age", column_type=ColumnType.INTEGER), - ], - ) - table2 = await table2.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table2.id) - - data1 = pd.DataFrame({"unique_identifier": [1, 2], "name": ["Alice", "Bob"]}) - await table1.store_rows_async(data1, synapse_client=self.syn) - - data2 = pd.DataFrame({"unique_identifier": [1, 3], "age": [30, 40]}) - await table2.store_rows_async(data2, synapse_client=self.syn) + table1, table2 = join_source_tables # WHEN creating a materialized view with a RIGHT JOIN right_join_view = MaterializedView( @@ -566,36 +563,10 @@ async def test_materialized_view_with_right_join( assert result["age"].tolist() == [30, 40] async def test_materialized_view_with_inner_join( - self, project_model: Project + self, project_model: Project, join_source_tables: Tuple[Table, Table] ) -> None: # GIVEN two tables with related data - table1 = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="unique_identifier", column_type=ColumnType.INTEGER), - Column(name="name", column_type=ColumnType.STRING), - ], - ) - table1 = await table1.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table1.id) - - table2 = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="unique_identifier", column_type=ColumnType.INTEGER), - Column(name="age", column_type=ColumnType.INTEGER), - ], - ) - table2 = await table2.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table2.id) - - data1 = pd.DataFrame({"unique_identifier": [1, 2], "name": ["Alice", "Bob"]}) - await table1.store_rows_async(data1, synapse_client=self.syn) - - data2 = pd.DataFrame({"unique_identifier": [1, 3], "age": [30, 40]}) - await table2.store_rows_async(data2, synapse_client=self.syn) + table1, table2 = join_source_tables # WHEN creating a materialized view with an INNER JOIN inner_join_view = MaterializedView( diff --git a/tests/integration/synapseclient/models/async/test_permissions_async.py b/tests/integration/synapseclient/models/async/test_permissions_async.py index 3e710c031..f33d6ee4b 100644 --- a/tests/integration/synapseclient/models/async/test_permissions_async.py +++ b/tests/integration/synapseclient/models/async/test_permissions_async.py @@ -3,12 +3,11 @@ import asyncio import logging import uuid -from typing import Callable, Dict, List, Optional, Type, Union +from typing import Callable, Dict, List, Optional, Tuple, Type, Union import pytest from synapseclient import Synapse -from synapseclient.core import utils from synapseclient.core.models.acl import AclListResult from synapseclient.models import ( Column, @@ -46,10 +45,13 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.schedule_for_cleanup = schedule_for_cleanup @pytest.fixture(scope="function") - def file(self, schedule_for_cleanup: Callable[..., None]) -> File: - filename = utils.make_bogus_uuid_file() - schedule_for_cleanup(filename) - return File(path=filename) + def file(self) -> File: + # Only the permission-bearing entity matters here, not its content, so + # an external_url file handle avoids a real upload. + return File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + ) @pytest.fixture(scope="function") def table(self, project_model: Project) -> Table: @@ -973,10 +975,13 @@ async def stored_project( return project @pytest.fixture(scope="function") - def file(self, schedule_for_cleanup: Callable[..., None]) -> File: - filename = utils.make_bogus_uuid_file() - schedule_for_cleanup(filename) - return File(path=filename) + def file(self) -> File: + # Only the permission-bearing entity matters here, not its content, so + # an external_url file handle avoids a real upload. + return File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + ) async def _set_custom_permissions( self, entity: Union[File, Folder, Project] @@ -1134,7 +1139,9 @@ async def create_simple_tree_structure( self.schedule_for_cleanup(folder_a.id) file_1 = await File( - path=utils.make_bogus_uuid_file(), name=f"file_1_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_1_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=folder_a, synapse_client=self.syn) self.schedule_for_cleanup(file_1.id) @@ -1169,7 +1176,9 @@ async def create_deep_nested_structure( # Create file_at_1 and level_2 in parallel since they don't depend on each other file_at_1_task = File( - path=utils.make_bogus_uuid_file(), name=f"file_at_1_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_at_1_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=level_1, synapse_client=self.syn) level_2_task = Folder(name=f"level_2_{uuid.uuid4()}").store_async( parent=level_1, synapse_client=self.syn @@ -1181,7 +1190,9 @@ async def create_deep_nested_structure( # Create file_at_2 and level_3 in parallel since they don't depend on each other file_at_2_task = File( - path=utils.make_bogus_uuid_file(), name=f"file_at_2_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_at_2_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=level_2, synapse_client=self.syn) level_3_task = Folder(name=f"level_3_{uuid.uuid4()}").store_async( parent=level_2, synapse_client=self.syn @@ -1193,7 +1204,9 @@ async def create_deep_nested_structure( # Create file_at_3 and level_4 in parallel since they don't depend on each other file_at_3_task = File( - path=utils.make_bogus_uuid_file(), name=f"file_at_3_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_at_3_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=level_3, synapse_client=self.syn) level_4_task = Folder(name=f"level_4_{uuid.uuid4()}").store_async( parent=level_3, synapse_client=self.syn @@ -1204,7 +1217,9 @@ async def create_deep_nested_structure( self.schedule_for_cleanup(level_4.id) file_at_4 = await File( - path=utils.make_bogus_uuid_file(), name=f"file_at_4_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_at_4_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=level_4, synapse_client=self.syn) self.schedule_for_cleanup(file_at_4.id) @@ -1253,15 +1268,18 @@ async def create_wide_tree_structure( # Create files in parallel file_tasks = [ File( - path=utils.make_bogus_uuid_file(), + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", name=f"file_{folder_letter}_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=folder, synapse_client=self.syn) for folder_letter, folder in zip(["a", "b", "c"], folders) ] # Create root file task root_file_task = File( - path=utils.make_bogus_uuid_file(), name=f"root_file_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"root_file_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=project_model, synapse_client=self.syn) # Execute file creation tasks in parallel @@ -1327,13 +1345,17 @@ async def create_complex_mixed_structure( # Create first level files and folders shallow_file = await File( - path=utils.make_bogus_uuid_file(), name=f"shallow_file_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"shallow_file_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=shallow_folder, synapse_client=self.syn) self.schedule_for_cleanup(shallow_file.id) # Deep branch structure deep_file_1_task = File( - path=utils.make_bogus_uuid_file(), name=f"deep_file_1_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"deep_file_1_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=deep_branch, synapse_client=self.syn) sub_deep_task = Folder(name=f"sub_deep_{uuid.uuid4()}").store_async( @@ -1346,7 +1368,9 @@ async def create_complex_mixed_structure( # Continue deep structure deep_file_2_task = File( - path=utils.make_bogus_uuid_file(), name=f"deep_file_2_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"deep_file_2_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=sub_deep, synapse_client=self.syn) sub_sub_deep_task = Folder(name=f"sub_sub_deep_{uuid.uuid4()}").store_async( @@ -1360,13 +1384,17 @@ async def create_complex_mixed_structure( self.schedule_for_cleanup(sub_sub_deep.id) deep_file_3 = await File( - path=utils.make_bogus_uuid_file(), name=f"deep_file_3_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"deep_file_3_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=sub_sub_deep, synapse_client=self.syn) self.schedule_for_cleanup(deep_file_3.id) # Mixed folder structure mixed_file_task = File( - path=utils.make_bogus_uuid_file(), name=f"mixed_file_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"mixed_file_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=mixed_folder, synapse_client=self.syn) mixed_sub_a_task = Folder(name=f"mixed_sub_a_{uuid.uuid4()}").store_async( @@ -1387,11 +1415,15 @@ async def create_complex_mixed_structure( # Create files in mixed sub-folders in parallel mixed_file_a_task = File( - path=utils.make_bogus_uuid_file(), name=f"mixed_file_a_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"mixed_file_a_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=mixed_sub_a, synapse_client=self.syn) mixed_file_b_task = File( - path=utils.make_bogus_uuid_file(), name=f"mixed_file_b_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"mixed_file_b_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=mixed_sub_b, synapse_client=self.syn) mixed_file_a, mixed_file_b = await asyncio.gather( @@ -1571,12 +1603,37 @@ async def test_delete_permissions_wide_tree_structure( *[self._verify_permissions_deleted(entity) for entity in entities_to_verify] ) + @pytest.fixture(scope="class") + async def shared_complex_mixed_structure( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> Tuple[Project, Dict[str, Union[Folder, File, List]]]: + """ + Built once for the whole class rather than once per test: the five + `test_delete_permissions_*` tests below each assert a different + `delete_permissions_async` behavior against this structure, but each + re-sets the permissions it verifies immediately before verifying, so + they don't depend on which of them ran first. Repeating the 7 file + uploads inside `create_complex_mixed_structure` for every test bought + no additional coverage. + """ + self.syn = syn + self.schedule_for_cleanup = schedule_for_cleanup + project = await Project( + name=f"integration_test_project_{uuid.uuid4()}" + ).store_async(synapse_client=syn) + schedule_for_cleanup(project.id) + return project, await self.create_complex_mixed_structure(project) + async def test_delete_permissions_complex_mixed_structure( - self, stored_project: Project, caplog: pytest.LogCaptureFixture + self, + shared_complex_mixed_structure: Tuple[ + Project, Dict[str, Union[Folder, File, List]] + ], + caplog: pytest.LogCaptureFixture, ) -> None: """Test deleting permissions on a complex mixed structure.""" # GIVEN a complex mixed structure with permissions - structure = await self.create_complex_mixed_structure(stored_project) + project, structure = shared_complex_mixed_structure # Set permissions on all entities entities_to_set = ( @@ -1600,7 +1657,7 @@ async def test_delete_permissions_complex_mixed_structure( # WHEN - Verify list_acl_functionality before deletion await self._verify_list_acl_functionality( - entity=stored_project, + entity=project, expected_entity_count=12, # complex structure with multiple entities recursive=True, include_container_content=True, @@ -1612,7 +1669,7 @@ async def test_delete_permissions_complex_mixed_structure( caplog.clear() # WHEN I delete permissions recursively from the project - await stored_project.delete_permissions_async( + await project.delete_permissions_async( recursive=True, include_container_content=True, dry_run=False, @@ -1672,7 +1729,9 @@ async def test_delete_permissions_folder_with_only_files( self.schedule_for_cleanup(folder.id) file = await File( - path=utils.make_bogus_uuid_file(), name=f"only_file_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"only_file_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=folder, synapse_client=self.syn) self.schedule_for_cleanup(file.id) @@ -1767,11 +1826,15 @@ async def test_delete_permissions_folder_with_only_folders( ) async def test_delete_permissions_target_files_only_complex( - self, stored_project: Project, caplog: pytest.LogCaptureFixture + self, + shared_complex_mixed_structure: Tuple[ + Project, Dict[str, Union[Folder, File, List]] + ], + caplog: pytest.LogCaptureFixture, ) -> None: """Test deleting permissions targeting only files in a complex structure.""" # GIVEN a complex structure with permissions - structure = await self.create_complex_mixed_structure(stored_project) + project, structure = shared_complex_mixed_structure # Set permissions on all entities await asyncio.gather( @@ -1785,7 +1848,7 @@ async def test_delete_permissions_target_files_only_complex( # WHEN - Verify list_acl_async with target_entity_types for files only await self._verify_list_acl_functionality( - entity=stored_project, + entity=project, expected_entity_count=4, # shallow_file + 3 deep_files recursive=True, include_container_content=True, @@ -1798,7 +1861,7 @@ async def test_delete_permissions_target_files_only_complex( caplog.clear() # WHEN I delete permissions targeting only files - await stored_project.delete_permissions_async( + await project.delete_permissions_async( recursive=True, include_container_content=True, target_entity_types=["file"], @@ -1875,11 +1938,15 @@ async def test_delete_permissions_include_container_only_deep_structure( ) async def test_delete_permissions_skip_self_complex_structure( - self, stored_project: Project, caplog: pytest.LogCaptureFixture + self, + shared_complex_mixed_structure: Tuple[ + Project, Dict[str, Union[Folder, File, List]] + ], + caplog: pytest.LogCaptureFixture, ) -> None: """Test include_self=False on a complex structure.""" # GIVEN a complex mixed structure with permissions - structure = await self.create_complex_mixed_structure(stored_project) + _, structure = shared_complex_mixed_structure # Set permissions on all entities await asyncio.gather( @@ -1998,11 +2065,15 @@ async def test_delete_permissions_dry_run_no_changes( ) async def test_delete_permissions_dry_run_complex_logging( - self, stored_project: Project, caplog: pytest.LogCaptureFixture + self, + shared_complex_mixed_structure: Tuple[ + Project, Dict[str, Union[Folder, File, List]] + ], + caplog: pytest.LogCaptureFixture, ) -> None: """Test dry run logging for complex structures.""" # GIVEN a complex structure with permissions - structure = await self.create_complex_mixed_structure(stored_project) + _, structure = shared_complex_mixed_structure # Set permissions on a subset of entities await asyncio.gather( @@ -2069,7 +2140,9 @@ async def test_delete_permissions_large_flat_structure( # Create files in parallel file_tasks = [ File( - path=utils.make_bogus_uuid_file(), name=f"large_file_{i}_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"large_file_{i}_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=large_folder, synapse_client=self.syn) for i in range(10) # Reduced from larger number for test performance ] @@ -2158,8 +2231,9 @@ async def test_delete_permissions_multiple_nested_branches( for level in range(2): parent_folder = nested_folders[folder_index] file_task = File( - path=utils.make_bogus_uuid_file(), + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", name=f"{branch_name}_file_{level}_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=parent_folder, synapse_client=self.syn) file_tasks.append(file_task) folder_index += 1 @@ -2227,10 +2301,14 @@ async def test_delete_permissions_selective_branches( # Create files in each branch in parallel file_tasks = [ File( - path=utils.make_bogus_uuid_file(), name=f"file_a_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_a_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=branch_a, synapse_client=self.syn), File( - path=utils.make_bogus_uuid_file(), name=f"file_b_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"file_b_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=branch_b, synapse_client=self.syn), ] file_a, file_b = await asyncio.gather(*file_tasks) @@ -2282,11 +2360,15 @@ async def test_delete_permissions_selective_branches( ) async def test_delete_permissions_mixed_entity_types_in_structure( - self, stored_project: Project, caplog: pytest.LogCaptureFixture + self, + shared_complex_mixed_structure: Tuple[ + Project, Dict[str, Union[Folder, File, List]] + ], + caplog: pytest.LogCaptureFixture, ) -> None: """Test deleting permissions with mixed entity types in complex structure.""" # GIVEN a structure with both files and folders at multiple levels - structure = await self.create_complex_mixed_structure(stored_project) + project, structure = shared_complex_mixed_structure # Set permissions on a mix of entities await asyncio.gather( @@ -2300,7 +2382,7 @@ async def test_delete_permissions_mixed_entity_types_in_structure( # WHEN - Verify list_acl_async with mixed entity types await self._verify_list_acl_functionality( - entity=stored_project, + entity=project, expected_entity_count=5, # All the entities we set permissions on recursive=True, include_container_content=True, @@ -2313,7 +2395,7 @@ async def test_delete_permissions_mixed_entity_types_in_structure( caplog.clear() # WHEN I delete permissions targeting both files and folders - await stored_project.delete_permissions_async( + await project.delete_permissions_async( recursive=True, include_container_content=True, target_entity_types=["file", "folder"], @@ -2341,7 +2423,9 @@ async def test_delete_permissions_no_container_content_but_has_children( self.schedule_for_cleanup(parent_folder.id) child_file = await File( - path=utils.make_bogus_uuid_file(), name=f"child_file_{uuid.uuid4()}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"child_file_{uuid.uuid4()}", + synapse_store=False, ).store_async(parent=parent_folder, synapse_client=self.syn) self.schedule_for_cleanup(child_file.id) @@ -2447,12 +2531,11 @@ async def create_all_entity_types(self, project_model: Project) -> Dict[str, any entities = {"project": project_model} - file_path = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(file_path) file_entity = File( name=f"test_file_{str(uuid.uuid4())}.txt", parent_id=project_model.id, - path=file_path, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, ) file_entity = await file_entity.store_async(synapse_client=self.syn) self.schedule_for_cleanup(file_entity.id) @@ -2549,11 +2632,32 @@ async def create_all_entity_types_with_acl( await asyncio.sleep(2) return entities - async def test_list_acl_async_all_entity_types(self) -> None: + @pytest.fixture(scope="class") + async def shared_entities_with_acl( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> Dict[str, any]: + """ + Built once for the class: `test_list_acl_async_all_entity_types`, + `test_list_acl_async_specific_entity_types`, and + `test_delete_permissions_async_all_entity_types` below only read ACLs or + run `delete_permissions_async` with `dry_run=True`, so none of them mutates + this structure's permissions. The two `..._actual_deletion` tests below + are NOT given this fixture: each removes the local ACLs it just verified + were present, which would corrupt the "before" assertions of any test + sharing the same structure that ran afterward, so they keep their own + fresh entities. + """ + self.syn = syn + self.schedule_for_cleanup = schedule_for_cleanup + project = Project(name=f"test_project_{uuid.uuid4()}") + return await self.create_all_entity_types_with_acl(project) + + async def test_list_acl_async_all_entity_types( + self, shared_entities_with_acl: Dict[str, any] + ) -> None: """Test list_acl_async functionality with all supported entity types.""" # GIVEN a project with all supported entity types and local ACL permissions - project = Project(name=f"test_project_{uuid.uuid4()}") - entities = await self.create_all_entity_types_with_acl(project) + entities = shared_entities_with_acl # WHEN I call list_acl_async on the project with all entity types result = await entities["project"].list_acl_async( @@ -2612,11 +2716,12 @@ async def test_list_acl_async_all_entity_types(self) -> None: entity.id in entities_with_read_permissions ), f"Entity {entity.id} ({entity_type}) should appear in AclListResult with READ permissions" - async def test_list_acl_async_specific_entity_types(self) -> None: + async def test_list_acl_async_specific_entity_types( + self, shared_entities_with_acl: Dict[str, any] + ) -> None: """Test list_acl_async functionality with specific entity types.""" # GIVEN a project with all supported entity types - project = Project(name=f"test_project_{uuid.uuid4()}") - entities = await self.create_all_entity_types_with_acl(project) + entities = shared_entities_with_acl # WHEN I call list_acl_async with only table-related entity types result = await entities["project"].list_acl_async( @@ -2712,11 +2817,12 @@ async def test_list_acl_async_specific_entity_types(self) -> None: has_read_permission ), f"Entity {entity_id} should have READ permissions for AUTHENTICATED_USERS" - async def test_delete_permissions_async_all_entity_types(self) -> None: + async def test_delete_permissions_async_all_entity_types( + self, shared_entities_with_acl: Dict[str, any] + ) -> None: """Test delete_permissions_async functionality with all supported entity types.""" # GIVEN a project with all supported entity types and local ACL permissions - project = Project(name=f"test_project_{uuid.uuid4()}") - entities = await self.create_all_entity_types_with_acl(project) + entities = shared_entities_with_acl # AND I verify AUTHENTICATED_USERS has READ permissions before deletion for entity_type, entity in entities.items(): diff --git a/tests/integration/synapseclient/models/async/test_project_async.py b/tests/integration/synapseclient/models/async/test_project_async.py index 9a8be2574..b6edd3830 100644 --- a/tests/integration/synapseclient/models/async/test_project_async.py +++ b/tests/integration/synapseclient/models/async/test_project_async.py @@ -1,5 +1,6 @@ """Integration tests for the synapseclient.models.Project class.""" +import copy import os import uuid from typing import Callable, List @@ -407,13 +408,31 @@ def verify_copied_project( assert sub_file.name is not None assert sub_file.parent_id == folder.id - async def test_copy_project_variations(self) -> None: + @pytest.fixture(scope="class") + async def _shared_nested_project( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> Project: + """ + Built once for the whole class rather than once per test: + `test_copy_project_variations` and `test_sync_from_synapse` both need a + stored project with the same nested files/folders/annotations shape. + """ + self.syn = syn + self.schedule_for_cleanup = schedule_for_cleanup + project = self.create_nested_project() + stored_project = await project.store_async(synapse_client=syn) + schedule_for_cleanup(stored_project.id) + return stored_project + + @pytest.fixture + def shared_nested_project(self, _shared_nested_project: Project) -> Project: + return copy.deepcopy(_shared_nested_project) + + async def test_copy_project_variations( + self, shared_nested_project: Project + ) -> None: # GIVEN a nested source project and a destination project - source_project = self.create_nested_project() - stored_source_project = await source_project.store_async( - synapse_client=self.syn - ) - self.schedule_for_cleanup(stored_source_project.id) + stored_source_project = shared_nested_project # Test Case 1: Copy project with all contents # Create first destination project @@ -456,15 +475,13 @@ async def test_copy_project_variations(self) -> None: copied_project_no_files, stored_source_project, expected_files_empty=True ) - async def test_sync_from_synapse(self, file: File) -> None: + async def test_sync_from_synapse( + self, file: File, shared_nested_project: Project + ) -> None: # GIVEN a nested project structure root_directory_path = os.path.dirname(file.path) - project = self.create_nested_project() - - # WHEN I store the Project on Synapse - stored_project = await project.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(project.id) + stored_project = shared_nested_project # AND I sync the project from Synapse copied_project = await stored_project.sync_from_synapse_async( @@ -552,7 +569,8 @@ async def test_sync_all_entity_types(self) -> None: file = File( name=f"test_file_{str(uuid.uuid4())}.txt", parent_id=project_model.id, - path=utils.make_bogus_uuid_file(), + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, ) file = await file.store_async(synapse_client=self.syn) self.schedule_for_cleanup(file.id) @@ -622,10 +640,11 @@ def init( self.schedule_for_cleanup = schedule_for_cleanup def create_file_instance(self, schedule_for_cleanup: Callable[..., None]) -> File: - filename = utils.make_bogus_uuid_file() - schedule_for_cleanup(filename) + # Only the entity's existence matters for walk_async results, not its + # content, so an external_url file handle avoids a real upload. return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description=DESCRIPTION_FILE, content_type=CONTENT_TYPE, ) diff --git a/tests/integration/synapseclient/models/async/test_recordset_async.py b/tests/integration/synapseclient/models/async/test_recordset_async.py index 75f8c2471..85871d388 100644 --- a/tests/integration/synapseclient/models/async/test_recordset_async.py +++ b/tests/integration/synapseclient/models/async/test_recordset_async.py @@ -373,7 +373,7 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="function") + @pytest.fixture(scope="class") def create_test_schema( self, syn: Synapse ) -> Generator[Tuple[JsonSchemaOrganization, str, list], None, None]: @@ -448,13 +448,17 @@ def create_test_schema( except Exception: pass # Ignore if org can't be deleted - @pytest.fixture(scope="function") + @pytest.fixture(scope="class") async def record_set_with_validation_fixture( self, + syn: Synapse, project_model: Project, create_test_schema: Tuple[JsonSchemaOrganization, str, list], + schedule_for_cleanup: Callable[..., None], ) -> RecordSet: - """Create and store a RecordSet with schema bound, then export via Grid to generate validation results.""" + """Create and store a RecordSet with schema bound, then export via Grid to generate + validation results. Shared by every test in this class that reads validation results; + none of them mutates the RecordSet or its validation results.""" from tests.integration import ASYNC_JOB_TIMEOUT_SEC _, schema_uri, record_set_ids = create_test_schema @@ -495,7 +499,7 @@ async def record_set_with_validation_fixture( try: os.close(temp_fd) # Close the file descriptor test_data.to_csv(filename, index=False) - self.schedule_for_cleanup(filename) + schedule_for_cleanup(filename) # Create and store the RecordSet record_set = RecordSet( @@ -508,9 +512,9 @@ async def record_set_with_validation_fixture( ) stored_record_set = await record_set.store_async( - parent=project_model, synapse_client=self.syn + parent=project_model, synapse_client=syn ) - self.schedule_for_cleanup(stored_record_set.id) + schedule_for_cleanup(stored_record_set.id) record_set_ids.append(stored_record_set.id) # Track for schema cleanup await asyncio.sleep(3) @@ -519,11 +523,11 @@ async def record_set_with_validation_fixture( await stored_record_set.bind_schema_async( json_schema_uri=schema_uri, enable_derived_annotations=False, - synapse_client=self.syn, + synapse_client=syn, ) # Verify the schema is bound by getting the schema from the entity - await stored_record_set.get_schema_async(synapse_client=self.syn) + await stored_record_set.get_schema_async(synapse_client=syn) # Wait for schema binding to be fully processed by backend await asyncio.sleep(5) @@ -531,22 +535,22 @@ async def record_set_with_validation_fixture( # Create a Grid session from the RecordSet grid = Grid(record_set_id=stored_record_set.id) created_grid = await grid.create_async( - timeout=ASYNC_JOB_TIMEOUT_SEC, synapse_client=self.syn + timeout=ASYNC_JOB_TIMEOUT_SEC, synapse_client=syn ) await asyncio.sleep(3) # Export the Grid back to RecordSet to generate validation results exported_grid = await created_grid.export_to_record_set_async( - timeout=ASYNC_JOB_TIMEOUT_SEC, synapse_client=self.syn + timeout=ASYNC_JOB_TIMEOUT_SEC, synapse_client=syn ) # Clean up the Grid session - await exported_grid.delete_async(synapse_client=self.syn) + await exported_grid.delete_async(synapse_client=syn) # Re-fetch the RecordSet to get the updated validation_file_handle_id updated_record_set = await RecordSet(id=stored_record_set.id).get_async( - synapse_client=self.syn + synapse_client=syn ) return updated_record_set diff --git a/tests/integration/synapseclient/models/async/test_storable_container_async.py b/tests/integration/synapseclient/models/async/test_storable_container_async.py index f87b92721..7b316863f 100644 --- a/tests/integration/synapseclient/models/async/test_storable_container_async.py +++ b/tests/integration/synapseclient/models/async/test_storable_container_async.py @@ -10,7 +10,6 @@ import pytest import pytest_asyncio -import synapseclient.core.utils as utils from synapseclient import Synapse from synapseclient.models import File, Folder, Project from synapseclient.models.activity import UsedURL @@ -69,11 +68,10 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: async def _create_test_file(self, project: Project, **kwargs) -> File: """Upload a small test file to Synapse and return the File model.""" - path = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(path) file = File( parent_id=project.id, - path=path, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name=f"test_file_{uuid.uuid4()}", **kwargs, ) diff --git a/tests/integration/synapseclient/models/async/test_submission_async.py b/tests/integration/synapseclient/models/async/test_submission_async.py index 71ca65f96..55f5bcd9c 100644 --- a/tests/integration/synapseclient/models/async/test_submission_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_async.py @@ -11,71 +11,55 @@ from synapseclient.models import Evaluation, File, Project, Submission +@pytest_asyncio.fixture(scope="module") +async def test_evaluation( + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], +) -> Evaluation: + """ + Create a test evaluation shared by the classes in this module that need + one: none of them mutate the Evaluation itself, only the Submission + objects created against it, so one evaluation on the existing shared + project_model is safe for the whole file. + """ + evaluation = Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="A test evaluation for submission tests", + content_source=project_model.id, + submission_instructions_message="Please submit your results", + submission_receipt_message="Thank you!", + ) + created_evaluation = await evaluation.store_async(synapse_client=syn) + schedule_for_cleanup(created_evaluation.id) + return created_evaluation + + class TestSubmissionCreationAsync: @pytest.fixture(autouse=True, scope="function") def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest_asyncio.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for submission tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest_asyncio.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission tests", - content_source=test_project.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - @pytest_asyncio.fixture(scope="function") async def test_file( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> File: """Create a test file for submission tests.""" - import os - import tempfile - - # Create a temporary file - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission testing.") - temp_file_path = temp_file.name - - try: - file = await File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=test_project.id, - ).store_async(synapse_client=syn) - schedule_for_cleanup(file.id) - return file - finally: - # Clean up the temporary file - os.unlink(temp_file_path) + # Only the file's existence as a submission subject matters here, + # not its content, so an external_url file handle avoids a real + # upload. + file = await File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + name=f"test_file_{uuid.uuid4()}.txt", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(file.id) + return file async def test_store_submission_successfully_async( self, test_evaluation: Evaluation, test_file: File @@ -100,14 +84,14 @@ async def test_store_submission_successfully_async( async def test_store_submission_to_evaluation_without_message_fields_async( self, - test_project: Project, + project_model: Project, test_file: File, ): # GIVEN an evaluation created without submission_instructions_message or submission_receipt_message evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="Evaluation without optional message fields", - content_source=test_project.id, + content_source=project_model.id, ) created_evaluation = await evaluation.store_async(synapse_client=self.syn) self.schedule_for_cleanup(created_evaluation.id) @@ -165,63 +149,26 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest_asyncio.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for submission tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest_asyncio.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission tests", - content_source=test_project.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - @pytest_asyncio.fixture(scope="function") async def test_file( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> File: - """Create a test file for submission tests.""" - import os - import tempfile - - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission testing.") - temp_file_path = temp_file.name - - try: - file = await File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=test_project.id, - ).store_async(synapse_client=syn) - schedule_for_cleanup(file.id) - return file - finally: - os.unlink(temp_file_path) + """ + Create a test file for submission tests. None of these tests + assert on file content, so an external_url file handle avoids a + real upload. + """ + file = await File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + name=f"test_file_{uuid.uuid4()}.txt", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(file.id) + return file @pytest_asyncio.fixture(scope="function") async def test_submission( @@ -339,63 +286,26 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest_asyncio.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for submission tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest_asyncio.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission tests", - content_source=test_project.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - @pytest_asyncio.fixture(scope="function") async def test_file( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> File: - """Create a test file for submission tests.""" - import os - import tempfile - - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission testing.") - temp_file_path = temp_file.name - - try: - file = await File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=test_project.id, - ).store_async(synapse_client=syn) - schedule_for_cleanup(file.id) - return file - finally: - os.unlink(temp_file_path) + """ + Create a test file for submission tests. None of these tests + assert on file content, so an external_url file handle avoids a + real upload. + """ + file = await File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + name=f"test_file_{uuid.uuid4()}.txt", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(file.id) + return file async def test_delete_submission_successfully_async( self, test_evaluation: Evaluation, test_file: File @@ -432,64 +342,6 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest_asyncio.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for submission tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest_asyncio.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission tests", - content_source=test_project.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - - @pytest_asyncio.fixture(scope="function") - async def test_file( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> File: - """Create a test file for submission tests.""" - import os - import tempfile - - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission testing.") - temp_file_path = temp_file.name - - try: - file = await File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=test_project.id, - ).store_async(synapse_client=syn) - schedule_for_cleanup(file.id) - return file - finally: - os.unlink(temp_file_path) - async def test_cancel_submission_without_id_async(self): # WHEN I try to cancel a submission without an ID using async method submission = Submission(entity_id="syn123", evaluation_id="456") diff --git a/tests/integration/synapseclient/models/async/test_submission_bundle_async.py b/tests/integration/synapseclient/models/async/test_submission_bundle_async.py index cd41f4717..88d8101d5 100644 --- a/tests/integration/synapseclient/models/async/test_submission_bundle_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_bundle_async.py @@ -17,6 +17,33 @@ ) +@pytest.fixture(scope="module") +async def test_evaluation( + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], +) -> Evaluation: + """ + Shared by TestSubmissionBundleRetrievalAsync and + TestSubmissionBundleDataIntegrityAsync: neither asserts on the total + submission count for this evaluation, only that its own submissions are + present, so one evaluation for both is safe. + + TestSubmissionBundleEdgeCasesAsync defines its own test_evaluation + below, which shadows this one for that class only - its tests assert + the evaluation has zero submissions, so it must stay isolated. + """ + evaluation = await Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="Test evaluation for SubmissionBundle async testing", + content_source=project_model.id, + submission_instructions_message="Submit your files here", + submission_receipt_message="Thank you for your submission!", + ).store_async(synapse_client=syn) + schedule_for_cleanup(evaluation.id) + return evaluation + + class TestSubmissionBundleRetrievalAsync: """Tests for retrieving SubmissionBundle objects using async methods.""" @@ -25,50 +52,18 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - evaluation = await Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="Test evaluation for SubmissionBundle async testing", - content_source=test_project.id, - submission_instructions_message="Submit your files here", - submission_receipt_message="Thank you for your submission!", - ).store_async(synapse_client=syn) - schedule_for_cleanup(evaluation.id) - return evaluation - @pytest.fixture(scope="function") async def test_file( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> File: - file_content = ( - f"Test file content for submission bundle async tests {uuid.uuid4()}" - ) - with open("test_file_for_submission_bundle_async.txt", "w") as f: - f.write(file_content) - file_entity = await File( - path="test_file_for_submission_bundle_async.txt", + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name=f"test_submission_file_async_{uuid.uuid4()}", - parent_id=test_project.id, + parent_id=project_model.id, ).store_async(synapse_client=syn) schedule_for_cleanup(file_entity.id) return file_entity @@ -320,50 +315,18 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - @pytest.fixture(scope="class") - async def test_evaluation( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - evaluation = await Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="Test evaluation for data integrity async testing", - content_source=test_project.id, - submission_instructions_message="Submit your files here", - submission_receipt_message="Thank you for your submission!", - ).store_async(synapse_client=syn) - schedule_for_cleanup(evaluation.id) - return evaluation - @pytest.fixture(scope="function") async def test_file( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> File: - file_content = ( - f"Test file content for data integrity async tests {uuid.uuid4()}" - ) - with open("test_file_for_data_integrity_async.txt", "w") as f: - f.write(file_content) - file_entity = await File( - path="test_file_for_data_integrity_async.txt", + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name=f"test_integrity_file_async_{uuid.uuid4()}", - parent_id=test_project.id, + parent_id=project_model.id, ).store_async(synapse_client=syn) schedule_for_cleanup(file_entity.id) return file_entity @@ -498,69 +461,30 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - @pytest.fixture(scope="class") async def test_evaluation( self, - test_project: Project, + project_model: Project, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: + """ + Deliberately isolated from the module-level test_evaluation fixture: + this class's tests assert the evaluation has zero submissions, so it + cannot be shared with the classes above that submit to theirs. It + still reuses the existing project_model instead of creating another + project, since nothing here asserts the project itself is empty. + """ evaluation = await Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="Test evaluation for edge case async testing", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Submit your files here", submission_receipt_message="Thank you for your submission!", ).store_async(synapse_client=syn) schedule_for_cleanup(evaluation.id) return evaluation - @pytest.fixture(scope="function") - async def test_file( - self, - test_project: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> File: - file_content = f"Test file content for edge case async tests {uuid.uuid4()}" - with open("test_file_for_edge_case_async.txt", "w") as f: - f.write(file_content) - - file_entity = await File( - path="test_file_for_edge_case_async.txt", - name=f"test_edge_case_file_async_{uuid.uuid4()}", - parent_id=test_project.id, - ).store_async(synapse_client=syn) - schedule_for_cleanup(file_entity.id) - return file_entity - - @pytest.fixture(scope="function") - async def test_submission( - self, - test_evaluation: Evaluation, - test_file: File, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Submission: - submission = await Submission( - name=f"test_submission_{uuid.uuid4()}", - entity_id=test_file.id, - evaluation_id=test_evaluation.id, - submitter_alias="test_user_edge_case_async", - ).store_async(synapse_client=syn) - schedule_for_cleanup(submission.id) - return submission - async def test_get_evaluation_submission_bundles_empty_evaluation_async( self, test_evaluation: Evaluation ): diff --git a/tests/integration/synapseclient/models/async/test_submission_status_async.py b/tests/integration/synapseclient/models/async/test_submission_status_async.py index f345fa2e8..c09109f63 100644 --- a/tests/integration/synapseclient/models/async/test_submission_status_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_status_async.py @@ -1,7 +1,5 @@ """Integration tests for the synapseclient.models.SubmissionStatus class async methods.""" -import os -import tempfile import uuid from typing import Callable @@ -26,6 +24,52 @@ ] +@pytest.fixture(scope="module") +async def test_evaluation( + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], +) -> Evaluation: + """ + Create a test evaluation shared by every class in this module: none of + them mutate the Evaluation itself, only the Submission/SubmissionStatus + objects created against it, so one evaluation for the whole file is safe. + """ + evaluation = Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="A test evaluation for submission status tests", + content_source=project_model.id, + submission_instructions_message="Please submit your results", + submission_receipt_message="Thank you!", + ) + created_evaluation = await evaluation.store_async(synapse_client=syn) + schedule_for_cleanup(created_evaluation.id) + return created_evaluation + + +@pytest.fixture(scope="module") +async def test_file( + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], +) -> File: + """ + Create a test file shared by every class in this module that needs a + single submittable entity. None of these tests assert on file content, + only on submission/status behavior, so an external_url file handle + avoids a real upload. + """ + file = File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + name=f"test_file_{uuid.uuid4()}.txt", + parent_id=project_model.id, + ) + stored_file = await file.store_async(synapse_client=syn) + schedule_for_cleanup(stored_file.id) + return stored_file + + class TestSubmissionStatusRetrieval: """Tests for retrieving SubmissionStatus objects async.""" @@ -34,53 +78,6 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_evaluation( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission status tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission status tests", - content_source=project_model.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - - @pytest.fixture(scope="class") - async def test_file( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> File: - """Create a test file for submission status tests.""" - # Create a temporary file - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission status testing.") - temp_file_path = temp_file.name - - try: - file = File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=project_model.id, - ) - stored_file = await file.store_async(synapse_client=syn) - schedule_for_cleanup(stored_file.id) - return stored_file - finally: - # Clean up the temporary file - os.unlink(temp_file_path) - @pytest.fixture(scope="function") async def test_submission( self, @@ -145,53 +142,6 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_evaluation( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission status tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission status tests", - content_source=project_model.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - - @pytest.fixture(scope="class") - async def test_file( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> File: - """Create a test file for submission status tests.""" - # Create a temporary file - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission status testing.") - temp_file_path = temp_file.name - - try: - file = File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=project_model.id, - ) - stored_file = await file.store_async(synapse_client=syn) - schedule_for_cleanup(stored_file.id) - return stored_file - finally: - # Clean up the temporary file - os.unlink(temp_file_path) - @pytest.fixture(scope="function") async def test_submission( self, @@ -497,7 +447,13 @@ async def test_evaluation( syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> Evaluation: - """Create a test evaluation for submission status tests.""" + """ + Deliberately isolated from the module-level test_evaluation fixture: + this class's tests exercise get_all_submission_statuses_async's + default page (limit=10), so submissions created against the shared + evaluation by the other classes in this module would already fill + that page and make the assertions below flaky. + """ evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for submission status tests", @@ -516,30 +472,22 @@ async def test_files( syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> list[File]: - """Create multiple test files for submission status tests.""" + """ + Create multiple test files for submission status tests. None of + these tests assert on file content, so external_url file handles + avoid real uploads. + """ files = [] for i in range(3): - # Create a temporary file - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write( - f"This is test content {i} for submission status testing." - ) - temp_file_path = temp_file.name - - try: - file = File( - path=temp_file_path, - name=f"test_file_{i}_{uuid.uuid4()}.txt", - parent_id=project_model.id, - ) - stored_file = await file.store_async(synapse_client=syn) - schedule_for_cleanup(stored_file.id) - files.append(stored_file) - finally: - # Clean up the temporary file - os.unlink(temp_file_path) + file = File( + external_url=f"https://example.com/bogus-file-{i}-{uuid.uuid4()}.txt", + synapse_store=False, + name=f"test_file_{i}_{uuid.uuid4()}.txt", + parent_id=project_model.id, + ) + stored_file = await file.store_async(synapse_client=syn) + schedule_for_cleanup(stored_file.id) + files.append(stored_file) return files @@ -678,53 +626,6 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_evaluation( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> Evaluation: - """Create a test evaluation for submission status tests.""" - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation for submission status tests", - content_source=project_model.id, - submission_instructions_message="Please submit your results", - submission_receipt_message="Thank you!", - ) - created_evaluation = await evaluation.store_async(synapse_client=syn) - schedule_for_cleanup(created_evaluation.id) - return created_evaluation - - @pytest.fixture(scope="class") - async def test_file( - self, - project_model: Project, - syn: Synapse, - schedule_for_cleanup: Callable[..., None], - ) -> File: - """Create a test file for submission status tests.""" - # Create a temporary file - with tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".txt" - ) as temp_file: - temp_file.write("This is test content for submission status testing.") - temp_file_path = temp_file.name - - try: - file = File( - path=temp_file_path, - name=f"test_file_{uuid.uuid4()}.txt", - parent_id=project_model.id, - ) - stored_file = await file.store_async(synapse_client=syn) - schedule_for_cleanup(stored_file.id) - return stored_file - finally: - # Clean up the temporary file - os.unlink(temp_file_path) - @pytest.fixture(scope="function") async def test_submission( self, diff --git a/tests/integration/synapseclient/models/async/test_submissionview_async.py b/tests/integration/synapseclient/models/async/test_submissionview_async.py index eda0a17b2..c55c2340a 100644 --- a/tests/integration/synapseclient/models/async/test_submissionview_async.py +++ b/tests/integration/synapseclient/models/async/test_submissionview_async.py @@ -1,5 +1,4 @@ import asyncio -import tempfile import uuid from typing import Callable @@ -235,15 +234,34 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - async def test_column_modifications(self, project_model: Project) -> None: + @pytest.fixture(scope="class") + async def shared_evaluations( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> "list[Evaluation]": + """Two evaluations, built once for the class. Both tests below only use + these as scope_ids targets on their own submissionviews -- neither + submits to, deletes, or otherwise mutates the evaluations themselves, + and neither queries submission content, so sharing them is read-only.""" + evaluations = [] + for i in range(2): + evaluation = await Evaluation( + name=str(uuid.uuid4()), + description=f"Test evaluation {i + 1} for submission view", + content_source=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(evaluation) + evaluations.append(evaluation) + return evaluations + + async def test_column_modifications( + self, project_model: Project, shared_evaluations: "list[Evaluation]" + ) -> None: # GIVEN a project to work with # AND an evaluation to use in the scope - evaluation = await Evaluation( - name=str(uuid.uuid4()), - description="Test evaluation for submission view", - content_source=project_model.id, - ).store_async(synapse_client=self.syn) - self.schedule_for_cleanup(evaluation) + evaluation = shared_evaluations[0] # AND a submissionview in Synapse with two columns submissionview_name = str(uuid.uuid4()) @@ -298,22 +316,12 @@ async def test_column_modifications(self, project_model: Project) -> None: assert new_column_name not in updated_view2.columns assert column_to_keep in updated_view2.columns - async def test_scope_modifications(self, project_model: Project) -> None: + async def test_scope_modifications( + self, project_model: Project, shared_evaluations: "list[Evaluation]" + ) -> None: # GIVEN a project to work with # AND two evaluations for testing scope changes - evaluation1 = await Evaluation( - name=str(uuid.uuid4()), - description="Test evaluation 1", - content_source=project_model.id, - ).store_async(synapse_client=self.syn) - self.schedule_for_cleanup(evaluation1) - - evaluation2 = await Evaluation( - name=str(uuid.uuid4()), - description="Test evaluation 2", - content_source=project_model.id, - ).store_async(synapse_client=self.syn) - self.schedule_for_cleanup(evaluation2) + evaluation1, evaluation2 = shared_evaluations # AND a submissionview with one evaluation in scope submissionview_name = str(uuid.uuid4()) @@ -544,12 +552,15 @@ async def test_submission_lifecycle(self, project_model: Project) -> None: self.schedule_for_cleanup(submissionview) # AND a file for submission - with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f: - filename = f.name - f.write("Test content for submission") - self.schedule_for_cleanup(filename) - - file_entity = File(path=filename, parent_id=project_model.id, name="Test file") + # Only the file's existence as a submission subject matters here, + # not its content, so an external_url file handle avoids a real + # upload. + file_entity = File( + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + parent_id=project_model.id, + name="Test file", + ) file_entity = await file_entity.store_async(synapse_client=self.syn) self.schedule_for_cleanup(file_entity.id) @@ -632,17 +643,14 @@ async def test_multiple_submissions(self, project_model: Project) -> None: submissions = [] for i in range(3): - # Create test file - with tempfile.NamedTemporaryFile( - mode="w", suffix=".txt", delete=False - ) as f: - filename = f.name - f.write(f"Test content for submission {i}") - self.schedule_for_cleanup(filename) - - # Store file in Synapse + # Create test file. Only the file's existence as a submission + # subject matters here, not its content, so an external_url + # file handle avoids a real upload. file_entity = File( - path=filename, parent_id=project_model.id, name=f"Test file {i}" + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, + parent_id=project_model.id, + name=f"Test file {i}", ) file_entity = await file_entity.store_async(synapse_client=self.syn) self.schedule_for_cleanup(file_entity.id) diff --git a/tests/integration/synapseclient/models/async/test_table_async.py b/tests/integration/synapseclient/models/async/test_table_async.py index 5b4fedfc0..f189f339d 100644 --- a/tests/integration/synapseclient/models/async/test_table_async.py +++ b/tests/integration/synapseclient/models/async/test_table_async.py @@ -2144,98 +2144,93 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - async def test_delete_single_row_via_query(self, project_model: Project) -> None: - # GIVEN a table in Synapse - table_name = str(uuid.uuid4()) + @pytest.fixture(scope="class") + async def table_with_groups( + self, + project_model: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> Table: + """Class-scoped table holding four independent row groups, one per + delete scenario below, populated with a single store_rows_async call + instead of one per scenario, since no scenario's rows overlap with + another's.""" table = Table( - name=table_name, + name=str(uuid.uuid4()), parent_id=project_model.id, columns=[Column(name="column_string", column_type=ColumnType.STRING)], ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) + table = await table.store_async(synapse_client=syn) + schedule_for_cleanup(table.id) - # AND data for a column already stored in Synapse - data_for_table = pd.DataFrame({"column_string": ["value1", "value2", "value3"]}) + data_for_table = pd.DataFrame( + { + "column_string": [ + f"{group}_value{i}" + for group in ("g1", "g2", "g3", "g4") + for i in (1, 2, 3) + ] + } + ) await table.store_rows_async( - values=data_for_table, schema_storage_strategy=None, synapse_client=self.syn + values=data_for_table, schema_storage_strategy=None, synapse_client=syn ) + return table + + async def test_delete_single_row_via_query(self, table_with_groups: Table) -> None: + table = table_with_groups # WHEN I delete a single row from the table await table.delete_rows_async( - query=f"SELECT ROW_ID, ROW_VERSION FROM {table.id} WHERE column_string = 'value2'", + query=f"SELECT ROW_ID, ROW_VERSION FROM {table.id} WHERE column_string = 'g1_value2'", synapse_client=self.syn, ) # AND I query the table results = await query_async( - f"SELECT * FROM {table.id}", synapse_client=self.syn + f"SELECT * FROM {table.id} WHERE column_string IN ('g1_value1', 'g1_value2', 'g1_value3')", + synapse_client=self.syn, ) # THEN the data in the columns should match pd.testing.assert_series_equal( - results["column_string"], - pd.DataFrame({"column_string": ["value1", "value3"]})["column_string"], + results["column_string"].reset_index(drop=True), + pd.Series(["g1_value1", "g1_value3"], name="column_string"), check_dtype=False, ) - # AND only 2 rows should exist on the table + # AND only 2 rows should exist in this group assert len(results) == 2 - async def test_delete_multiple_rows_via_query(self, project_model: Project) -> None: - # GIVEN a table in Synapse - table_name = str(uuid.uuid4()) - table = Table( - name=table_name, - parent_id=project_model.id, - columns=[Column(name="column_string", column_type=ColumnType.STRING)], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # AND data for a column already stored in Synapse - data_for_table = pd.DataFrame({"column_string": ["value1", "value2", "value3"]}) - await table.store_rows_async( - values=data_for_table, schema_storage_strategy=None, synapse_client=self.syn - ) + async def test_delete_multiple_rows_via_query( + self, table_with_groups: Table + ) -> None: + table = table_with_groups # WHEN I delete a single row from the table await table.delete_rows_async( - query=f"SELECT ROW_ID, ROW_VERSION FROM {table.id} WHERE column_string IN ('value2','value3')", + query=f"SELECT ROW_ID, ROW_VERSION FROM {table.id} WHERE column_string IN ('g2_value2','g2_value3')", synapse_client=self.syn, ) # AND I query the table results = await query_async( - f"SELECT * FROM {table.id}", synapse_client=self.syn + f"SELECT * FROM {table.id} WHERE column_string IN ('g2_value1', 'g2_value2', 'g2_value3')", + synapse_client=self.syn, ) # THEN the data in the columns should match pd.testing.assert_series_equal( - results["column_string"], - pd.DataFrame({"column_string": ["value1"]})["column_string"], + results["column_string"].reset_index(drop=True), + pd.Series(["g2_value1"], name="column_string"), check_dtype=False, ) - # AND only 1 row should exist on the table + # AND only 1 row should exist in this group assert len(results) == 1 - async def test_delete_no_rows_via_query(self, project_model: Project) -> None: - # GIVEN a table in Synapse - table_name = str(uuid.uuid4()) - table = Table( - name=table_name, - parent_id=project_model.id, - columns=[Column(name="column_string", column_type=ColumnType.STRING)], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # AND data for a column already stored in Synapse - data_for_table = pd.DataFrame({"column_string": ["value1", "value2", "value3"]}) - await table.store_rows_async( - values=data_for_table, schema_storage_strategy=None, synapse_client=self.syn - ) + async def test_delete_no_rows_via_query(self, table_with_groups: Table) -> None: + table = table_with_groups # WHEN I delete a single row from the table await table.delete_rows_async( @@ -2245,78 +2240,60 @@ async def test_delete_no_rows_via_query(self, project_model: Project) -> None: # AND I query the table results = await query_async( - f"SELECT * FROM {table.id}", synapse_client=self.syn + f"SELECT * FROM {table.id} WHERE column_string IN ('g3_value1', 'g3_value2', 'g3_value3')", + synapse_client=self.syn, ) # THEN the data in the columns should match pd.testing.assert_series_equal( - results["column_string"], data_for_table["column_string"], check_dtype=False + results["column_string"].reset_index(drop=True), + pd.Series(["g3_value1", "g3_value2", "g3_value3"], name="column_string"), + check_dtype=False, ) - # AND 3 rows should exist on the table + # AND 3 rows should exist in this group assert len(results) == 3 async def test_delete_multiple_rows_via_dataframe( - self, project_model: Project + self, table_with_groups: Table ) -> None: - # GIVEN a table in Synapse - table_name = str(uuid.uuid4()) - table = Table( - name=table_name, - parent_id=project_model.id, - columns=[Column(name="column_string", column_type=ColumnType.STRING)], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) + table = table_with_groups - # AND data for a column already stored in Synapse - data_for_table = pd.DataFrame({"column_string": ["value1", "value2", "value3"]}) - await table.store_rows_async( - values=data_for_table, schema_storage_strategy=None, synapse_client=self.syn + # GIVEN the ROW_ID and ROW_VERSION for this group's rows + group_rows = await query_async( + f"SELECT ROW_ID, ROW_VERSION, column_string FROM {table.id} WHERE column_string IN ('g4_value2', 'g4_value3')", + synapse_client=self.syn, ) - # Get the ROW_ID and ROW_VERSION for the data we just added + # WHEN I delete rows from the table using a dataframe await table.delete_rows_async( - df=pd.DataFrame({"ROW_ID": [2, 3], "ROW_VERSION": [1, 1]}), + df=group_rows[["ROW_ID", "ROW_VERSION"]], synapse_client=self.syn, ) # AND I query the table results = await query_async( - f"SELECT * FROM {table.id}", synapse_client=self.syn + f"SELECT * FROM {table.id} WHERE column_string IN ('g4_value1', 'g4_value2', 'g4_value3')", + synapse_client=self.syn, ) # THEN the data in the columns should match pd.testing.assert_series_equal( - results["column_string"], - pd.DataFrame({"column_string": ["value1"]})["column_string"], + results["column_string"].reset_index(drop=True), + pd.Series(["g4_value1"], name="column_string"), check_dtype=False, ) - # AND only 1 row should exist on the table + # AND only 1 row should exist in this group assert len(results) == 1 async def test_delete_multiple_rows_via_dataframe_exception( - self, project_model: Project + self, table_with_groups: Table ) -> None: - # GIVEN a table in Synapse - table_name = str(uuid.uuid4()) - table = Table( - name=table_name, - parent_id=project_model.id, - columns=[Column(name="column_string", column_type=ColumnType.STRING)], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # AND data for a column already stored in Synapse - data_for_table = pd.DataFrame({"column_string": ["value1", "value2", "value3"]}) - await table.store_rows_async( - values=data_for_table, schema_storage_strategy=None, synapse_client=self.syn - ) + table = table_with_groups # AND row ids and versions that do not exist in the table - row_ids = [4, 5] + row_ids = [999001, 999002] row_versions = [1, 1] # And an excpeted error message that should be displayed @@ -2554,8 +2531,14 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - async def test_snapshot_basic(self, project_model: Project) -> None: - """Test creating a basic snapshot of a table.""" + async def test_snapshot_scenarios(self, project_model: Project) -> None: + """Exercises snapshot_async's comment/label, activity-included, + activity-excluded, and minimal-argument paths against one shared + table and one store_rows_async call instead of four, taking each + snapshot in sequence and asserting against the version number it + actually produced (each snapshot fixes the table's current + "in progress" version and bumps a new one) rather than a hardcoded 1. + """ # GIVEN a table with some data table = Table( name=str(uuid.uuid4()), @@ -2572,193 +2555,95 @@ async def test_snapshot_basic(self, project_model: Project) -> None: data = {"col1": ["A", "B"], "col2": [1, 2]} await table.store_rows_async(values=data, synapse_client=self.syn) - # WHEN I create a snapshot + expected_version = 1 + + # Scenario 1: basic snapshot snapshot_response = await table.snapshot_async( comment="Test snapshot", label="v1.0", synapse_client=self.syn ) - - # THEN the snapshot should be created successfully assert snapshot_response is not None assert "snapshotVersionNumber" in snapshot_response - assert snapshot_response["snapshotVersionNumber"] is not None - - # AND the snapshot version should be 1 snapshot_version = snapshot_response["snapshotVersionNumber"] - assert snapshot_version == 1 - - # AND when I retrieve the snapshot version, it should have the correct comment and label + assert snapshot_version == expected_version snapshot_table = await Table( id=table.id, version_number=snapshot_version ).get_async(synapse_client=self.syn) assert snapshot_table.version_comment == "Test snapshot" assert snapshot_table.version_label == "v1.0" - assert snapshot_table.version_number == 1 - - # AND when I retrieve the latest version (without specifying version), it should be "in progress" + assert snapshot_table.version_number == expected_version latest_table = await Table(id=table.id).get_async(synapse_client=self.syn) assert latest_table.version_label == "in progress" assert latest_table.version_comment == "in progress" - assert latest_table.version_number > 1 + assert latest_table.version_number > snapshot_version + expected_version += 1 - async def test_snapshot_with_activity(self, project_model: Project) -> None: - """Test creating a snapshot with activity (provenance).""" - # GIVEN a table with some data and an activity - table = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="col1", column_type=ColumnType.STRING), - Column(name="col2", column_type=ColumnType.INTEGER), - ], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # Create and store an activity + # Scenario 2: snapshot with activity included activity = Activity( name="Test Activity", description="Test activity for snapshot", ) table.activity = activity await table.store_async(synapse_client=self.syn) - - # Store some data - data = {"col1": ["A", "B"], "col2": [1, 2]} - await table.store_rows_async(values=data, synapse_client=self.syn) - - # WHEN I create a snapshot with activity included snapshot_response = await table.snapshot_async( comment="Test snapshot with activity", - label="v1.0", + label="v2.0", include_activity=True, associate_activity_to_new_version=False, synapse_client=self.syn, ) - - # THEN the snapshot should be created successfully assert snapshot_response is not None - assert "snapshotVersionNumber" in snapshot_response - assert snapshot_response["snapshotVersionNumber"] is not None - - # AND the snapshot version should be 1 snapshot_version = snapshot_response["snapshotVersionNumber"] - assert snapshot_version == 1 - - # AND when I retrieve the snapshot version, it should have the correct comment and label + assert snapshot_version == expected_version snapshot_table = await Table( id=table.id, version_number=snapshot_version ).get_async(synapse_client=self.syn) assert snapshot_table.version_comment == "Test snapshot with activity" - assert snapshot_table.version_label == "v1.0" - assert snapshot_table.version_number == 1 - - # AND when I retrieve the latest version (without specifying version), it should be "in progress" + assert snapshot_table.version_label == "v2.0" + assert snapshot_table.version_number == expected_version latest_table = await Table(id=table.id).get_async(synapse_client=self.syn) assert latest_table.version_label == "in progress" assert latest_table.version_comment == "in progress" - assert latest_table.version_number > 1 - - async def test_snapshot_without_activity(self, project_model: Project) -> None: - """Test creating a snapshot without including activity.""" - # GIVEN a table with some data and an activity - table = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="col1", column_type=ColumnType.STRING), - Column(name="col2", column_type=ColumnType.INTEGER), - ], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # Create and store an activity - activity = Activity( - name="Test Activity", - description="Test activity for snapshot", - ) - table.activity = activity - await table.store_async(synapse_client=self.syn) + assert latest_table.version_number > snapshot_version + expected_version += 1 - # Store some data - data = {"col1": ["A", "B"], "col2": [1, 2]} - await table.store_rows_async(values=data, synapse_client=self.syn) - - # WHEN I create a snapshot without including activity + # Scenario 3: snapshot without activity snapshot_response = await table.snapshot_async( comment="Test snapshot without activity", - label="v2.0", + label="v3.0", include_activity=False, synapse_client=self.syn, ) - - # THEN the snapshot should be created successfully assert snapshot_response is not None - assert "snapshotVersionNumber" in snapshot_response - assert snapshot_response["snapshotVersionNumber"] is not None - - # AND the snapshot version should be 1 snapshot_version = snapshot_response["snapshotVersionNumber"] - assert snapshot_version == 1 - - # AND when I retrieve the snapshot version, it should have the correct comment and label + assert snapshot_version == expected_version snapshot_table = await Table( id=table.id, version_number=snapshot_version ).get_async(synapse_client=self.syn) assert snapshot_table.version_comment == "Test snapshot without activity" - assert snapshot_table.version_label == "v2.0" - assert snapshot_table.version_number == 1 - - # AND when I retrieve the latest version (without specifying version), it should be "in progress" + assert snapshot_table.version_label == "v3.0" + assert snapshot_table.version_number == expected_version latest_table = await Table(id=table.id).get_async(synapse_client=self.syn) assert latest_table.version_label == "in progress" assert latest_table.version_comment == "in progress" - assert latest_table.version_number > 1 + assert latest_table.version_number > snapshot_version + expected_version += 1 - async def test_snapshot_minimal_args(self, project_model: Project) -> None: - """Test creating a snapshot with minimal arguments.""" - # GIVEN a table with some data - table = Table( - name=str(uuid.uuid4()), - parent_id=project_model.id, - columns=[ - Column(name="col1", column_type=ColumnType.STRING), - Column(name="col2", column_type=ColumnType.INTEGER), - ], - ) - table = await table.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(table.id) - - # Store some data - data = {"col1": ["A", "B"], "col2": [1, 2]} - await table.store_rows_async(values=data, synapse_client=self.syn) - - # WHEN I create a snapshot with minimal arguments + # Scenario 4: snapshot with minimal arguments snapshot_response = await table.snapshot_async(synapse_client=self.syn) - - # THEN the snapshot should be created successfully assert snapshot_response is not None - assert "snapshotVersionNumber" in snapshot_response - assert snapshot_response["snapshotVersionNumber"] is not None - - # AND the snapshot version should be 1 snapshot_version = snapshot_response["snapshotVersionNumber"] - assert snapshot_version == 1 - - # AND when I retrieve the snapshot version, it should have the correct version number + assert snapshot_version == expected_version snapshot_table = await Table( id=table.id, version_number=snapshot_version ).get_async(synapse_client=self.syn) - assert snapshot_table.version_number == 1 + assert snapshot_table.version_number == expected_version # Comment and label should be None or empty when not specified assert ( snapshot_table.version_comment is None or snapshot_table.version_comment == "" ) - assert snapshot_table.version_label == "1" - - # AND when I retrieve the latest version (without specifying version), it should be "in progress" + assert snapshot_table.version_label == str(expected_version) latest_table = await Table(id=table.id).get_async(synapse_client=self.syn) assert latest_table.version_label == "in progress" assert latest_table.version_comment == "in progress" - assert latest_table.version_number > 1 + assert latest_table.version_number > snapshot_version diff --git a/tests/integration/synapseclient/models/async/test_virtualtable_async.py b/tests/integration/synapseclient/models/async/test_virtualtable_async.py index 78cb688b5..619fbe79d 100644 --- a/tests/integration/synapseclient/models/async/test_virtualtable_async.py +++ b/tests/integration/synapseclient/models/async/test_virtualtable_async.py @@ -1,6 +1,5 @@ -import asyncio import uuid -from typing import Callable +from typing import Awaitable, Callable import pandas as pd import pytest @@ -10,6 +9,24 @@ from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import Column, ColumnType, Project, Table, VirtualTable from tests.integration import QUERY_TIMEOUT_SEC +from tests.integration.helpers import wait_for_condition + + +async def _query_until( + query_fn: Callable[[], Awaitable[pd.DataFrame]], + condition: Callable[[pd.DataFrame], bool], + description: str, +) -> pd.DataFrame: + """Poll a virtual table query until the result satisfies condition.""" + result_holder = {} + + async def _check() -> bool: + result = await query_fn() + result_holder["value"] = result + return condition(result) + + await wait_for_condition(_check, description=description) + return result_holder["value"] class TestVirtualTableBasicOperations: @@ -193,58 +210,27 @@ async def test_virtual_table_data_queries( ) -> None: table = base_table_with_data - # GIVEN various virtual tables with different SQL transformations + # GIVEN a single virtual table whose defining SQL is updated across + # different transformations, rather than creating a separate virtual + # table per transformation - # Test case 1: Basic selection of all data - virtual_table_all = VirtualTable( + # WHEN querying a virtual table that selects all data + virtual_table = VirtualTable( name=str(uuid.uuid4()), parent_id=project_model.id, defining_sql=f"SELECT * FROM {table.id}", ) - virtual_table_all = await virtual_table_all.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(virtual_table_all.id) - - # Test case 2: Column selection - virtual_table_columns = VirtualTable( - name=str(uuid.uuid4()), - parent_id=project_model.id, - defining_sql=f"SELECT name, city FROM {table.id}", - ) - virtual_table_columns = await virtual_table_columns.store_async( - synapse_client=self.syn - ) - self.schedule_for_cleanup(virtual_table_columns.id) - - # Test case 3: Filtering - virtual_table_filtered = VirtualTable( - name=str(uuid.uuid4()), - parent_id=project_model.id, - defining_sql=f"SELECT * FROM {table.id} WHERE age > 25", - ) - virtual_table_filtered = await virtual_table_filtered.store_async( - synapse_client=self.syn - ) - self.schedule_for_cleanup(virtual_table_filtered.id) - - # Test case 4: Ordering - virtual_table_ordered = VirtualTable( - name=str(uuid.uuid4()), - parent_id=project_model.id, - defining_sql=f"SELECT * FROM {table.id} ORDER BY age DESC", - ) - virtual_table_ordered = await virtual_table_ordered.store_async( - synapse_client=self.syn - ) - self.schedule_for_cleanup(virtual_table_ordered.id) - - # Wait for the virtual tables to be ready - await asyncio.sleep(2) + virtual_table = await virtual_table.store_async(synapse_client=self.syn) + self.schedule_for_cleanup(virtual_table.id) - # WHEN querying the full-data virtual table - all_result = await virtual_table_all.query_async( - f"SELECT * FROM {virtual_table_all.id}", - synapse_client=self.syn, - timeout=QUERY_TIMEOUT_SEC, + all_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", + synapse_client=self.syn, + timeout=QUERY_TIMEOUT_SEC, + ), + condition=lambda r: len(r) == 3, + description="virtual table row count == 3", ) # THEN all data should be returned @@ -253,11 +239,18 @@ async def test_virtual_table_data_queries( assert set(all_result["age"].tolist()) == {30, 25, 35} assert set(all_result["city"].tolist()) == {"New York", "Boston", "Chicago"} - # WHEN querying the column-selection virtual table - columns_result = await virtual_table_columns.query_async( - f"SELECT * FROM {virtual_table_columns.id}", - synapse_client=self.syn, - timeout=QUERY_TIMEOUT_SEC, + # WHEN updating the SQL to select specific columns + virtual_table.defining_sql = f"SELECT name, city FROM {table.id}" + virtual_table = await virtual_table.store_async(synapse_client=self.syn) + + columns_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", + synapse_client=self.syn, + timeout=QUERY_TIMEOUT_SEC, + ), + condition=lambda r: len(r) == 3 and "age" not in r.columns, + description="virtual table columns updated", ) # THEN only specified columns should be returned @@ -266,11 +259,18 @@ async def test_virtual_table_data_queries( assert "city" in columns_result.columns assert "age" not in columns_result.columns - # WHEN querying the filtered virtual table - filtered_result = await virtual_table_filtered.query_async( - f"SELECT * FROM {virtual_table_filtered.id}", - synapse_client=self.syn, - timeout=QUERY_TIMEOUT_SEC, + # WHEN updating the SQL to filter rows + virtual_table.defining_sql = f"SELECT * FROM {table.id} WHERE age > 25" + virtual_table = await virtual_table.store_async(synapse_client=self.syn) + + filtered_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", + synapse_client=self.syn, + timeout=QUERY_TIMEOUT_SEC, + ), + condition=lambda r: len(r) == 2, + description="virtual table row count == 2", ) # THEN only filtered rows should be returned @@ -278,11 +278,18 @@ async def test_virtual_table_data_queries( assert set(filtered_result["name"].tolist()) == {"Alice", "Charlie"} assert set(filtered_result["age"].tolist()) == {30, 35} - # WHEN querying the ordered virtual table - ordered_result = await virtual_table_ordered.query_async( - f"SELECT * FROM {virtual_table_ordered.id}", - synapse_client=self.syn, - timeout=QUERY_TIMEOUT_SEC, + # WHEN updating the SQL to order rows + virtual_table.defining_sql = f"SELECT * FROM {table.id} ORDER BY age DESC" + virtual_table = await virtual_table.store_async(synapse_client=self.syn) + + ordered_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", + synapse_client=self.syn, + timeout=QUERY_TIMEOUT_SEC, + ), + condition=lambda r: r["age"].tolist() == [35, 30, 25], + description="virtual table ordered by age desc", ) # THEN data should be in the specified order @@ -315,12 +322,13 @@ async def test_virtual_table_data_synchronization( virtual_table = await virtual_table.store_async(synapse_client=self.syn) self.schedule_for_cleanup(virtual_table.id) - # Wait for the virtual table to be ready - await asyncio.sleep(2) - # WHEN querying the virtual table with empty source table - empty_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + empty_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 0, + description="virtual table ready with no data", ) # THEN no data should be returned @@ -330,12 +338,13 @@ async def test_virtual_table_data_synchronization( data = pd.DataFrame({"name": ["Alice", "Bob"], "age": [30, 25]}) await table.store_rows_async(data, synapse_client=self.syn) - # Wait for the updates to propagate - await asyncio.sleep(2) - # AND querying the virtual table again - added_data_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + added_data_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 2, + description="virtual table reflects added data", ) # THEN the virtual table should reflect the new data @@ -348,12 +357,13 @@ async def test_virtual_table_data_synchronization( query=f"SELECT ROW_ID, ROW_VERSION FROM {table.id}", synapse_client=self.syn ) - # Wait for changes to propagate - await asyncio.sleep(2) - # AND querying the virtual table again - removed_data_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + removed_data_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 0, + description="virtual table reflects removed data", ) # THEN the virtual table should reflect the removed data @@ -373,12 +383,13 @@ async def test_virtual_table_sql_updates( virtual_table = await virtual_table.store_async(synapse_client=self.syn) self.schedule_for_cleanup(virtual_table.id) - # Wait for the virtual table to be ready - await asyncio.sleep(2) - # WHEN querying the virtual table with initial SQL - initial_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + initial_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 3, + description="virtual table ready with initial SQL", ) # THEN all columns should be present @@ -391,12 +402,13 @@ async def test_virtual_table_sql_updates( virtual_table.defining_sql = f"SELECT name, city FROM {table.id}" virtual_table = await virtual_table.store_async(synapse_client=self.syn) - # Wait for the update to propagate - await asyncio.sleep(2) - # AND querying the virtual table again - updated_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + updated_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 3 and "age" not in r.columns, + description="virtual table reflects SQL change", ) # THEN the result should reflect the SQL change @@ -451,12 +463,13 @@ async def test_virtual_table_with_aggregation(self, project_model: Project) -> N virtual_table = await virtual_table.store_async(synapse_client=self.syn) self.schedule_for_cleanup(virtual_table.id) - # Wait for virtual table to be ready - await asyncio.sleep(2) - # WHEN querying the aggregation virtual table - query_result = await virtual_table.query_async( - f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + query_result = await _query_until( + lambda: virtual_table.query_async( + f"SELECT * FROM {virtual_table.id}", synapse_client=self.syn + ), + condition=lambda r: len(r) == 3, + description="virtual table aggregation ready", ) # THEN the result should contain the aggregated data diff --git a/tests/integration/synapseclient/models/async/test_wiki_async.py b/tests/integration/synapseclient/models/async/test_wiki_async.py index 92469d3ea..e09fb27cf 100644 --- a/tests/integration/synapseclient/models/async/test_wiki_async.py +++ b/tests/integration/synapseclient/models/async/test_wiki_async.py @@ -14,6 +14,7 @@ from synapseclient.core import utils from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import ( + Folder, Project, WikiHeader, WikiHistorySnapshot, @@ -28,16 +29,26 @@ class TestWikiPageBasicOperations: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Basic Operations Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) @@ -143,16 +154,26 @@ class TestWikiPageAttachments: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Attachments Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) @@ -517,16 +538,26 @@ class TestWikiPageMarkdown: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Markdown Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) @@ -674,16 +705,26 @@ class TestWikiPageVersioning: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Versioning Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) @@ -765,16 +806,26 @@ class TestWikiHeader: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Header Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) @@ -810,18 +861,28 @@ class TestWikiPageCopy: @pytest.fixture(scope="class") async def source_wiki_tree( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> dict: - """Create a source project with a three-level wiki tree. + """Create a source entity with a three-level wiki tree. The tree is root -> sub -> sub_sub. The sub and sub_sub pages each have a file attachment. The root page markdown contains an internal link to the sub page and a reference to a fake entity ID used to test entity_map rewriting. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki tree via a Folder created inside the session-shared + project rather than creating its own Project. """ - project = Project(name=f"Test Wiki Copy Source_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + owner_folder = await Folder( + name=f"Test Wiki Copy Source Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(owner_folder.id) + project = owner_folder root_wiki = await WikiPage( owner_id=project.id, @@ -893,13 +954,23 @@ async def source_wiki_tree( @pytest.fixture(scope="function") async def destination_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a fresh destination project for each test.""" - project = Project(name=f"Test Wiki Copy Destination_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) - return project + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, + ) -> Folder: + """Create a fresh destination Folder for each test. + + Each test writes a new root wiki page into this fixture, so it must + stay function-scoped for isolation. A Folder inside the session-shared + project is a valid wiki owner and much cheaper to create than a Project. + """ + folder = await Folder( + name=f"Test Wiki Copy Destination Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) + return folder @staticmethod async def _read_markdown( @@ -938,7 +1009,7 @@ async def _attachment_file_names( async def test_copy_entire_wiki_tree( self, source_wiki_tree: dict, - destination_project: Project, + destination_project: Folder, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> None: @@ -1032,7 +1103,7 @@ async def test_copy_entire_wiki_tree( async def test_copy_wiki_sub_tree( self, source_wiki_tree: dict, - destination_project: Project, + destination_project: Folder, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> None: @@ -1063,7 +1134,7 @@ async def test_copy_wiki_sub_tree( async def test_copy_wiki_into_existing_destination_page( self, source_wiki_tree: dict, - destination_project: Project, + destination_project: Folder, syn: Synapse, schedule_for_cleanup: Callable[..., None], ) -> None: @@ -1128,16 +1199,15 @@ async def test_copy_wiki_from_entity_without_wiki( source_wiki_tree: dict, syn: Synapse, schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> None: """Test that copying from an entity that has no wiki returns an empty list instead of raising an error.""" - # GIVEN a source project without any wiki pages - empty_source_project = Project( - name=f"Test Wiki Copy Empty Source_" + str(uuid.uuid4()) - ) - empty_source_project = await empty_source_project.store_async( - synapse_client=syn - ) + # GIVEN a source Folder without any wiki pages + empty_source_project = await Folder( + name=f"Test Wiki Copy Empty Source Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) schedule_for_cleanup(empty_source_project.id) # WHEN copying its wiki to another entity. No destination project is @@ -1159,16 +1229,26 @@ class TestWikiOrderHint: @pytest.fixture(scope="class") async def wiki_page_fixture( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + self, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + project_model: Project, ) -> WikiPage: - """Create a root wiki page fixture shared across tests in this class.""" - project = Project(name=f"Test Wiki Project_" + str(uuid.uuid4())) - project = await project.store_async(synapse_client=syn) - schedule_for_cleanup(project.id) + """Create a root wiki page fixture shared across tests in this class. + + Synapse allows only one root wiki page per owner entity, so this class + owns its wiki via a Folder created inside the session-shared project + rather than creating its own Project. + """ + folder = await Folder( + name=f"Test Wiki Order Hint Folder_{uuid.uuid4()}", + parent_id=project_model.id, + ).store_async(synapse_client=syn) + schedule_for_cleanup(folder.id) wiki_title = f"Root Wiki Page {str(uuid.uuid4())}" wiki_markdown = "# Root Wiki Page\n\nThis is a root wiki page." wiki_page = WikiPage( - owner_id=project.id, + owner_id=folder.id, title=wiki_title, markdown=wiki_markdown, ) diff --git a/tests/integration/synapseclient/models/synchronous/test_sync_wrapper_smoke.py b/tests/integration/synapseclient/models/synchronous/test_sync_wrapper_smoke.py index 22b2ae305..b804ff604 100644 --- a/tests/integration/synapseclient/models/synchronous/test_sync_wrapper_smoke.py +++ b/tests/integration/synapseclient/models/synchronous/test_sync_wrapper_smoke.py @@ -15,7 +15,6 @@ import pytest from synapseclient import Synapse -from synapseclient.core import utils from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.models import ( Column, @@ -76,11 +75,11 @@ def test_project_store_get_delete(self) -> None: def test_file_store_and_get(self, project_model: Project) -> None: """Verify File store/get sync wrappers work.""" - # GIVEN a file - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) + # GIVEN a file. Only the store/get wrapper mechanics are under test here + # (never the file's content), so an external URL avoids a real upload. file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, name=f"sync_smoke_file_{uuid.uuid4()}.txt", description="Sync wrapper smoke test", parent_id=project_model.id, diff --git a/tests/integration/synapseclient/operations/async/test_delete_operations_async.py b/tests/integration/synapseclient/operations/async/test_delete_operations_async.py index 2ec6bbc88..6c7b8285b 100644 --- a/tests/integration/synapseclient/operations/async/test_delete_operations_async.py +++ b/tests/integration/synapseclient/operations/async/test_delete_operations_async.py @@ -25,11 +25,9 @@ def init( async def test_delete_file_by_id_string(self, project_model: Project) -> None: """Test deleting a file using a string ID.""" # GIVEN a file stored in synapse - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file for deletion", ) @@ -48,11 +46,9 @@ async def test_delete_file_by_id_string(self, project_model: Project) -> None: async def test_delete_file_by_object(self, project_model: Project) -> None: """Test deleting a file using a File object.""" # GIVEN a file stored in synapse - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file for deletion", ) @@ -249,11 +245,9 @@ async def test_delete_version_only_without_version_raises_error( ) -> None: """Test that version_only=True without a version number raises an error.""" # GIVEN a file without version_number set - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file", ) @@ -310,11 +304,9 @@ async def test_delete_with_dot_notation_without_version_only_raises_error( ) -> None: """Test that using dot notation without version_only=True raises an error.""" # GIVEN a file with multiple versions - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file version 1", ) @@ -373,11 +365,9 @@ async def test_delete_version_param_without_conflict_no_warning( ) -> None: """Test that no warning is logged when version parameter is used without conflict.""" # GIVEN a file with multiple versions - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file version 1", ) @@ -434,11 +424,9 @@ async def test_no_warning_when_version_only_false_despite_conflict( ) -> None: """Test that no warning is logged when version_only=False even with version conflict.""" # GIVEN a file - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file", ) @@ -466,11 +454,9 @@ async def test_delete_file_with_version_number_none_no_warning( ) -> None: """Test that no warning when entity.version_number is explicitly None.""" # GIVEN a file with multiple versions - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, description="Test file version 1", ) diff --git a/tests/integration/synapseclient/operations/async/test_download_list_operations_async.py b/tests/integration/synapseclient/operations/async/test_download_list_operations_async.py index 33e7e3ff6..7e066f1a4 100644 --- a/tests/integration/synapseclient/operations/async/test_download_list_operations_async.py +++ b/tests/integration/synapseclient/operations/async/test_download_list_operations_async.py @@ -592,7 +592,7 @@ async def test_download_list_manifest_with_custom_csv_descriptor( assert "\r" not in content, "Expected LF-only line endings; found CR" # AND there is no header row -- the first non-empty line is the data row - # NOTE: The cart is per-user and shared across all parallel workers (-n 8). + # NOTE: The cart is per-user and shared across all parallel workers (-n 4). # Other tests running concurrently can add items to the cart, so the manifest # may contain more than just this test's file. lines = [line for line in content.split("\n") if line] diff --git a/tests/integration/synapseclient/operations/async/test_factory_operations_store_async.py b/tests/integration/synapseclient/operations/async/test_factory_operations_store_async.py index b30691b69..4ef7b24fb 100644 --- a/tests/integration/synapseclient/operations/async/test_factory_operations_store_async.py +++ b/tests/integration/synapseclient/operations/async/test_factory_operations_store_async.py @@ -60,10 +60,9 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: def create_file_instance(self) -> File: """Helper method to create a test file.""" - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) return File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, description="Test file for store factory operations", content_type="text/plain", name=f"test_file_{str(uuid.uuid4())[:8]}.txt", diff --git a/tests/integration/synapseclient/operations/async/test_utility_operations_async.py b/tests/integration/synapseclient/operations/async/test_utility_operations_async.py index a58eb8fd9..e7c1e13cb 100644 --- a/tests/integration/synapseclient/operations/async/test_utility_operations_async.py +++ b/tests/integration/synapseclient/operations/async/test_utility_operations_async.py @@ -57,13 +57,12 @@ async def test_find_entity_id_async_file_by_name_in_parent( self, project_model: Project ) -> None: """Test finding a file by name within a parent folder asynchronously.""" - # GIVEN a file stored in a project - filename = utils.make_bogus_uuid_file() - self.schedule_for_cleanup(filename) - + # GIVEN a file stored in a project. Only the name-based lookup is under + # test here, so an external URL avoids a real upload. file_name = f"test_file_{str(uuid.uuid4())[:8]}.txt" file = File( - path=filename, + external_url=f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + synapse_store=False, parent_id=project_model.id, name=file_name, description="Test file for find_entity_id_async", diff --git a/tests/integration/synapseutils/test_synapseutils_copy.py b/tests/integration/synapseutils/test_synapseutils_copy.py index 7a4898994..7bbd8d0d7 100644 --- a/tests/integration/synapseutils/test_synapseutils_copy.py +++ b/tests/integration/synapseutils/test_synapseutils_copy.py @@ -152,9 +152,16 @@ def execute_test_copy(syn: Synapse, schedule_for_cleanup): # ------------------------------------ # TEST COPY LINKS # ------------------------------------ - second_file = utils.make_bogus_data_file() - # schedule_for_cleanup(filename) - second_file_entity = syn.store(File(second_file, parent=project_entity)) + # Only used as a Link target below (referenced by id, never downloaded), so an + # external URL avoids a real upload. + second_file_entity = syn.store( + File( + f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name="bogus_link_target.txt", + parent=project_entity, + synapseStore=False, + ) + ) link_entity = Link(second_file_entity.id, parent=folder_entity.id) link_entity = syn.store(link_entity) diff --git a/tests/integration/synapseutils/test_synapseutils_walk.py b/tests/integration/synapseutils/test_synapseutils_walk.py index e2c4a98a6..ae8dc205c 100644 --- a/tests/integration/synapseutils/test_synapseutils_walk.py +++ b/tests/integration/synapseutils/test_synapseutils_walk.py @@ -4,7 +4,6 @@ import pytest from func_timeout import FunctionTimedOut, func_set_timeout -import synapseclient.core.utils as utils import synapseutils from synapseclient import File, Folder, Project @@ -23,16 +22,23 @@ async def test_walk(syn, schedule_for_cleanup): # When running with multiple threads it can lock up and do nothing until pipeline is killed at 6hrs @func_set_timeout(120) def execute_test_walk(syn, schedule_for_cleanup): + # walk only ever inspects entity names/ids/structure, never downloads or reads + # file content, so every File below uses an external URL to avoid a real upload. walked = [] - firstfile = utils.make_bogus_data_file() - schedule_for_cleanup(firstfile) project_entity = syn.store(Project(name=str(uuid.uuid4()))) schedule_for_cleanup(project_entity.id) folder_entity = syn.store(Folder(name=str(uuid.uuid4()), parent=project_entity)) schedule_for_cleanup(folder_entity.id) second_folder = syn.store(Folder(name=str(uuid.uuid4()), parent=project_entity)) schedule_for_cleanup(second_folder.id) - file_entity = syn.store(File(firstfile, parent=project_entity)) + file_entity = syn.store( + File( + f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"bogus_file_{uuid.uuid4()}.txt", + parent=project_entity, + synapseStore=False, + ) + ) schedule_for_cleanup(file_entity.id) walked.append( @@ -48,13 +54,23 @@ def execute_test_walk(syn, schedule_for_cleanup): nested_folder = syn.store(Folder(name=str(uuid.uuid4()), parent=folder_entity)) schedule_for_cleanup(nested_folder.id) - secondfile = utils.make_bogus_data_file() - schedule_for_cleanup(secondfile) - second_file = syn.store(File(secondfile, parent=nested_folder)) + second_file = syn.store( + File( + f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"bogus_file_{uuid.uuid4()}.txt", + parent=nested_folder, + synapseStore=False, + ) + ) schedule_for_cleanup(second_file.id) - thirdfile = utils.make_bogus_data_file() - schedule_for_cleanup(thirdfile) - third_file = syn.store(File(thirdfile, parent=second_folder)) + third_file = syn.store( + File( + f"https://example.com/bogus-file-{uuid.uuid4()}.txt", + name=f"bogus_file_{uuid.uuid4()}.txt", + parent=second_folder, + synapseStore=False, + ) + ) schedule_for_cleanup(third_file.id) walked.append(