diff --git a/tests/__init__.py b/tests/__init__.py index e69de29..12078ae 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,4 @@ +"""Test package for omni-tool-runtime. + +Developer: Manish Kumar +""" diff --git a/tests/test_azureblob_uploader.py b/tests/test_azureblob_uploader.py index 045e02e..77601e8 100644 --- a/tests/test_azureblob_uploader.py +++ b/tests/test_azureblob_uploader.py @@ -6,6 +6,8 @@ pytest tests/test_azureblob_uploader.py -v pytest tests/test_azureblob_uploader.py \ --cov=omni_tool_runtime/uploaders/azureblob_uploader --cov-report=term-missing -v + +Developer: Manish Kumar """ from __future__ import annotations @@ -23,6 +25,7 @@ def _install_azure_stubs(): + """Register fake azure/azure.storage/azure.storage.blob/azure.identity modules in sys.modules so the uploader imports cleanly without the real azure SDK.""" azure = types.ModuleType("azure") azure_storage = types.ModuleType("azure.storage") azure_storage_blob = types.ModuleType("azure.storage.blob") @@ -56,6 +59,7 @@ def _install_azure_stubs(): def _make_uploader(**kw) -> AzureBlobUploader: + """Build an AzureBlobUploader with sensible defaults, overridable per test.""" defaults = dict(account_name="myaccount", auth="managed_identity", connection_string=None) return AzureBlobUploader(**{**defaults, **kw}) @@ -74,20 +78,26 @@ def _mock_blob_service(): class TestAzureBlobUploaderConstruction: + """The AzureBlobUploader dataclass stores account_name/auth/connection_string verbatim, defaulting to managed-identity auth.""" def test_account_name_stored(self): + """Store the given account_name unchanged.""" assert _make_uploader(account_name="acct").account_name == "acct" def test_default_auth_is_managed_identity(self): + """Default auth to managed_identity when not given.""" assert AzureBlobUploader(account_name="acct").auth == "managed_identity" def test_explicit_auth_connection_string(self): + """Store an explicitly given connection_string auth mode.""" u = _make_uploader(auth="connection_string", connection_string="cs") assert u.auth == "connection_string" def test_connection_string_default_is_none(self): + """Default connection_string to None when not given.""" assert AzureBlobUploader(account_name="acct").connection_string is None def test_connection_string_stored(self): + """Store the given connection_string unchanged.""" cs = "DefaultEndpointsProtocol=https;..." assert _make_uploader(connection_string=cs).connection_string == cs @@ -98,10 +108,14 @@ def test_connection_string_stored(self): class TestClientConnectionString: + """_client() builds a BlobServiceClient via from_connection_string() and never touches DefaultAzureCredential.""" + def _make(self, connection_string="cs://fake"): + """Build an uploader configured for connection_string auth.""" return _make_uploader(auth="connection_string", connection_string=connection_string) def test_calls_from_connection_string(self): + """_client() builds the BlobServiceClient via from_connection_string() using the stored connection string.""" mock_bsc = MagicMock() _azure_storage_blob_stub.BlobServiceClient = mock_bsc uploader = self._make() @@ -109,6 +123,7 @@ def test_calls_from_connection_string(self): mock_bsc.from_connection_string.assert_called_once_with("cs://fake") def test_returns_blob_service_client(self): + """_client() returns the BlobServiceClient instance produced by from_connection_string().""" mock_bsc = MagicMock() sentinel = object() mock_bsc.from_connection_string.return_value = sentinel @@ -116,16 +131,19 @@ def test_returns_blob_service_client(self): assert self._make()._client() is sentinel def test_missing_connection_string_raises(self): + """Reject connection_string auth when no connection string is configured (None).""" uploader = _make_uploader(auth="connection_string", connection_string=None) with pytest.raises(RuntimeError, match="connection_string auth requires connection_string"): uploader._client() def test_empty_connection_string_raises(self): + """Reject connection_string auth when the connection string is an empty string.""" uploader = _make_uploader(auth="connection_string", connection_string="") with pytest.raises(RuntimeError, match="connection_string auth requires connection_string"): uploader._client() def test_does_not_use_default_azure_credential(self): + """Never construct a DefaultAzureCredential when using connection_string auth.""" mock_dac = MagicMock() _azure_identity_stub.DefaultAzureCredential = mock_dac _azure_storage_blob_stub.BlobServiceClient = MagicMock() @@ -139,10 +157,14 @@ def test_does_not_use_default_azure_credential(self): class TestClientManagedIdentity: + """_client() builds a BlobServiceClient authenticated via DefaultAzureCredential and never calls from_connection_string().""" + def _make(self): + """Build an uploader configured for managed_identity auth against a fixed account name.""" return _make_uploader(auth="managed_identity", account_name="storageacct") def test_creates_default_azure_credential(self): + """_client() constructs a DefaultAzureCredential with interactive browser login excluded.""" mock_dac = MagicMock() mock_bsc = MagicMock() _azure_identity_stub.DefaultAzureCredential = mock_dac @@ -151,6 +173,7 @@ def test_creates_default_azure_credential(self): mock_dac.assert_called_once_with(exclude_interactive_browser_credential=True) def test_builds_correct_account_url(self): + """_client() builds the account_url as https://.blob.core.windows.net.""" mock_dac = MagicMock() mock_bsc = MagicMock() _azure_identity_stub.DefaultAzureCredential = mock_dac @@ -160,6 +183,7 @@ def test_builds_correct_account_url(self): assert call_kwargs["account_url"] == "https://storageacct.blob.core.windows.net" def test_credential_passed_to_blob_service_client(self): + """_client() passes the constructed DefaultAzureCredential through to BlobServiceClient.""" fake_cred = object() mock_dac = MagicMock(return_value=fake_cred) mock_bsc = MagicMock() @@ -169,6 +193,7 @@ def test_credential_passed_to_blob_service_client(self): assert mock_bsc.call_args.kwargs["credential"] is fake_cred def test_does_not_call_from_connection_string(self): + """Never call from_connection_string() when using managed_identity auth.""" mock_dac = MagicMock() mock_bsc = MagicMock() _azure_identity_stub.DefaultAzureCredential = mock_dac @@ -177,6 +202,7 @@ def test_does_not_call_from_connection_string(self): mock_bsc.from_connection_string.assert_not_called() def test_returns_blob_service_client_instance(self): + """_client() returns the BlobServiceClient instance built for managed-identity auth.""" sentinel = object() mock_dac = MagicMock() mock_bsc = MagicMock(return_value=sentinel) @@ -191,7 +217,10 @@ def test_returns_blob_service_client_instance(self): class TestClientMissingPackages: + """_client() must fail with a clear, actionable error when the azure-storage-blob or azure-identity packages are not installed.""" + def test_missing_azure_storage_blob_raises(self): + """Raise RuntimeError naming azure-storage-blob when that package is unavailable.""" uploader = _make_uploader(auth="managed_identity") with ( patch.dict(sys.modules, {"azure.storage.blob": None}), @@ -200,6 +229,7 @@ def test_missing_azure_storage_blob_raises(self): uploader._client() def test_missing_azure_identity_raises(self): + """Raise RuntimeError naming azure-identity when that package is unavailable under managed-identity auth.""" uploader = _make_uploader(auth="managed_identity") # azure.storage.blob is present but azure.identity is missing real_bsc = _azure_storage_blob_stub.BlobServiceClient @@ -212,6 +242,7 @@ def test_missing_azure_identity_raises(self): _azure_storage_blob_stub.BlobServiceClient = real_bsc def test_missing_blob_error_message_mentions_install(self): + """Point to the omnibioai-tool-runtime install extra in the missing-azure-storage-blob error message.""" uploader = _make_uploader(auth="managed_identity") with ( patch.dict(sys.modules, {"azure.storage.blob": None}), @@ -226,7 +257,10 @@ def test_missing_blob_error_message_mentions_install(self): class TestUploadBytes: + """upload_bytes() forwards container/blob_path/data/content_type to the Azure client, lowercasing the container name.""" + def _call(self, uploader=None, **kw): + """Call upload_bytes with defaults, injecting a mocked BlobServiceClient via _client().""" uploader = uploader or _make_uploader() defaults = dict( container="MyContainer", @@ -240,43 +274,53 @@ def _call(self, uploader=None, **kw): return mock_svc, mock_bc def test_get_blob_client_called(self): + """Call get_blob_client exactly once per upload.""" mock_svc, _ = self._call() mock_svc.get_blob_client.assert_called_once() def test_container_lowercased(self): + """Lowercase a mixed-case container name before passing it to get_blob_client (Azure containers are case-sensitive and conventionally lowercase).""" mock_svc, _ = self._call(container="MyContainer") assert mock_svc.get_blob_client.call_args.kwargs["container"] == "mycontainer" def test_already_lowercase_container_unchanged(self): + """Leave an already-lowercase container name unchanged.""" mock_svc, _ = self._call(container="mycontainer") assert mock_svc.get_blob_client.call_args.kwargs["container"] == "mycontainer" def test_blob_path_passed_correctly(self): + """Forward the given blob_path unchanged to get_blob_client.""" mock_svc, _ = self._call(blob_path="output/run-1/results.json") assert mock_svc.get_blob_client.call_args.kwargs["blob"] == "output/run-1/results.json" def test_upload_blob_called(self): + """Call upload_blob exactly once per upload.""" _, mock_bc = self._call() mock_bc.upload_blob.assert_called_once() def test_data_passed_to_upload_blob(self): + """Forward the given data bytes as upload_blob's first positional argument unmodified.""" payload = b'{"result": 42}' _, mock_bc = self._call(data=payload) assert mock_bc.upload_blob.call_args.args[0] == payload def test_overwrite_is_true(self): + """Always pass overwrite=True to upload_blob so re-runs replace prior results.""" _, mock_bc = self._call() assert mock_bc.upload_blob.call_args.kwargs["overwrite"] is True def test_content_type_passed(self): + """Forward the given content_type to upload_blob.""" _, mock_bc = self._call(content_type="text/plain") assert mock_bc.upload_blob.call_args.kwargs["content_type"] == "text/plain" def test_default_content_type_json(self): + """Forward application/json as content_type when that is the given content_type.""" _, mock_bc = self._call(content_type="application/json") assert mock_bc.upload_blob.call_args.kwargs["content_type"] == "application/json" def test_client_called_once(self): + """Call _client() exactly once per upload.""" uploader = _make_uploader() mock_svc, _ = _mock_blob_service() with patch.object(uploader, "_client", return_value=mock_svc) as mock_client: @@ -289,6 +333,7 @@ def test_client_called_once(self): mock_client.assert_called_once() def test_returns_none(self): + """upload_bytes returns None (the Azure call result is not surfaced to the caller).""" uploader = _make_uploader() mock_svc, _ = _mock_blob_service() with patch.object(uploader, "_client", return_value=mock_svc): @@ -301,12 +346,14 @@ def test_returns_none(self): assert result is None def test_upload_blob_receives_bytes_not_string(self): + """Pass the payload through to upload_blob as bytes, not a str.""" payload = b"binary-content" _, mock_bc = self._call(data=payload) uploaded = mock_bc.upload_blob.call_args.args[0] assert isinstance(uploaded, bytes) def test_stdout_logged(self, capsys): + """Log the container and blob path (structural, non-sensitive identifiers) to stdout on upload.""" uploader = _make_uploader() mock_svc, _ = _mock_blob_service() with patch.object(uploader, "_client", return_value=mock_svc): @@ -327,7 +374,10 @@ def test_stdout_logged(self, capsys): class TestUploadBytesEndToEndConnectionString: + """Full upload_bytes() flow using connection_string auth, from client construction through the final upload_blob call.""" + def test_full_flow_connection_string(self): + """Full upload_bytes flow using connection_string auth: builds the client via from_connection_string, uploads to the exact container/blob/body/content-type given.""" conn = "DefaultEndpointsProtocol=https;AccountName=x;..." uploader = _make_uploader( auth="connection_string", @@ -360,7 +410,10 @@ def test_full_flow_connection_string(self): class TestUploadBytesEndToEndManagedIdentity: + """Full upload_bytes() flow using managed_identity auth, from client construction through the final upload_blob call.""" + def test_full_flow_managed_identity(self): + """Full upload_bytes flow using managed_identity auth: builds the client via DefaultAzureCredential, lowercases the container, and uploads the exact blob/body/content-type given.""" uploader = _make_uploader(auth="managed_identity", account_name="storageacct") mock_svc, mock_bc = _mock_blob_service() diff --git a/tests/test_contract.py b/tests/test_contract.py index 96dc673..8b81d1c 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -4,6 +4,8 @@ Run: pytest tests/test_contract.py -v pytest tests/test_contract.py --cov=omni_tool_runtime/contract --cov-report=term-missing -v + +Developer: Manish Kumar """ from __future__ import annotations @@ -20,7 +22,9 @@ class TestToolContract: + """The ToolContract dataclass stores tool_id/run_id/inputs/resources/result_uri verbatim and stays mutable.""" def _make(self, **kw) -> ToolContract: + """Build a ToolContract with sensible defaults, overridable per test.""" defaults = dict( tool_id="tool-1", run_id="run-1", @@ -31,39 +35,49 @@ def _make(self, **kw) -> ToolContract: return ToolContract(**{**defaults, **kw}) def test_stores_tool_id(self): + """Store the given tool_id unchanged.""" assert self._make(tool_id="my-tool").tool_id == "my-tool" def test_stores_run_id(self): + """Store the given run_id unchanged.""" assert self._make(run_id="run-abc").run_id == "run-abc" def test_stores_inputs(self): + """Store the given inputs dict unchanged.""" inputs = {"alpha": 1, "beta": "x"} assert self._make(inputs=inputs).inputs == inputs def test_stores_resources(self): + """Store the given resources dict unchanged.""" resources = {"memory": "4Gi", "gpu": 0} assert self._make(resources=resources).resources == resources def test_stores_result_uri(self): + """Store the given result_uri unchanged.""" uri = "s3://my-bucket/path/result.json" assert self._make(result_uri=uri).result_uri == uri def test_empty_inputs_ok(self): + """Accept an empty inputs dict without error.""" assert self._make(inputs={}).inputs == {} def test_empty_resources_ok(self): + """Accept an empty resources dict without error.""" assert self._make(resources={}).resources == {} def test_empty_strings_ok(self): + """Accept empty strings for tool_id/run_id/result_uri without error.""" c = self._make(tool_id="", run_id="", result_uri="") assert c.tool_id == "" and c.run_id == "" and c.result_uri == "" def test_is_dataclass_instance(self): + """ToolContract is a real dataclass, not a plain object mimicking one.""" import dataclasses assert dataclasses.is_dataclass(self._make()) def test_fields_are_assignable(self): + """ToolContract fields are mutable after construction.""" c = self._make() c.tool_id = "updated" assert c.tool_id == "updated" @@ -75,13 +89,16 @@ def test_fields_are_assignable(self): class TestReadContractFromEnvHappyPath: + """read_contract_from_env() builds a ToolContract from well-formed TOOL_ID/RUN_ID/RESULT_URI/INPUTS_JSON/RESOURCES_JSON.""" def _call(self, env: dict) -> ToolContract: + """Read a ToolContract from a given environment mapping.""" import unittest.mock as mock with mock.patch.dict("os.environ", env, clear=True): return read_contract_from_env() def test_returns_tool_contract(self): + """Return a ToolContract instance from well-formed environment variables.""" result = self._call( { "TOOL_ID": "t1", @@ -94,6 +111,7 @@ def test_returns_tool_contract(self): assert isinstance(result, ToolContract) def test_tool_id_read(self): + """Read tool_id verbatim from TOOL_ID.""" result = self._call( { "TOOL_ID": "my-tool", @@ -106,6 +124,7 @@ def test_tool_id_read(self): assert result.tool_id == "my-tool" def test_run_id_read(self): + """Read run_id verbatim from RUN_ID.""" result = self._call( { "TOOL_ID": "", @@ -118,6 +137,7 @@ def test_run_id_read(self): assert result.run_id == "run-xyz" def test_result_uri_read(self): + """Read result_uri verbatim from RESULT_URI.""" result = self._call( { "TOOL_ID": "", @@ -130,6 +150,7 @@ def test_result_uri_read(self): assert result.result_uri == "az://container/out.json" def test_inputs_json_parsed(self): + """Parse INPUTS_JSON into the inputs dict.""" payload = {"vcf": "/data/file.vcf", "threshold": 0.05} result = self._call( { @@ -143,6 +164,7 @@ def test_inputs_json_parsed(self): assert result.inputs == payload def test_resources_json_parsed(self): + """Parse RESOURCES_JSON into the resources dict.""" resources = {"cpu": 4, "memory": "8Gi"} result = self._call( { @@ -156,6 +178,7 @@ def test_resources_json_parsed(self): assert result.resources == resources def test_empty_inputs_json_gives_empty_dict(self): + """Parse an INPUTS_JSON of "{}" into an empty inputs dict.""" result = self._call( { "TOOL_ID": "", @@ -168,6 +191,7 @@ def test_empty_inputs_json_gives_empty_dict(self): assert result.inputs == {} def test_empty_resources_json_gives_empty_dict(self): + """Parse a RESOURCES_JSON of "{}" into an empty resources dict.""" result = self._call( { "TOOL_ID": "", @@ -180,6 +204,7 @@ def test_empty_resources_json_gives_empty_dict(self): assert result.resources == {} def test_nested_inputs_parsed(self): + """Parse nested objects inside INPUTS_JSON without flattening or dropping structure.""" payload = {"filters": {"maf": 0.01, "tags": ["a", "b"]}} result = self._call( { @@ -193,6 +218,7 @@ def test_nested_inputs_parsed(self): assert result.inputs == payload def test_inputs_list_value_ok(self): + """Parse a list-valued field inside INPUTS_JSON without coercion.""" payload = {"samples": ["s1", "s2", "s3"]} result = self._call( { @@ -206,6 +232,7 @@ def test_inputs_list_value_ok(self): assert result.inputs["samples"] == ["s1", "s2", "s3"] def test_all_fields_populated_together(self): + """Populate all five ToolContract fields correctly from one combined environment.""" inputs = {"k": "v"} resources = {"cpu": 1} result = self._call( @@ -230,25 +257,32 @@ def test_all_fields_populated_together(self): class TestReadContractFromEnvDefaults: + """read_contract_from_env() falls back to empty strings/dicts when the corresponding env vars are unset.""" def _call_empty(self) -> ToolContract: + """Read a ToolContract from a completely empty environment.""" import unittest.mock as mock with mock.patch.dict("os.environ", {}, clear=True): return read_contract_from_env() def test_tool_id_defaults_to_empty_string(self): + """Default tool_id to an empty string when TOOL_ID is unset.""" assert self._call_empty().tool_id == "" def test_run_id_defaults_to_empty_string(self): + """Default run_id to an empty string when RUN_ID is unset.""" assert self._call_empty().run_id == "" def test_result_uri_defaults_to_empty_string(self): + """Default result_uri to an empty string when RESULT_URI is unset.""" assert self._call_empty().result_uri == "" def test_inputs_defaults_to_empty_dict(self): + """Default inputs to an empty dict when INPUTS_JSON is unset.""" assert self._call_empty().inputs == {} def test_resources_defaults_to_empty_dict(self): + """Default resources to an empty dict when RESOURCES_JSON is unset.""" assert self._call_empty().resources == {} @@ -258,13 +292,16 @@ def test_resources_defaults_to_empty_dict(self): class TestReadContractFromEnvBadJson: + """read_contract_from_env() raises RuntimeError, naming the offending variable, on malformed INPUTS_JSON/RESOURCES_JSON.""" def _call(self, env: dict): + """Read a ToolContract from a given environment mapping, expected to raise.""" import unittest.mock as mock with mock.patch.dict("os.environ", env, clear=True): return read_contract_from_env() def test_bad_inputs_json_raises_runtime_error(self): + """Raise RuntimeError when INPUTS_JSON is not valid JSON.""" with pytest.raises(RuntimeError): self._call( { @@ -277,6 +314,7 @@ def test_bad_inputs_json_raises_runtime_error(self): ) def test_bad_inputs_json_message_mentions_inputs(self): + """Name INPUTS_JSON in the error message for invalid inputs JSON.""" with pytest.raises(RuntimeError, match="INPUTS_JSON"): self._call( { @@ -289,6 +327,7 @@ def test_bad_inputs_json_message_mentions_inputs(self): ) def test_bad_resources_json_raises_runtime_error(self): + """Raise RuntimeError when RESOURCES_JSON is not valid JSON.""" with pytest.raises(RuntimeError): self._call( { @@ -301,6 +340,7 @@ def test_bad_resources_json_raises_runtime_error(self): ) def test_bad_resources_json_message_mentions_resources(self): + """Name RESOURCES_JSON in the error message for invalid resources JSON.""" with pytest.raises(RuntimeError, match="RESOURCES_JSON"): self._call( { @@ -313,6 +353,7 @@ def test_bad_resources_json_message_mentions_resources(self): ) def test_truncated_inputs_json_raises(self): + """Raise RuntimeError for a truncated (incomplete) INPUTS_JSON payload.""" with pytest.raises(RuntimeError): self._call( { @@ -327,6 +368,7 @@ def test_truncated_inputs_json_raises(self): def test_plain_string_inputs_json_raises(self): # A bare string is valid JSON but not a dict — however json.loads # succeeds so it is stored as-is; this test confirms no error is raised. + """A bare JSON string (not an object) for INPUTS_JSON parses successfully and is stored as-is, without raising.""" result = self._call( { "TOOL_ID": "", @@ -340,6 +382,7 @@ def test_plain_string_inputs_json_raises(self): def test_both_bad_raises_on_inputs_first(self): # inputs is parsed first, so a bad inputs raises before resources is checked + """When both INPUTS_JSON and RESOURCES_JSON are malformed, the inputs error is raised first.""" with pytest.raises(RuntimeError, match="INPUTS_JSON"): self._call( { diff --git a/tests/test_echo_run.py b/tests/test_echo_run.py index ffe7208..56c2d11 100644 --- a/tests/test_echo_run.py +++ b/tests/test_echo_run.py @@ -7,6 +7,8 @@ - Missing inputs.text - RESULT_URI upload dispatch - __main__ block + +Developer: Manish Kumar """ from __future__ import annotations @@ -32,6 +34,7 @@ def _env( result_uri: str = "", inputs_json: str = '{"text": "hello"}', ) -> dict[str, str]: + """Build the TOOL_ID/RUN_ID/RESULT_URI/INPUTS_JSON environment mapping for echo_test main().""" return { "TOOL_ID": tool_id, "RUN_ID": run_id, @@ -44,20 +47,24 @@ def _env( # 1. Environment variable reading # =========================================================================== class TestEnvReading: + """TOOL_ID, RUN_ID, and INPUTS_JSON are read from the environment with documented defaults.""" def test_tool_id_read_from_env(self, capsys): + """Echo back the TOOL_ID value taken from the environment.""" with patch.dict("os.environ", _env(tool_id="my-tool"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["tool_id"] == "my-tool" def test_run_id_read_from_env(self, capsys): + """Echo back the RUN_ID value taken from the environment.""" with patch.dict("os.environ", _env(run_id="run-42"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["run_id"] == "run-42" def test_tool_id_defaults_to_empty_string(self, capsys): + """Default tool_id to an empty string when TOOL_ID is unset.""" env = _env() env.pop("TOOL_ID") with patch.dict("os.environ", env, clear=True): @@ -66,6 +73,7 @@ def test_tool_id_defaults_to_empty_string(self, capsys): assert out["tool_id"] == "" def test_run_id_defaults_to_empty_string(self, capsys): + """Default run_id to an empty string when RUN_ID is unset.""" env = _env() env.pop("RUN_ID") with patch.dict("os.environ", env, clear=True): @@ -94,43 +102,51 @@ def test_inputs_json_defaults_to_empty_object(self, capsys): # 2. Bad INPUTS_JSON # =========================================================================== class TestBadInputsJson: + """Malformed INPUTS_JSON must produce a controlled error result, not a crash.""" def test_returns_2_on_invalid_json(self): + """Return exit code 2 when INPUTS_JSON is not valid JSON.""" with patch.dict("os.environ", _env(inputs_json="not-json"), clear=True): rc = main() assert rc == 2 def test_ok_is_false_on_invalid_json(self, capsys): + """Report ok: false in the result body when INPUTS_JSON is invalid.""" with patch.dict("os.environ", _env(inputs_json="{bad}"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["ok"] is False def test_error_message_mentions_bad_inputs_json(self, capsys): + """Explain the invalid-JSON failure in the error message.""" with patch.dict("os.environ", _env(inputs_json="[[["), clear=True): main() out = json.loads(capsys.readouterr().out) assert "bad INPUTS_JSON" in out["error"] def test_tool_id_included_in_error_response(self, capsys): + """Include tool_id in the error response even when INPUTS_JSON fails to parse.""" with patch.dict("os.environ", _env(tool_id="t1", inputs_json="!!!"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["tool_id"] == "t1" def test_run_id_included_in_error_response(self, capsys): + """Include run_id in the error response even when INPUTS_JSON fails to parse.""" with patch.dict("os.environ", _env(run_id="r99", inputs_json="???"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["run_id"] == "r99" def test_upload_not_called_on_bad_json(self): + """Skip the RESULT_URI upload entirely when INPUTS_JSON fails to parse.""" with patch.dict("os.environ", _env(result_uri="s3://b/k", inputs_json="bad"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() mock_upload.assert_not_called() def test_truncated_json_returns_2(self): + """Return exit code 2 for a truncated (incomplete) JSON payload.""" with patch.dict("os.environ", _env(inputs_json='{"text":'), clear=True): rc = main() assert rc == 2 @@ -140,19 +156,23 @@ def test_truncated_json_returns_2(self): # 3. Missing inputs.text # =========================================================================== class TestMissingText: + """A well-formed inputs object lacking the required 'text' field is a soft (non-crashing) error.""" def test_returns_0_when_text_missing(self): + """Return exit code 0 even when inputs.text is missing — this is a handled, not fatal, condition.""" with patch.dict("os.environ", _env(inputs_json='{"other": 1}'), clear=True): rc = main() assert rc == 0 def test_ok_is_false_when_text_missing(self, capsys): + """Report ok: false in the result body when inputs.text is missing.""" with patch.dict("os.environ", _env(inputs_json="{}"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["ok"] is False def test_error_mentions_missing_text(self, capsys): + """Explain the missing-text failure in the error message.""" with patch.dict("os.environ", _env(inputs_json="{}"), clear=True): main() out = json.loads(capsys.readouterr().out) @@ -168,18 +188,21 @@ def test_inputs_not_echoed_back_in_log(self, capsys): assert out["input_summary"] == {"foo": {"type": "str", "len": 3, "ref": out["input_summary"]["foo"]["ref"]}} def test_tool_id_present_in_missing_text_response(self, capsys): + """Include tool_id in the response even when inputs.text is missing.""" with patch.dict("os.environ", _env(tool_id="t2", inputs_json="{}"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["tool_id"] == "t2" def test_run_id_present_in_missing_text_response(self, capsys): + """Include run_id in the response even when inputs.text is missing.""" with patch.dict("os.environ", _env(run_id="r2", inputs_json="{}"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["run_id"] == "r2" def test_upload_not_called_when_no_result_uri(self): + """Skip the RESULT_URI upload in local mode even on the missing-text error path.""" with patch.dict("os.environ", _env(inputs_json="{}"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() @@ -190,13 +213,16 @@ def test_upload_not_called_when_no_result_uri(self): # 4. Happy path — local mode (no RESULT_URI) # =========================================================================== class TestHappyPathLocalMode: + """With RESULT_URI unset (local mode), the tool exits 0 and prints only a redacted summary.""" def test_returns_0(self): + """Return exit code 0 on a normal local-mode run.""" with patch.dict("os.environ", _env(), clear=True): rc = main() assert rc == 0 def test_ok_is_true(self, capsys): + """Report ok: true in the result body on a normal local-mode run.""" with patch.dict("os.environ", _env(), clear=True): main() out = json.loads(capsys.readouterr().out) @@ -226,12 +252,14 @@ def test_echo_value_not_in_printed_log(self, capsys): assert parsed["echo_summary"] == {"type": "str", "len": 5, "ref": parsed["echo_summary"]["ref"]} def test_tool_id_in_response(self, capsys): + """Include tool_id in the local-mode response body.""" with patch.dict("os.environ", _env(tool_id="echo-tool"), clear=True): main() out = json.loads(capsys.readouterr().out) assert out["tool_id"] == "echo-tool" def test_run_id_in_response(self, capsys): + """Include run_id in the local-mode response body.""" with patch.dict("os.environ", _env(run_id="run-77"), clear=True): main() out = json.loads(capsys.readouterr().out) @@ -246,6 +274,7 @@ def test_results_key_present(self, capsys): assert "results" not in out def test_upload_not_called_in_local_mode(self): + """Skip the RESULT_URI upload entirely when RESULT_URI is empty.""" with patch.dict("os.environ", _env(result_uri=""), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() @@ -260,6 +289,7 @@ def test_empty_text_string_is_valid(self, capsys): assert out["echo_summary"] == {"type": "str", "len": 0, "ref": out["echo_summary"]["ref"]} def test_numeric_text_value(self, capsys): + """Accept a numeric inputs.text value and summarize it by type rather than echoing it.""" with patch.dict("os.environ", _env(inputs_json='{"text": 42}'), clear=True): main() out = json.loads(capsys.readouterr().out) @@ -267,6 +297,7 @@ def test_numeric_text_value(self, capsys): assert out["echo_summary"] == {"type": "int"} def test_output_is_valid_json(self, capsys): + """Print a body that parses as valid JSON.""" with patch.dict("os.environ", _env(), clear=True): main() raw = capsys.readouterr().out @@ -285,20 +316,24 @@ def test_output_is_indented_json(self, capsys): # 5. Happy path — cloud mode (RESULT_URI set) # =========================================================================== class TestHappyPathCloudMode: + """With RESULT_URI set (cloud mode), the raw result is uploaded while stdout stays redacted.""" def test_returns_0_in_cloud_mode(self): + """Return exit code 0 when RESULT_URI is set and the upload succeeds.""" with patch.dict("os.environ", _env(result_uri="s3://bucket/key"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri"): rc = main() assert rc == 0 def test_upload_called_once(self): + """Call upload_to_result_uri exactly once when RESULT_URI is set.""" with patch.dict("os.environ", _env(result_uri="s3://bucket/key"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() mock_upload.assert_called_once() def test_upload_receives_correct_result_uri(self): + """Pass the exact RESULT_URI value through to upload_to_result_uri.""" uri = "s3://my-bucket/results/out.json" with patch.dict("os.environ", _env(result_uri=uri), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: @@ -307,6 +342,7 @@ def test_upload_receives_correct_result_uri(self): assert kwargs["result_uri"] == uri def test_upload_content_is_bytes(self): + """Pass the upload content as a bytes object, not a str.""" with patch.dict("os.environ", _env(result_uri="s3://b/k"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() @@ -314,6 +350,7 @@ def test_upload_content_is_bytes(self): assert isinstance(kwargs["content"], bytes) def test_upload_content_is_utf8_encoded_json(self): + """Encode the uploaded content as UTF-8 JSON that decodes back to the expected result.""" with patch.dict("os.environ", _env(result_uri="s3://b/k"), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: main() @@ -334,6 +371,7 @@ def test_upload_content_differs_from_printed_log(self, capsys): assert "hello" not in printed # but never in the printed log def test_azure_uri_also_triggers_upload(self): + """Trigger the upload path for an azureblob:// RESULT_URI just as for s3://.""" uri = "azureblob://account/container/blob.json" with patch.dict("os.environ", _env(result_uri=uri), clear=True): with patch("tools.echo_test.run.upload_to_result_uri") as mock_upload: @@ -356,8 +394,10 @@ def test_upload_called_even_when_text_missing(self): # 6. __main__ block # =========================================================================== class TestMainBlock: + """The `if __name__ == "__main__"` execution path raises SystemExit with main()'s return code.""" def test_raises_system_exit(self): + """Executing the module's __main__ guard code raises SystemExit.""" with patch.dict("os.environ", _env(), clear=True): with pytest.raises(SystemExit): with patch.object(sys, "argv", ["tools/echo_test/run.py"]): @@ -371,12 +411,14 @@ def test_raises_system_exit(self): ) def test_raises_system_exit_with_code_0(self): + """SystemExit carries code 0 on a successful run.""" with patch.dict("os.environ", _env(), clear=True): with pytest.raises(SystemExit) as exc_info: raise SystemExit(main()) assert exc_info.value.code == 0 def test_raises_system_exit_with_code_2_on_bad_json(self): + """SystemExit carries code 2 when INPUTS_JSON is invalid.""" with patch.dict("os.environ", _env(inputs_json="bad"), clear=True): with pytest.raises(SystemExit) as exc_info: raise SystemExit(main()) diff --git a/tests/test_generic_sif_runner_run.py b/tests/test_generic_sif_runner_run.py index 29f8bd7..df1d6c4 100644 --- a/tests/test_generic_sif_runner_run.py +++ b/tests/test_generic_sif_runner_run.py @@ -12,6 +12,8 @@ - _collect_outputs (1 match / many matches / no match) - main() (every early-exit path + success + upload) - __main__ block + +Developer: Manish Kumar """ from __future__ import annotations @@ -63,6 +65,7 @@ def _base_env( work_dir: str = "", sif_cache_dir: str = "/tmp/test_sif_cache", ) -> dict[str, str]: + """Build a minimal TOOL_ID/RUN_ID/RESULT_URI/INPUTS_JSON/RESOURCES_JSON/SIF_CACHE_DIR environment for main().""" env: dict[str, str] = { "TOOL_ID": tool_id, "RUN_ID": run_id, @@ -79,6 +82,7 @@ def _base_env( def _env_with_tool_def(tool_def: dict = None, **kwargs) -> dict[str, str]: + """Build a main() environment carrying a given (or minimal) tool definition as TOOL_DEF_JSON.""" td = tool_def or MINIMAL_TOOL_DEF return _base_env(tool_def_json=json.dumps(td), **kwargs) @@ -87,12 +91,15 @@ def _env_with_tool_def(tool_def: dict = None, **kwargs) -> dict[str, str]: # 1. _env() # =========================================================================== class TestEnvHelper: + """_env() reads an environment variable, treating unset and empty-string values as falling back to the given default.""" def test_returns_env_value(self): + """Return the value of a set environment variable.""" with patch.dict("os.environ", {"MY_VAR": "hello"}, clear=False): assert _env("MY_VAR") == "hello" def test_returns_default_when_missing(self): + """Return the given default when the environment variable is unset.""" with patch.dict("os.environ", {}, clear=False): os.environ.pop("MISSING_VAR", None) assert _env("MISSING_VAR", "fallback") == "fallback" @@ -103,10 +110,12 @@ def test_empty_string_env_uses_default(self): assert _env("MY_VAR", "default") == "default" def test_default_is_empty_string_when_not_given(self): + """Return an empty string when the variable is unset and no default is given.""" os.environ.pop("TOTALLY_ABSENT", None) assert _env("TOTALLY_ABSENT") == "" def test_returns_string_type(self): + """Always return a str, even for a numeric-looking environment value.""" with patch.dict("os.environ", {"NUM_VAR": "42"}, clear=False): result = _env("NUM_VAR") assert isinstance(result, str) @@ -116,31 +125,39 @@ def test_returns_string_type(self): # 2. _resolve_env_refs() # =========================================================================== class TestResolveEnvRefs: + """_resolve_env_refs() expands ${VAR} and $VAR references from the process environment, leaving unresolvable references untouched.""" def test_dollar_brace_syntax(self): + """Expand a ${VAR}-style reference to the environment value.""" with patch.dict("os.environ", {"MY_VAR": "world"}, clear=False): assert _resolve_env_refs("hello ${MY_VAR}") == "hello world" def test_dollar_plain_syntax(self): + """Expand a bare $VAR-style reference to the environment value.""" with patch.dict("os.environ", {"MY_VAR": "world"}, clear=False): assert _resolve_env_refs("hello $MY_VAR") == "hello world" def test_missing_var_kept_as_is(self): + """Leave a ${VAR} reference unexpanded (verbatim) when the variable is not set.""" os.environ.pop("ABSENT_VAR", None) result = _resolve_env_refs("${ABSENT_VAR}") assert result == "${ABSENT_VAR}" def test_multiple_vars_expanded(self): + """Expand multiple distinct ${VAR} references within the same string.""" with patch.dict("os.environ", {"A": "foo", "B": "bar"}, clear=False): assert _resolve_env_refs("${A}-${B}") == "foo-bar" def test_no_vars_unchanged(self): + """Leave a string with no $-references unchanged.""" assert _resolve_env_refs("no vars here") == "no vars here" def test_empty_string(self): + """Return an empty string unchanged.""" assert _resolve_env_refs("") == "" def test_mixed_syntax(self): + """Expand ${VAR} and $VAR references together in the same string.""" with patch.dict("os.environ", {"X": "1", "Y": "2"}, clear=False): assert _resolve_env_refs("${X} $Y") == "1 2" @@ -151,30 +168,36 @@ def test_mixed_syntax(self): class TestFetchSif: # --- local path --- + """_fetch_sif() resolves a SIF image reference — local path, s3://, azureblob://, gs://, or docker:// — to a usable local path or pass-through URI, using a local cache for cloud downloads.""" def test_local_path_returns_path_object(self, tmp_path): + """Return the given local path unchanged (as a Path) when the SIF file exists on disk.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") result = _fetch_sif(str(sif), tmp_path) assert result == sif def test_local_path_not_found_raises(self, tmp_path): + """Raise FileNotFoundError when a local SIF path does not exist and no cloud fallback applies.""" with pytest.raises(FileNotFoundError, match="SIF not found"): _fetch_sif("/nonexistent/path/tool.sif", tmp_path) # --- cache hit --- def test_s3_cache_hit_returns_cached(self, tmp_path): + """Return the cached local file, skipping any S3 download, when an s3:// SIF is already present in the cache directory.""" cached = tmp_path / "tool.sif" cached.write_bytes(b"x" * 1024 * 1024 * 5) # 5 MB result = _fetch_sif("s3://bucket/tool.sif", tmp_path) assert result == cached def test_azure_cache_hit_returns_cached(self, tmp_path): + """Return the cached local file, skipping any Azure download, when an azureblob:// SIF is already present in the cache directory.""" cached = tmp_path / "tool.sif" cached.write_bytes(b"data") result = _fetch_sif("azureblob://account/container/tool.sif", tmp_path) assert result == cached def test_cache_hit_prints_message(self, tmp_path, capsys): + """Log a cache-hit message (not the raw SIF URI's sensitive detail) when serving a cached SIF file.""" cached = tmp_path / "tool.sif" cached.write_bytes(b"x" * 1024 * 1024) _fetch_sif("s3://bucket/tool.sif", tmp_path) @@ -182,6 +205,7 @@ def test_cache_hit_prints_message(self, tmp_path, capsys): # --- s3 download --- def test_s3_miss_calls_fetch_from_s3(self, tmp_path): + """Delegate to _fetch_from_s3 exactly once when an s3:// SIF is not already cached.""" cache_dir = tmp_path / "cache" with patch("tools.generic_sif_runner.run._fetch_from_s3") as mock_s3: mock_s3.side_effect = lambda uri, dest: dest.write_bytes(b"sif") @@ -189,6 +213,7 @@ def test_s3_miss_calls_fetch_from_s3(self, tmp_path): mock_s3.assert_called_once() def test_s3_miss_creates_cache_dir(self, tmp_path): + """Create the SIF cache directory before attempting an S3 download.""" cache_dir = tmp_path / "new_cache" with patch("tools.generic_sif_runner.run._fetch_from_s3") as mock_s3: mock_s3.side_effect = lambda uri, dest: dest.write_bytes(b"sif") @@ -197,6 +222,7 @@ def test_s3_miss_creates_cache_dir(self, tmp_path): # --- azure download --- def test_azure_miss_calls_fetch_from_azure(self, tmp_path): + """Delegate to _fetch_from_azure exactly once when an azureblob:// SIF is not already cached.""" cache_dir = tmp_path / "cache" with patch("tools.generic_sif_runner.run._fetch_from_azure") as mock_az: mock_az.side_effect = lambda uri, dest: dest.write_bytes(b"sif") @@ -205,6 +231,7 @@ def test_azure_miss_calls_fetch_from_azure(self, tmp_path): # --- env var expansion --- def test_env_var_in_uri_expanded(self, tmp_path): + """Expand an environment variable reference embedded in the SIF URI before resolving it.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") with patch.dict("os.environ", {"SIF_PATH": str(sif)}, clear=False): @@ -213,20 +240,24 @@ def test_env_var_in_uri_expanded(self, tmp_path): # --- docker:// passthrough --- def test_docker_uri_returned_as_is(self, tmp_path): + """Return a docker:// URI unchanged as a pass-through string for native singularity/Docker handling.""" uri = "docker://quay.io/biocontainers/bwa:0.7.17--h7132678_9" result = _fetch_sif(uri, tmp_path) assert result == uri def test_docker_uri_returns_str_not_path(self, tmp_path): + """Return a docker:// URI as a plain str, never wrapped in a Path.""" result = _fetch_sif("docker://quay.io/biocontainers/bwa:latest", tmp_path) assert isinstance(result, str) def test_docker_uri_not_collapsed(self, tmp_path): # Path() collapses "//" — guard against regressing to that + """Never collapse the "//" in a docker:// URI (Path() would corrupt the scheme) — regression guard.""" result = _fetch_sif("docker://quay.io/biocontainers/bwa:latest", tmp_path) assert result.startswith("docker://") def test_docker_uri_skips_cache_dir_creation(self, tmp_path): + """Never create the SIF cache directory for a docker:// URI, since no local caching applies.""" cache_dir = tmp_path / "unused_cache" _fetch_sif("docker://quay.io/biocontainers/bwa:latest", cache_dir) assert not cache_dir.exists() @@ -236,6 +267,7 @@ def test_docker_uri_skips_cache_dir_creation(self, tmp_path): # 4. _fetch_from_s3() # =========================================================================== class TestFetchFromS3: + """_fetch_from_s3() downloads a SIF image from S3 via boto3, raising RuntimeError naming the URI on failure.""" def _make_boto3_mock(self): """Return a boto3 mock whose download_file writes the dest file.""" @@ -246,12 +278,14 @@ def _make_boto3_mock(self): return mock_boto3 def test_boto3_download_success(self, tmp_path): + """Download an S3 SIF object to the destination path via boto3.""" dest = tmp_path / "tool.sif" with patch.dict("sys.modules", {"boto3": self._make_boto3_mock()}): _fetch_from_s3("s3://bucket/tool.sif", dest) assert dest.exists() def test_boto3_called_with_correct_bucket_and_key(self, tmp_path): + """Call boto3's download_file with the exact bucket and key parsed from the s3:// URI.""" dest = tmp_path / "tool.sif" mock_boto3 = self._make_boto3_mock() with patch.dict("sys.modules", {"boto3": mock_boto3}): @@ -261,6 +295,7 @@ def test_boto3_called_with_correct_bucket_and_key(self, tmp_path): assert call_args[1] == "my/key.sif" def test_boto3_called_once_per_download(self, tmp_path): + """Call boto3's download_file exactly once per SIF download.""" dest = tmp_path / "tool.sif" mock_boto3 = self._make_boto3_mock() with patch.dict("sys.modules", {"boto3": mock_boto3}): @@ -268,12 +303,14 @@ def test_boto3_called_once_per_download(self, tmp_path): mock_boto3.client.return_value.download_file.assert_called_once() def test_boto3_creates_parent_directory(self, tmp_path): + """Create the destination's parent directory before downloading.""" dest = tmp_path / "subdir" / "tool.sif" with patch.dict("sys.modules", {"boto3": self._make_boto3_mock()}): _fetch_from_s3("s3://bucket/tool.sif", dest) assert dest.parent.exists() def test_both_fail_raises_runtime_error(self, tmp_path): + """Raise RuntimeError naming the S3 download failure when boto3 raises.""" dest = tmp_path / "tool.sif" mock_boto3 = MagicMock() mock_boto3.client.return_value.download_file.side_effect = Exception("boom") @@ -284,6 +321,7 @@ def test_both_fail_raises_runtime_error(self, tmp_path): _fetch_from_s3("s3://bucket/tool.sif", dest) def test_runtime_error_message_contains_uri(self, tmp_path): + """Include the failing s3:// URI in the download-failure error message.""" dest = tmp_path / "tool.sif" mock_boto3 = MagicMock() mock_boto3.client.return_value.download_file.side_effect = Exception("x") @@ -299,8 +337,10 @@ def test_runtime_error_message_contains_uri(self, tmp_path): # 5. _fetch_from_azure() # =========================================================================== class TestFetchFromAzure: + """_fetch_from_azure() downloads a SIF image from Azure Blob Storage using either managed identity or a connection string, raising RuntimeError on failure.""" def _make_azure_mocks(self): + """Build a mocked BlobServiceClient constructor/instance chain returning fixed SIF bytes.""" mock_blob_data = MagicMock() mock_blob_data.readall.return_value = b"sif-bytes" mock_bc = MagicMock() @@ -312,6 +352,7 @@ def _make_azure_mocks(self): return mock_bsc_cls, mock_svc, mock_bc def test_managed_identity_path(self, tmp_path): + """Download an azureblob:// SIF using DefaultAzureCredential when AZURE_AUTH is managed_identity.""" dest = tmp_path / "tool.sif" mock_bsc_cls, mock_svc, _ = self._make_azure_mocks() mock_cred = MagicMock() @@ -325,6 +366,7 @@ def test_managed_identity_path(self, tmp_path): assert dest.read_bytes() == b"sif-bytes" def test_connection_string_path(self, tmp_path): + """Download an azureblob:// SIF via from_connection_string when AZURE_AUTH is connection_string.""" dest = tmp_path / "tool.sif" mock_bsc_cls, mock_svc, _ = self._make_azure_mocks() env = {"AZURE_AUTH": "connection_string", "AZURE_STORAGE_CONNECTION_STRING": "DefaultEndpointsProtocol=https"} @@ -337,6 +379,7 @@ def test_connection_string_path(self, tmp_path): mock_bsc_cls.from_connection_string.assert_called_once() def test_failure_raises_runtime_error(self, tmp_path): + """Raise RuntimeError naming the Azure Blob download failure when the SDK call raises.""" dest = tmp_path / "tool.sif" mock_bsc_cls = MagicMock(side_effect=Exception("azure boom")) with patch.dict("os.environ", {"AZURE_AUTH": "managed_identity"}, clear=False): @@ -348,6 +391,7 @@ def test_failure_raises_runtime_error(self, tmp_path): _fetch_from_azure("azureblob://account/container/tool.sif", dest) def test_container_and_blob_parsed_correctly(self, tmp_path): + """Parse the container and blob path correctly out of a deep azureblob:// URI.""" dest = tmp_path / "tool.sif" mock_bsc_cls, mock_svc, mock_bc = self._make_azure_mocks() env = {"AZURE_AUTH": "managed_identity", "AZURE_STORAGE_CONNECTION_STRING": ""} @@ -366,14 +410,17 @@ def test_container_and_blob_parsed_correctly(self, tmp_path): # 6. _load_tool_def() # =========================================================================== class TestLoadToolDef: + """_load_tool_def() resolves a tool definition from TOOL_DEF_JSON, TOOL_DEF_PATH, or a live TES API call, in that priority order.""" def test_loads_from_tool_def_json_env(self): + """Load the tool definition by parsing TOOL_DEF_JSON directly.""" td = {"slurm": {"image": "/sif/tool.sif"}} with patch.dict("os.environ", {"TOOL_DEF_JSON": json.dumps(td)}, clear=False): result = _load_tool_def() assert result == td def test_tool_def_json_takes_priority_over_path(self, tmp_path): + """Prefer TOOL_DEF_JSON over TOOL_DEF_PATH when both are set.""" td_json = {"source": "env"} td_file = {"source": "file"} p = tmp_path / "tool.json" @@ -384,6 +431,7 @@ def test_tool_def_json_takes_priority_over_path(self, tmp_path): assert result["source"] == "env" def test_loads_from_tool_def_path_env(self, tmp_path): + """Load the tool definition from the file named by TOOL_DEF_PATH when TOOL_DEF_JSON is unset.""" td = {"slurm": {"image": "/sif/tool.sif"}} p = tmp_path / "tool.json" p.write_text(json.dumps(td)) @@ -405,6 +453,7 @@ def test_tool_def_path_missing_file_skipped(self, tmp_path): _load_tool_def() def test_loads_from_tes_url(self): + """Load the tool definition by fetching it from the live TES API at TES_URL when no local source is set.""" tool_id = "my-tool" td = {"tool_id": tool_id, "slurm": {}} mock_response = MagicMock() @@ -418,12 +467,14 @@ def test_loads_from_tes_url(self): assert result["tool_id"] == tool_id def test_all_missing_raises_runtime_error(self): + """Raise RuntimeError when none of TOOL_DEF_JSON/TOOL_DEF_PATH/TES_URL yield a tool definition.""" env = {"TOOL_DEF_JSON": "", "TOOL_DEF_PATH": "", "TES_URL": "", "TOOL_ID": ""} with patch.dict("os.environ", env, clear=False): with pytest.raises(RuntimeError, match="Cannot load tool definition"): _load_tool_def() def test_error_message_mentions_env_vars(self): + """Name the relevant environment variables in the cannot-load-tool-definition error message.""" env = {"TOOL_DEF_JSON": "", "TOOL_DEF_PATH": "", "TES_URL": "", "TOOL_ID": ""} with patch.dict("os.environ", env, clear=False): with pytest.raises(RuntimeError) as exc_info: @@ -436,16 +487,20 @@ def test_error_message_mentions_env_vars(self): # 7. _resolve_command() # =========================================================================== class TestResolveCommand: + """_resolve_command() fills {placeholder} slots in the command template from inputs/resources/work_dir, and raises RuntimeError naming any placeholder it cannot fill.""" def test_simple_substitution(self): + """Substitute a single {placeholder} in a command template from the given inputs.""" result = _resolve_command(["echo", "{msg}"], {"msg": "hello"}, "/work") assert result == ["echo", "hello"] def test_work_dir_substituted(self): + """Substitute {work_dir} in a command template with the given working directory.""" result = _resolve_command(["{work_dir}/out.bam"], {}, "/work/dir") assert result == ["/work/dir/out.bam"] def test_multiple_inputs_substituted(self): + """Substitute multiple distinct {placeholder}s in a command template from the given inputs.""" result = _resolve_command( ["tool", "--in", "{infile}", "--out", "{outfile}"], {"infile": "/a.bam", "outfile": "/b.bam"}, @@ -454,19 +509,23 @@ def test_multiple_inputs_substituted(self): assert result == ["tool", "--in", "/a.bam", "--out", "/b.bam"] def test_missing_key_raises_runtime_error(self): + """Raise RuntimeError when a command template references a placeholder absent from inputs/resources/work_dir.""" with pytest.raises(RuntimeError, match="Missing input for command placeholder"): _resolve_command(["{missing_key}"], {}, "/work") def test_missing_key_error_mentions_key_name(self): + """Name the missing placeholder key in the resolve-command error message.""" with pytest.raises(RuntimeError) as exc_info: _resolve_command(["{my_missing_key}"], {}, "/work") assert "my_missing_key" in str(exc_info.value) def test_no_placeholders_returned_as_is(self): + """Return a command template with no {placeholders} unchanged.""" cmd = ["singularity", "exec", "tool.sif", "echo"] assert _resolve_command(cmd, {}, "/work") == cmd def test_returns_list_of_strings(self): + """Always return every resolved command argument as a str.""" result = _resolve_command(["echo", "{val}"], {"val": "x"}, "/w") assert all(isinstance(r, str) for r in result) @@ -475,13 +534,16 @@ def test_returns_list_of_strings(self): # 8. _collect_outputs() # =========================================================================== class TestCollectOutputs: + """_collect_outputs() globs each output spec's pattern in the work directory, storing a string for one match, a list for many, and None for zero.""" def test_single_match_stored_as_string(self, tmp_path): + """Store a single glob match as a plain string path, not a list.""" (tmp_path / "output.bam").write_bytes(b"") result = _collect_outputs(tmp_path, [{"name": "bam", "pattern": "*.bam"}]) assert result["bam"] == str(tmp_path / "output.bam") def test_multiple_matches_stored_as_list(self, tmp_path): + """Store multiple glob matches for one output name as a list of paths.""" (tmp_path / "a.bam").write_bytes(b"") (tmp_path / "b.bam").write_bytes(b"") result = _collect_outputs(tmp_path, [{"name": "bam", "pattern": "*.bam"}]) @@ -489,24 +551,29 @@ def test_multiple_matches_stored_as_list(self, tmp_path): assert len(result["bam"]) == 2 def test_no_match_stored_as_none(self, tmp_path): + """Store None for an output whose glob pattern matches nothing.""" result = _collect_outputs(tmp_path, [{"name": "vcf", "pattern": "*.vcf"}]) assert result["vcf"] is None def test_no_match_prints_warning(self, tmp_path, capsys): + """Log a WARNING when an output pattern matches no files.""" _collect_outputs(tmp_path, [{"name": "vcf", "pattern": "*.vcf"}]) assert "WARNING" in capsys.readouterr().out def test_default_name_is_output(self, tmp_path): + """Default an output spec's name to "output" when not given.""" (tmp_path / "file.txt").write_bytes(b"") result = _collect_outputs(tmp_path, [{"pattern": "*.txt"}]) assert "output" in result def test_default_pattern_matches_all(self, tmp_path): + """Default an output spec's glob pattern to "*" (match everything) when not given.""" (tmp_path / "anything.xyz").write_bytes(b"") result = _collect_outputs(tmp_path, [{"name": "out"}]) assert result["out"] is not None def test_multiple_specs_returned(self, tmp_path): + """Collect results for multiple independent output specs in one call.""" (tmp_path / "a.bam").write_bytes(b"") (tmp_path / "b.vcf").write_bytes(b"") result = _collect_outputs(tmp_path, [ @@ -516,9 +583,11 @@ def test_multiple_specs_returned(self, tmp_path): assert "bam" in result and "vcf" in result def test_empty_spec_list_returns_empty_dict(self, tmp_path): + """Return an empty dict when no output specs are given.""" assert _collect_outputs(tmp_path, []) == {} def test_matches_are_sorted(self, tmp_path): + """Return multiple glob matches for one output name in sorted order.""" (tmp_path / "z.bam").write_bytes(b"") (tmp_path / "a.bam").write_bytes(b"") result = _collect_outputs(tmp_path, [{"name": "bam", "pattern": "*.bam"}]) @@ -529,30 +598,36 @@ def test_matches_are_sorted(self, tmp_path): # 9. main() — early-exit paths # =========================================================================== class TestMainEarlyExits: + """main() exits with code 2, logging to stderr, at each validation/setup failure point before the tool is actually executed.""" def test_bad_inputs_json_returns_2(self): + """Return exit code 2 when INPUTS_JSON is not valid JSON.""" env = _base_env(inputs_json="not-json") with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_bad_resources_json_returns_2(self): + """Return exit code 2 when RESOURCES_JSON is not valid JSON.""" env = _base_env(resources_json="{bad}") with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_bad_json_prints_to_stderr(self, capsys): + """Print an ERROR diagnostic to stderr when INPUTS_JSON/RESOURCES_JSON fails to parse.""" env = _base_env(inputs_json="bad") with patch.dict("os.environ", env, clear=True): main() assert "ERROR" in capsys.readouterr().err def test_load_tool_def_failure_returns_2(self): + """Return exit code 2 when no tool definition source (JSON/path/TES) is available.""" env = _base_env() # no TOOL_DEF_JSON → will fail env.update({"TOOL_DEF_JSON": "", "TOOL_DEF_PATH": "", "TES_URL": "", "TOOL_ID": ""}) with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_load_tool_def_failure_prints_error(self, capsys): + """Print an ERROR diagnostic to stderr when the tool definition cannot be loaded.""" env = _base_env() env.update({"TOOL_DEF_JSON": "", "TOOL_DEF_PATH": "", "TES_URL": "", "TOOL_ID": ""}) with patch.dict("os.environ", env, clear=True): @@ -560,12 +635,14 @@ def test_load_tool_def_failure_prints_error(self, capsys): assert "ERROR" in capsys.readouterr().err def test_missing_slurm_image_returns_2(self): + """Return exit code 2 when the tool definition's slurm.image is empty.""" td = {"slurm": {"image": "", "command": [], "outputs": []}} env = _env_with_tool_def(td) with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_missing_slurm_image_prints_error(self, capsys): + """Name the missing slurm.image in the stderr diagnostic.""" td = {"slurm": {"image": "", "command": [], "outputs": []}} env = _env_with_tool_def(td) with patch.dict("os.environ", env, clear=True): @@ -573,18 +650,21 @@ def test_missing_slurm_image_prints_error(self, capsys): assert "no slurm.image" in capsys.readouterr().err def test_no_slurm_key_returns_2(self): + """Return exit code 2 when the tool definition has no slurm key at all.""" td = {} env = _env_with_tool_def(td) with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_sif_fetch_failure_returns_2(self, tmp_path): + """Return exit code 2 when the SIF image cannot be fetched and no Docker fallback is configured.""" td = {"slurm": {"image": "/nonexistent/tool.sif", "command": ["echo"], "outputs": []}} env = _env_with_tool_def(td, work_dir=str(tmp_path)) with patch.dict("os.environ", env, clear=True): assert main() == 2 def test_sif_fetch_failure_prints_error(self, tmp_path, capsys): + """Report the SIF fetch failure on stderr.""" td = {"slurm": {"image": "/nonexistent/tool.sif", "command": ["echo"], "outputs": []}} env = _env_with_tool_def(td, work_dir=str(tmp_path)) with patch.dict("os.environ", env, clear=True): @@ -592,6 +672,7 @@ def test_sif_fetch_failure_prints_error(self, tmp_path, capsys): assert "SIF fetch failed" in capsys.readouterr().err def test_resolve_command_failure_returns_2(self, tmp_path): + """Return exit code 2 when the command template references a missing input placeholder.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["{missing}"], "outputs": []}} @@ -600,6 +681,7 @@ def test_resolve_command_failure_returns_2(self, tmp_path): assert main() == 2 def test_resolve_command_failure_prints_error(self, tmp_path, capsys): + """Report the command-resolution failure on stderr.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["{missing}"], "outputs": []}} @@ -613,6 +695,7 @@ def test_resolve_command_failure_prints_error(self, tmp_path, capsys): # 10. main() — successful execution # =========================================================================== class TestMainSuccess: + """A full main() run through the singularity path — command construction, environment, output binding, and result reporting — for a mocked tool subprocess.""" def _run_with_mock_proc( self, @@ -624,6 +707,7 @@ def _run_with_mock_proc( inputs_json: str = "{}", resources_json: str = "{}", ): + """Build a full main() environment plus a mocked subprocess.run() result for a successful or failed tool execution.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -641,6 +725,7 @@ def _run_with_mock_proc( return env, mock_proc def test_returns_0_on_success(self, tmp_path): + """Return exit code 0 when the singularity/tool subprocess exits 0.""" env, mock_proc = self._run_with_mock_proc(tmp_path) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc): @@ -648,6 +733,7 @@ def test_returns_0_on_success(self, tmp_path): assert rc == 0 def test_returns_1_when_singularity_fails(self, tmp_path): + """Return exit code 1 when the singularity/tool subprocess exits nonzero.""" env, mock_proc = self._run_with_mock_proc(tmp_path, returncode=1) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc): @@ -655,6 +741,7 @@ def test_returns_1_when_singularity_fails(self, tmp_path): assert rc == 1 def test_exit_code_in_result(self, tmp_path, capsys): + """Report the tool's exact exit_code in the printed result JSON.""" env, mock_proc = self._run_with_mock_proc(tmp_path, returncode=0) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc): @@ -671,6 +758,7 @@ def test_exit_code_in_result(self, tmp_path, capsys): pass def test_tool_id_in_result(self, tmp_path, capsys): + """Include the configured tool_id in the printed result.""" env, mock_proc = self._run_with_mock_proc(tmp_path) env["TOOL_ID"] = "my-tool" with patch.dict("os.environ", env, clear=True): @@ -680,6 +768,7 @@ def test_tool_id_in_result(self, tmp_path, capsys): assert "my-tool" in out def test_singularity_called_with_exec(self, tmp_path): + """Invoke `singularity exec` as the first two argv elements for a normal (non-Docker, non-mismatched-arch) run.""" env, mock_proc = self._run_with_mock_proc(tmp_path) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc) as mock_run: @@ -689,6 +778,7 @@ def test_singularity_called_with_exec(self, tmp_path): assert args[1] == "exec" def test_singularity_cmd_includes_sif_path(self, tmp_path): + """Include the resolved local SIF file path in the singularity command line.""" env, mock_proc = self._run_with_mock_proc(tmp_path) sif_path = str(tmp_path / "tool.sif") with patch.dict("os.environ", env, clear=True): @@ -698,6 +788,7 @@ def test_singularity_cmd_includes_sif_path(self, tmp_path): assert sif_path in args def test_omp_num_threads_set_from_resources(self, tmp_path): + """Set OMP_NUM_THREADS in the subprocess environment from resources.cpu.""" env, mock_proc = self._run_with_mock_proc(tmp_path, resources_json='{"cpu": 4}') with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc) as mock_run: @@ -706,6 +797,7 @@ def test_omp_num_threads_set_from_resources(self, tmp_path): assert passed_env["OMP_NUM_THREADS"] == "4" def test_omp_num_threads_defaults_to_1(self, tmp_path): + """Default OMP_NUM_THREADS to "1" when resources.cpu is not given.""" env, mock_proc = self._run_with_mock_proc(tmp_path) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc) as mock_run: @@ -714,6 +806,7 @@ def test_omp_num_threads_defaults_to_1(self, tmp_path): assert passed_env["OMP_NUM_THREADS"] == "1" def test_stderr_printed_to_stderr_stream(self, tmp_path, capsys): + """Pass the tool's stderr output through to the runtime's own stderr stream.""" env, mock_proc = self._run_with_mock_proc(tmp_path, stderr="some error") with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc): @@ -721,6 +814,7 @@ def test_stderr_printed_to_stderr_stream(self, tmp_path, capsys): assert "some error" in capsys.readouterr().err def test_work_dir_bound_in_singularity_cmd(self, tmp_path): + """Bind the work directory into the container via --bind in the singularity command.""" env, mock_proc = self._run_with_mock_proc(tmp_path) with patch.dict("os.environ", env, clear=True): with patch("subprocess.run", return_value=mock_proc) as mock_run: @@ -729,6 +823,7 @@ def test_work_dir_bound_in_singularity_cmd(self, tmp_path): assert "--bind" in args def test_input_file_path_bound_if_exists(self, tmp_path): + """Bind the parent directory of an existing local input file path into the container.""" input_file = tmp_path / "input.bam" input_file.write_bytes(b"data") env, mock_proc = self._run_with_mock_proc( @@ -743,6 +838,7 @@ def test_input_file_path_bound_if_exists(self, tmp_path): assert str(tmp_path) in " ".join(args) def test_non_path_input_not_bound(self, tmp_path): + """Never add a --bind entry for a non-path (e.g. numeric-string) input value.""" env, mock_proc = self._run_with_mock_proc( tmp_path, inputs_json=json.dumps({"count": "42"}), @@ -760,8 +856,10 @@ def test_non_path_input_not_bound(self, tmp_path): # 11. main() — upload path # =========================================================================== class TestMainUpload: + """main() uploads the run result to RESULT_URI via upload_to_result_uri, with the exact kwargs (uri/content/content_type/aws_profile) the uploader contract requires.""" def _run_with_upload(self, tmp_path, result_uri: str, upload_mock): + """Run main() with a mocked successful subprocess and a mocked upload_to_result_uri, for a given RESULT_URI.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -773,28 +871,33 @@ def _run_with_upload(self, tmp_path, result_uri: str, upload_mock): return main() def test_upload_called_when_result_uri_set(self, tmp_path): + """Call upload_to_result_uri exactly once when RESULT_URI is set.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key", mock_upload) mock_upload.assert_called_once() def test_upload_not_called_without_result_uri(self, tmp_path): + """Skip the upload entirely when RESULT_URI is empty.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "", mock_upload) mock_upload.assert_not_called() def test_upload_receives_result_uri(self, tmp_path): + """Pass the exact configured RESULT_URI through to the uploader.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key.json", mock_upload) _, kwargs = mock_upload.call_args assert kwargs["result_uri"] == "s3://bucket/key.json" def test_upload_content_is_bytes(self, tmp_path): + """Pass the upload content as bytes, not a str.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key", mock_upload) _, kwargs = mock_upload.call_args assert isinstance(kwargs["content"], bytes) def test_upload_content_is_valid_json(self, tmp_path): + """Upload content that parses as valid JSON containing an 'ok' field.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key", mock_upload) _, kwargs = mock_upload.call_args @@ -802,12 +905,14 @@ def test_upload_content_is_valid_json(self, tmp_path): assert "ok" in obj def test_upload_content_type_is_json(self, tmp_path): + """Pass application/json as the upload's content_type.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key", mock_upload) _, kwargs = mock_upload.call_args assert kwargs["content_type"] == "application/json" def test_upload_uses_aws_profile_from_env(self, tmp_path): + """Forward the AWS_PROFILE environment variable to the uploader as aws_profile.""" mock_upload = MagicMock() sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") @@ -823,6 +928,7 @@ def test_upload_uses_aws_profile_from_env(self, tmp_path): assert kwargs["aws_profile"] == "my-profile" def test_upload_prints_confirmation(self, tmp_path, capsys): + """Print an "uploaded" confirmation message after a successful upload.""" mock_upload = MagicMock() self._run_with_upload(tmp_path, "s3://bucket/key", mock_upload) assert "uploaded" in capsys.readouterr().out @@ -832,8 +938,10 @@ def test_upload_prints_confirmation(self, tmp_path, capsys): # 12. __main__ block # =========================================================================== class TestMainBlock: + """The `if __name__ == "__main__"` execution path raises SystemExit carrying main()'s return code, across success and failure.""" def test_raises_system_exit_on_success(self, tmp_path): + """SystemExit carries code 0 for a successful run executed via __main__.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo"], "outputs": []}} @@ -846,6 +954,7 @@ def test_raises_system_exit_on_success(self, tmp_path): assert exc_info.value.code == 0 def test_raises_system_exit_with_code_2_on_bad_json(self): + """SystemExit carries code 2 when INPUTS_JSON is invalid.""" env = _base_env(inputs_json="bad") with patch.dict("os.environ", env, clear=True): with pytest.raises(SystemExit) as exc_info: @@ -853,6 +962,7 @@ def test_raises_system_exit_with_code_2_on_bad_json(self): assert exc_info.value.code == 2 def test_raises_system_exit_with_code_1_on_tool_failure(self, tmp_path): + """SystemExit carries code 1 when the underlying tool exits nonzero.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["false"], "outputs": []}} @@ -869,8 +979,10 @@ def test_raises_system_exit_with_code_1_on_tool_failure(self, tmp_path): # 13. _fetch_sif() — SIF_BASE rewrite (lines 43-47) # =========================================================================== class TestFetchSifBase: + """_fetch_sif() rewrites a missing local SIF path to a SIF_BASE-prefixed URI, but only for genuinely local (non-cloud-scheme) paths.""" def test_sif_base_rewrites_missing_local_to_cloud(self, tmp_path): + """Rewrite a missing local SIF path to SIF_BASE + filename when SIF_BASE is configured.""" cache_dir = tmp_path / "cache" cache_dir.mkdir() cached = cache_dir / "tool.sif" @@ -880,6 +992,7 @@ def test_sif_base_rewrites_missing_local_to_cloud(self, tmp_path): assert result == cached def test_sif_base_not_applied_to_cloud_uris(self, tmp_path): + """Never apply the SIF_BASE rewrite to a URI that is already a cloud scheme (s3/azureblob/gs).""" cache_dir = tmp_path / "cache" cache_dir.mkdir() cached = cache_dir / "tool.sif" @@ -889,6 +1002,7 @@ def test_sif_base_not_applied_to_cloud_uris(self, tmp_path): assert result == cached def test_sif_base_prints_rewrite_message(self, tmp_path, capsys): + """Log the SIF_BASE rewrite decision (local SIF not found) to stdout.""" cache_dir = tmp_path / "cache" cache_dir.mkdir() (cache_dir / "tool.sif").write_bytes(b"sif") @@ -901,8 +1015,10 @@ def test_sif_base_prints_rewrite_message(self, tmp_path, capsys): # 14. _fetch_sif() — GCS download (lines 73-74) # =========================================================================== class TestFetchSifGcs: + """_fetch_sif() dispatches gs:// SIF URIs to _fetch_from_gcs, honoring the local cache the same way as the s3/azureblob paths.""" def test_gcs_miss_calls_fetch_from_gcs(self, tmp_path): + """Delegate to _fetch_from_gcs exactly once when a gs:// SIF is not already cached.""" cache_dir = tmp_path / "cache" with patch("tools.generic_sif_runner.run._fetch_from_gcs") as mock_gcs: mock_gcs.side_effect = lambda uri, dest: dest.write_bytes(b"sif") @@ -910,6 +1026,7 @@ def test_gcs_miss_calls_fetch_from_gcs(self, tmp_path): mock_gcs.assert_called_once() def test_gcs_cache_hit_skips_download(self, tmp_path): + """Serve a gs:// SIF from the local cache without calling _fetch_from_gcs when already cached.""" cache_dir = tmp_path / "cache" cache_dir.mkdir() cached = cache_dir / "tool.sif" @@ -924,6 +1041,7 @@ def test_gcs_cache_hit_skips_download(self, tmp_path): # 15. _fetch_from_gcs() (lines 125-139) # =========================================================================== def _make_gcs_storage_mock(dest_path: Path): + """Build a mocked google.cloud.storage module whose Client().bucket().blob().download_to_filename writes the destination file.""" mock_blob = MagicMock() mock_blob.download_to_filename.side_effect = lambda p: Path(p).write_bytes(b"sif-data") mock_bucket = MagicMock() @@ -936,8 +1054,10 @@ def _make_gcs_storage_mock(dest_path: Path): class TestFetchFromGcs: + """_fetch_from_gcs() downloads a SIF image from Google Cloud Storage, raising RuntimeError naming the URI on failure.""" def test_gcs_download_success(self, tmp_path): + """Download a GCS SIF object to the destination path.""" dest = tmp_path / "tool.sif" mock_storage, _ = _make_gcs_storage_mock(dest) mock_gcloud = MagicMock(storage=mock_storage) @@ -949,6 +1069,7 @@ def test_gcs_download_success(self, tmp_path): assert dest.exists() def test_gcs_download_uses_correct_bucket(self, tmp_path): + """Look up the exact bucket name parsed from the gs:// SIF URI.""" dest = tmp_path / "tool.sif" mock_storage, _ = _make_gcs_storage_mock(dest) mock_gcloud = MagicMock(storage=mock_storage) @@ -960,6 +1081,7 @@ def test_gcs_download_uses_correct_bucket(self, tmp_path): mock_storage.Client.return_value.bucket.assert_called_with("my-bucket") def test_gcs_download_uses_correct_blob_path(self, tmp_path): + """Look up the exact blob path parsed from the gs:// SIF URI.""" dest = tmp_path / "tool.sif" mock_storage, _ = _make_gcs_storage_mock(dest) mock_gcloud = MagicMock(storage=mock_storage) @@ -971,6 +1093,7 @@ def test_gcs_download_uses_correct_blob_path(self, tmp_path): mock_storage.Client.return_value.bucket.return_value.blob.assert_called_with("path/to/tool.sif") def test_gcs_download_failure_raises_runtime_error(self, tmp_path): + """Raise RuntimeError naming the GCS download failure when the client raises.""" dest = tmp_path / "tool.sif" mock_storage = MagicMock() mock_storage.Client.side_effect = Exception("gcs boom") @@ -983,6 +1106,7 @@ def test_gcs_download_failure_raises_runtime_error(self, tmp_path): _fetch_from_gcs("gs://my-bucket/tool.sif", dest) def test_gcs_error_message_contains_uri(self, tmp_path): + """Include the failing gs:// URI in the download-failure error message.""" dest = tmp_path / "tool.sif" mock_storage = MagicMock() mock_storage.Client.side_effect = Exception("boom") @@ -1000,8 +1124,10 @@ def test_gcs_error_message_contains_uri(self, tmp_path): # 16. _load_tool_def() — TES URL properly mocked (lines 159-163) # =========================================================================== class TestLoadToolDefTesUrlFixed: + """_load_tool_def() matches TOOL_ID against the TES API's returned tool list, raising when no entry matches.""" def test_tes_url_finds_matching_tool(self): + """Select the tool definition whose tool_id matches TOOL_ID from the TES API's tool list.""" tool_id = "target-tool" tools_list = [ {"tool_id": "other-tool"}, @@ -1022,6 +1148,7 @@ def test_tes_url_finds_matching_tool(self): assert result["tool_id"] == tool_id def test_tes_url_not_found_falls_through_to_error(self): + """Raise RuntimeError when no tool in the TES API's response matches TOOL_ID.""" env = { "TOOL_DEF_JSON": "", "TOOL_DEF_PATH": "", @@ -1041,8 +1168,10 @@ def test_tes_url_not_found_falls_through_to_error(self): # 17. main() — Docker fallback when SIF fetch fails (lines 262-264, 381-383) # =========================================================================== class TestMainDockerFallback: + """main() falls back to direct (no-singularity) execution of the resolved command when the SIF image is unavailable but a docker_image is configured.""" def test_docker_used_when_sif_missing_and_docker_image_set(self, tmp_path): + """Fall back to direct (non-singularity) execution when the SIF is missing but a docker_image is configured.""" td = { "slurm": { "image": "/nonexistent/tool.sif", @@ -1061,6 +1190,7 @@ def test_docker_used_when_sif_missing_and_docker_image_set(self, tmp_path): assert args[0] != "singularity" def test_docker_fallback_prints_message(self, tmp_path, capsys): + """Log the Docker-fallback decision when the SIF is unavailable.""" td = { "slurm": { "image": "/nonexistent/tool.sif", @@ -1077,6 +1207,7 @@ def test_docker_fallback_prints_message(self, tmp_path, capsys): assert "Docker" in capsys.readouterr().out def test_direct_exec_uses_resolved_command(self, tmp_path): + """Execute the resolved command directly (no singularity wrapper) in the Docker-fallback path.""" td = { "slurm": { "image": "/nonexistent/tool.sif", @@ -1098,8 +1229,10 @@ def test_direct_exec_uses_resolved_command(self, tmp_path): # 18. main() — arch mismatch forces Docker (lines 375-376) # =========================================================================== class TestMainArchMismatch: + """main() forces the Docker fallback when the SIF filename's architecture suffix does not match the host's actual architecture.""" def test_arm64_sif_on_x86_uses_docker(self, tmp_path): + """Fall back to Docker when an arm64-named SIF is run on an x86_64 host.""" sif = tmp_path / "tool_arm64.sif" sif.write_bytes(b"fake") td = { @@ -1121,6 +1254,7 @@ def test_arm64_sif_on_x86_uses_docker(self, tmp_path): assert args[0] != "singularity" def test_amd64_sif_on_aarch64_uses_docker(self, tmp_path): + """Fall back to Docker when an amd64-named SIF is run on an aarch64 host.""" sif = tmp_path / "tool_amd64.sif" sif.write_bytes(b"fake") td = { @@ -1142,6 +1276,7 @@ def test_amd64_sif_on_aarch64_uses_docker(self, tmp_path): assert args[0] != "singularity" def test_arch_mismatch_prints_message(self, tmp_path, capsys): + """Log the arch-mismatch decision when falling back to Docker.""" sif = tmp_path / "tool_arm64.sif" sif.write_bytes(b"fake") td = { @@ -1165,6 +1300,7 @@ def test_arch_mismatch_prints_message(self, tmp_path, capsys): # 19. main() — S3 input download (lines 273-307) # =========================================================================== def _make_s3_mock(): + """Build a mocked boto3 module whose S3 client's paginator lists one object.""" mock_boto3 = MagicMock() mock_s3_client = MagicMock() mock_boto3.client.return_value = mock_s3_client @@ -1177,8 +1313,10 @@ def _make_s3_mock(): class TestMainS3Input: + """main() downloads s3:// input values (single file or directory) into the work directory before resolving the command, falling back to the original URI on download failure.""" def test_s3_single_file_input_downloaded(self, tmp_path): + """Download a single s3:// input file into the work directory before running the tool.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1197,6 +1335,7 @@ def test_s3_single_file_input_downloaded(self, tmp_path): ) def test_s3_directory_input_downloaded_trailing_slash(self, tmp_path): + """Treat an s3:// input ending in "/" as a directory and download it via list_objects_v2.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1212,6 +1351,7 @@ def test_s3_directory_input_downloaded_trailing_slash(self, tmp_path): mock_s3_client.get_paginator.assert_called_with("list_objects_v2") def test_s3_directory_input_no_suffix_treated_as_dir(self, tmp_path): + """Treat an s3:// input with no file extension as a directory and download it via list_objects_v2.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1227,6 +1367,7 @@ def test_s3_directory_input_no_suffix_treated_as_dir(self, tmp_path): mock_s3_client.get_paginator.assert_called_with("list_objects_v2") def test_s3_download_failure_falls_back_to_original_uri(self, tmp_path): + """Fall back to the original s3:// URI (and continue the run) when a single-file S3 download fails.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1242,6 +1383,7 @@ def test_s3_download_failure_falls_back_to_original_uri(self, tmp_path): assert rc == 0 def test_s3_dir_download_failure_falls_back(self, tmp_path): + """Fall back to the original s3:// URI (and continue the run) when an S3 directory listing fails.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1263,8 +1405,10 @@ def test_s3_dir_download_failure_falls_back(self, tmp_path): # 20. main() — Azure input download (lines 309-334) # =========================================================================== class TestMainAzureInput: + """main() downloads azureblob:// input values into the work directory before resolving the command, falling back to the original URI on download failure.""" def _make_azure_input_mocks(self, data=b"bam-data"): + """Build mocked BlobServiceClient constructor and instance wired to return the given blob data.""" mock_blob_data = MagicMock() mock_blob_data.readall.return_value = data mock_bc = MagicMock() @@ -1277,6 +1421,7 @@ def _make_azure_input_mocks(self, data=b"bam-data"): return mock_bsc_cls, mock_svc def test_azure_input_downloaded_with_connection_string(self, tmp_path): + """Download an azureblob:// input via from_connection_string when AZURE_STORAGE_CONNECTION_STRING is set.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1296,6 +1441,7 @@ def test_azure_input_downloaded_with_connection_string(self, tmp_path): mock_bsc_cls.from_connection_string.assert_called_once() def test_azure_input_downloaded_with_managed_identity(self, tmp_path): + """Download an azureblob:// input via DefaultAzureCredential when no connection string is set.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1313,6 +1459,7 @@ def test_azure_input_downloaded_with_managed_identity(self, tmp_path): assert rc == 0 def test_azure_input_download_failure_falls_back(self, tmp_path): + """Fall back to the original azureblob:// URI (and continue the run) when the Azure download fails.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1334,8 +1481,10 @@ def test_azure_input_download_failure_falls_back(self, tmp_path): # 21. main() — GCS input download (lines 336-352) # =========================================================================== class TestMainGCSInput: + """main() downloads gs:// input values into the work directory before resolving the command, falling back to the original URI on download failure, and never logs the raw URI.""" def _make_gcs_input_mock(): + """Build a mocked google.cloud.storage client/bucket/blob chain for a GCS input download.""" mock_blob = MagicMock() mock_bucket = MagicMock() mock_bucket.blob.return_value = mock_blob @@ -1346,6 +1495,7 @@ def _make_gcs_input_mock(): return mock_storage, mock_blob def test_gcs_input_downloaded(self, tmp_path): + """Download a gs:// input file into the work directory before running the tool.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1365,6 +1515,7 @@ def test_gcs_input_downloaded(self, tmp_path): mock_storage.Client.return_value.bucket.assert_called_with("bucket") def test_gcs_input_download_failure_falls_back(self, tmp_path): + """Fall back to the original gs:// URI (and continue the run) when the GCS download fails.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1409,8 +1560,10 @@ def test_gcs_input_download_prints_message(self, tmp_path, capsys): # 22. main() — GCS result upload (lines 445-463) # =========================================================================== class TestMainGCSResultUpload: + """main() uploads the run result to a gs:// RESULT_URI via google-cloud-storage's blob.upload_from_string, with the correct bucket/blob/content-type.""" def _run_with_gcs_upload(self, tmp_path, result_uri): + """Run main() with a mocked successful subprocess and a mocked google-cloud-storage module, for a given gs:// RESULT_URI.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} @@ -1434,25 +1587,30 @@ def _run_with_gcs_upload(self, tmp_path, result_uri): return rc, mock_blob def test_gcs_result_upload_called(self, tmp_path): + """Call blob.upload_from_string exactly once for a gs:// RESULT_URI.""" rc, mock_blob = self._run_with_gcs_upload(tmp_path, "gs://my-bucket/results.json") assert rc == 0 mock_blob.upload_from_string.assert_called_once() def test_gcs_result_upload_content_type_is_json(self, tmp_path): + """Upload the GCS result with content_type application/json.""" _, mock_blob = self._run_with_gcs_upload(tmp_path, "gs://my-bucket/results.json") _, kwargs = mock_blob.upload_from_string.call_args assert kwargs.get("content_type") == "application/json" def test_gcs_result_upload_content_is_bytes(self, tmp_path): + """Upload the GCS result body as bytes, not a str.""" _, mock_blob = self._run_with_gcs_upload(tmp_path, "gs://my-bucket/results.json") args, _ = mock_blob.upload_from_string.call_args assert isinstance(args[0], bytes) def test_gcs_result_upload_prints_confirmation(self, tmp_path, capsys): + """Print an "uploaded" confirmation message after a successful GCS result upload.""" self._run_with_gcs_upload(tmp_path, "gs://my-bucket/results.json") assert "uploaded" in capsys.readouterr().out def test_gcs_result_upload_uses_correct_bucket(self, tmp_path): + """Look up the exact bucket name parsed from the gs:// RESULT_URI.""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") td = {"slurm": {"image": str(sif), "command": ["echo", "hi"], "outputs": []}} diff --git a/tests/test_phi_safe_logging.py b/tests/test_phi_safe_logging.py index 8fe78e6..3a9525e 100644 --- a/tests/test_phi_safe_logging.py +++ b/tests/test_phi_safe_logging.py @@ -11,6 +11,8 @@ redacted — that channel is the tool's intended, access-controlled output and is expected to carry the real value. Only the log stream must be PHI-safe. + +Developer: Manish Kumar """ from __future__ import annotations @@ -25,6 +27,7 @@ def _sif_tool_def(tmp_path, command=None, outputs=None): + """Build a minimal tool-definition dict pointing at a fake SIF file for sif_main().""" sif = tmp_path / "tool.sif" sif.write_bytes(b"fake") return { @@ -37,6 +40,7 @@ def _sif_tool_def(tmp_path, command=None, outputs=None): def _sif_env(tmp_path, td, inputs, resources_json="{}", result_uri=""): + """Build the environment mapping sif_main() reads its TOOL_ID/INPUTS_JSON/etc. from.""" return { "TOOL_ID": "tool-1", "RUN_ID": "run-1", @@ -53,6 +57,7 @@ class TestGenericSifRunnerSentinelLeakage: """Sentinel values must never reach stdout/stderr across execution paths.""" def test_normal_execution_no_leak(self, tmp_path, capsys): + """A successful run must not print the sentinel sample id or secret token to stdout/stderr.""" td = _sif_tool_def(tmp_path, command=["echo", "{sample_id}", "{token}"]) inputs = {"sample_id": SENTINEL_PATIENT, "token": SENTINEL_SECRET} env = _sif_env(tmp_path, td, inputs) @@ -158,8 +163,10 @@ def test_useful_structural_logging_remains(self, tmp_path, capsys): class TestEchoTestSentinelLeakage: + """Sentinel values must never reach stdout/stderr across echo_test execution paths.""" def test_normal_execution_no_leak(self, capsys): + """A successful echo_test run must not print the sentinel input value to stdout.""" env = { "TOOL_ID": "echo-tool", "RUN_ID": "run-1", @@ -204,6 +211,7 @@ def test_missing_text_error_no_leak(self, capsys): assert SENTINEL_PATIENT not in captured.out def test_malformed_input_no_leak(self, capsys): + """Malformed INPUTS_JSON must not echo the offending raw text, which may embed the sentinel value.""" env = { "TOOL_ID": "echo-tool", "RUN_ID": "run-1", diff --git a/tests/test_result_uri_parse.py b/tests/test_result_uri_parse.py index bb2982b..d545fcc 100644 --- a/tests/test_result_uri_parse.py +++ b/tests/test_result_uri_parse.py @@ -1,3 +1,6 @@ +"""Parsing rules for RESULT_URI values across the s3://, azureblob://, and gs:// schemes. + +Developer: Manish Kumar """ # tests/test_result_uri_parse.py from __future__ import annotations @@ -7,6 +10,7 @@ def test_parse_s3_uri_ok(): + """Split an s3:// URI into bucket and key path components.""" p = parse_result_uri("s3://my-bucket/some/prefix/results.json") assert p.scheme == "s3" assert p.account_or_bucket == "my-bucket" @@ -15,6 +19,7 @@ def test_parse_s3_uri_ok(): def test_parse_s3_uri_requires_bucket_and_key(): + """Reject s3:// URIs missing a bucket, or missing a key after the bucket.""" with pytest.raises(ValueError): parse_result_uri("s3://") with pytest.raises(ValueError): @@ -24,6 +29,7 @@ def test_parse_s3_uri_requires_bucket_and_key(): def test_parse_azureblob_uri_ok(): + """Split an azureblob:// URI into account, container, and blob path components.""" p = parse_result_uri("azureblob://acct/container/path/to/results.json") assert p.scheme == "azureblob" assert p.account_or_bucket == "acct" @@ -32,6 +38,7 @@ def test_parse_azureblob_uri_ok(): def test_parse_azureblob_uri_requires_container_and_path(): + """Reject azureblob:// URIs missing a container, or missing a path after the container.""" with pytest.raises(ValueError): parse_result_uri("azureblob://acct/") with pytest.raises(ValueError): @@ -41,6 +48,7 @@ def test_parse_azureblob_uri_requires_container_and_path(): def test_parse_gs_uri_ok(): + """Split a gs:// URI into bucket and key path components.""" p = parse_result_uri("gs://my-bucket/some/prefix/results.json") assert p.scheme == "gs" assert p.account_or_bucket == "my-bucket" @@ -49,6 +57,7 @@ def test_parse_gs_uri_ok(): def test_parse_gs_uri_requires_bucket_and_key(): + """Reject gs:// URIs missing a bucket, or missing a key after the bucket.""" with pytest.raises(ValueError): parse_result_uri("gs://") with pytest.raises(ValueError): @@ -58,10 +67,12 @@ def test_parse_gs_uri_requires_bucket_and_key(): def test_parse_rejects_unknown_scheme(): + """Reject a RESULT_URI scheme outside the supported s3/azureblob/gs set.""" with pytest.raises(ValueError, match="Unsupported"): parse_result_uri("ftp://bucket/key") # ftp is not supported def test_parse_rejects_missing_scheme(): + """Reject a RESULT_URI with no scheme prefix at all.""" with pytest.raises(ValueError, match="missing scheme"): parse_result_uri("bucket/key") diff --git a/tests/test_run.py b/tests/test_run.py index 7e5e461..d57b860 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -4,6 +4,8 @@ Run: pytest tests/test_run.py -v pytest tests/test_run.py --cov=omni_tool_runtime/run --cov-report=term-missing -v + +Developer: Manish Kumar """ from __future__ import annotations @@ -54,24 +56,30 @@ def _make_mod(main_return=0, has_main=True) -> types.ModuleType: class TestToolIdMissing: + """The generic entrypoint must refuse to dispatch when TOOL_ID is unset, empty, or blank.""" def test_returns_2_when_tool_id_not_set(self, capsys): + """Return exit code 2 when TOOL_ID is entirely absent from the environment.""" with patch.dict("os.environ", {}, clear=True): assert main() == 2 def test_returns_2_when_tool_id_empty_string(self, capsys): + """Return exit code 2 when TOOL_ID is set to an empty string.""" with patch.dict("os.environ", {"TOOL_ID": ""}, clear=True): assert main() == 2 def test_returns_2_when_tool_id_whitespace_only(self, capsys): + """Return exit code 2 when TOOL_ID is set but contains only whitespace.""" with patch.dict("os.environ", {"TOOL_ID": " "}, clear=True): assert main() == 2 def test_stderr_message_when_tool_id_missing(self, capsys): + """Report the missing TOOL_ID by name on stderr for operator diagnosis.""" with patch.dict("os.environ", {}, clear=True): main() assert "TOOL_ID" in capsys.readouterr().err def test_import_not_called_when_tool_id_missing(self): + """Never attempt to import a tool module when TOOL_ID validation has already failed.""" with ( patch.dict("os.environ", {}, clear=True), patch("omni_tool_runtime.run.importlib.import_module") as mock_import, @@ -86,7 +94,9 @@ def test_import_not_called_when_tool_id_missing(self): class TestImportFailure: + """The entrypoint must convert any failure to import the resolved tool module into exit code 2 plus a diagnostic message.""" def _call(self, tool_id="bad_tool", exc=None): + """Call main() with a given TOOL_ID whose import raises the given (or a default ImportError) exception.""" exc = exc or ImportError("no module named tools.bad_tool.run") with ( patch.dict("os.environ", {"TOOL_ID": tool_id}, clear=True), @@ -95,15 +105,19 @@ def _call(self, tool_id="bad_tool", exc=None): return main() def test_returns_2_on_import_error(self): + """Return exit code 2 when the tool module raises ImportError.""" assert self._call() == 2 def test_returns_2_on_arbitrary_exception(self): + """Return exit code 2 even when the tool module import fails with a non-import exception.""" assert self._call(exc=RuntimeError("boom")) == 2 def test_returns_2_on_module_not_found_error(self): + """Return exit code 2 when the tool module raises ModuleNotFoundError.""" assert self._call(exc=ModuleNotFoundError("nope")) == 2 def test_stderr_contains_module_name(self, capsys): + """Include the resolved tools..run module path in the stderr diagnostic.""" with ( patch.dict("os.environ", {"TOOL_ID": "my_tool"}, clear=True), patch("omni_tool_runtime.run.importlib.import_module", side_effect=ImportError("x")), @@ -112,6 +126,7 @@ def test_stderr_contains_module_name(self, capsys): assert "tools.my_tool.run" in capsys.readouterr().err def test_stderr_contains_error_text(self, capsys): + """Include the underlying import exception's message text in the stderr diagnostic.""" with ( patch.dict("os.environ", {"TOOL_ID": "t"}, clear=True), patch( @@ -123,6 +138,7 @@ def test_stderr_contains_error_text(self, capsys): assert "specific error message" in capsys.readouterr().err def test_import_called_with_correct_module_path(self): + """Build the import path as tools..run from the given TOOL_ID.""" with ( patch.dict("os.environ", {"TOOL_ID": "my_tool"}, clear=True), patch( @@ -133,6 +149,7 @@ def test_import_called_with_correct_module_path(self): mock_import.assert_called_once_with("tools.my_tool.run") def test_tool_id_stripped_before_module_path_built(self): + """Strip surrounding whitespace from TOOL_ID before building the tools..run import path.""" with ( patch.dict("os.environ", {"TOOL_ID": " spaced_tool "}, clear=True), patch( @@ -149,7 +166,9 @@ def test_tool_id_stripped_before_module_path_built(self): class TestModuleMissingMain: + """The entrypoint must fail cleanly when the imported tool module has no callable main().""" def _call(self, tool_id="t"): + """Call main() with a given TOOL_ID against a fake module that has no main attribute.""" mod = _make_mod(has_main=False) with ( patch.dict("os.environ", {"TOOL_ID": tool_id}, clear=True), @@ -158,17 +177,21 @@ def _call(self, tool_id="t"): return main() def test_returns_2_when_no_main(self): + """Return exit code 2 when the imported tool module has no main attribute.""" assert self._call() == 2 def test_stderr_mentions_missing_main(self, capsys): + """Report the missing main() callable by name on stderr.""" self._call(tool_id="no_main_tool") assert "main()" in capsys.readouterr().err def test_stderr_mentions_module_name(self, capsys): + """Include the resolved module path in the missing-main() stderr diagnostic.""" self._call(tool_id="no_main_tool") assert "tools.no_main_tool.run" in capsys.readouterr().err def test_main_not_invoked_when_absent(self): + """Complete without raising AttributeError when the tool module has no main attribute to call.""" mod = _make_mod(has_main=False) with ( patch.dict("os.environ", {"TOOL_ID": "t"}, clear=True), @@ -185,7 +208,9 @@ def test_main_not_invoked_when_absent(self): class TestSuccessfulRun: + """A successfully imported tool module's main() return value must be propagated as the process exit code.""" def _call(self, tool_id="good_tool", main_return=0): + """Call main() with a given TOOL_ID against a fake module whose main() returns main_return.""" mod = _make_mod(main_return=main_return) with ( patch.dict("os.environ", {"TOOL_ID": tool_id}, clear=True), @@ -194,23 +219,28 @@ def _call(self, tool_id="good_tool", main_return=0): return main(), mod def test_returns_0_on_success(self): + """Propagate a 0 return value from the tool's main() as the overall exit code.""" result, _ = self._call(main_return=0) assert result == 0 def test_returns_1_when_tool_main_returns_1(self): + """Propagate a 1 return value from the tool's main() as the overall exit code.""" result, _ = self._call(main_return=1) assert result == 1 def test_returns_int(self): + """Always return an int exit code, never the tool's raw return type.""" result, _ = self._call(main_return=0) assert isinstance(result, int) def test_tool_main_called_once(self): + """Call the tool module's main() exactly once per invocation.""" _, mod = self._call() mod.main.assert_called_once() def test_result_cast_to_int(self): # mod.main() returns a string "0"; run.main() should cast via int() + """Cast a non-int return value (e.g. the string "0") from the tool's main() to int.""" mod = _make_mod() mod.main.return_value = "0" with ( @@ -222,10 +252,12 @@ def test_result_cast_to_int(self): assert isinstance(result, int) def test_no_stderr_on_success(self, capsys): + """Write nothing to stderr on a successful run.""" self._call() assert capsys.readouterr().err == "" def test_import_called_with_correct_path(self): + """Build the import path as tools..run for a successful run.""" mod = _make_mod() with ( patch.dict("os.environ", {"TOOL_ID": "specific_tool"}, clear=True), @@ -235,6 +267,7 @@ def test_import_called_with_correct_path(self): mock_import.assert_called_once_with("tools.specific_tool.run") def test_nonzero_exit_code_propagated(self): + """Propagate an arbitrary nonzero return value from the tool's main() unchanged.""" result, _ = self._call(main_return=3) assert result == 3 @@ -245,7 +278,9 @@ def test_nonzero_exit_code_propagated(self): class TestMainBlock: + """The `if __name__ == "__main__"` block must call SystemExit with main()'s return code.""" def test_raises_system_exit(self): + """Running the module as __main__ raises SystemExit carrying main()'s (patched) return code of 0.""" mod = _make_mod(main_return=0) with ( patch.dict("os.environ", {"TOOL_ID": "t"}, clear=True), @@ -259,6 +294,7 @@ def test_raises_system_exit(self): assert exc_info.value.code == 0 def test_raises_system_exit_with_error_code(self): + """Running the module as __main__ raises SystemExit carrying main()'s (patched) nonzero return code.""" with ( patch("omni_tool_runtime.run.main", return_value=2), pytest.raises(SystemExit) as exc_info, @@ -278,24 +314,29 @@ class TestRunHelper: """Exercises the _run() helper directly to cover lines 31-40.""" def test_run_with_module_returns_zero(self): + """The _run() test helper returns the tool module's main() result directly when a module is injected.""" mod = _make_mod(main_return=0) result = _run({"TOOL_ID": "tool-1"}, module=mod) assert result == 0 def test_run_with_module_calls_tool_main(self): + """The _run() test helper's injected module has its main() invoked.""" mod = _make_mod(main_return=0) _run({"TOOL_ID": "tool-1"}, module=mod) mod.main.assert_called_once() def test_run_with_module_propagates_nonzero_return(self): + """The _run() test helper propagates a nonzero main() return value unchanged.""" mod = _make_mod(main_return=3) result = _run({"TOOL_ID": "tool-1"}, module=mod) assert result == 3 def test_run_without_module_returns_tuple(self): + """The _run() test helper returns the (result, mock_import) tuple when no module is injected, driving the ImportError branch.""" rc, mock_import = _run({"TOOL_ID": "tool-1"}) assert rc == 2 def test_run_without_module_mock_import_was_called(self): + """The _run() test helper's mocked import_module is called with the tools..run path.""" _, mock_import = _run({"TOOL_ID": "tool-1"}) mock_import.assert_called_once_with("tools.tool-1.run") diff --git a/tests/test_s3_uploader.py b/tests/test_s3_uploader.py index 061782e..73211f9 100644 --- a/tests/test_s3_uploader.py +++ b/tests/test_s3_uploader.py @@ -6,6 +6,8 @@ pytest tests/test_s3_uploader.py -v pytest tests/test_s3_uploader.py \ --cov=omni_tool_runtime/uploaders/s3_uploader --cov-report=term-missing -v + +Developer: Manish Kumar """ from __future__ import annotations @@ -55,14 +57,18 @@ def _call(uploader: S3Uploader, **kw): class TestS3UploaderConstruction: + """The S3Uploader dataclass stores an optional aws_profile without side effects.""" def test_default_profile_is_none(self): + """Default aws_profile to None when not given.""" assert S3Uploader().aws_profile is None def test_explicit_profile_stored(self): + """Store an explicitly given aws_profile.""" assert S3Uploader(aws_profile="my-profile").aws_profile == "my-profile" def test_empty_string_profile_stored(self): # Empty string is falsy — treated as no profile in upload_bytes + """Store an empty-string aws_profile as-is (its falsy handling happens later, in upload_bytes).""" assert S3Uploader(aws_profile="").aws_profile == "" @@ -72,7 +78,9 @@ def test_empty_string_profile_stored(self): class TestMissingBoto3: + """upload_bytes must fail with a clear, actionable error when boto3 is not installed.""" def test_raises_runtime_error(self): + """Raise RuntimeError when boto3 cannot be imported.""" uploader = S3Uploader() with patch.dict(sys.modules, {"boto3": None}), pytest.raises(RuntimeError): uploader.upload_bytes( @@ -80,6 +88,7 @@ def test_raises_runtime_error(self): ) def test_error_message_mentions_boto3(self): + """Name boto3 in the missing-dependency error message.""" uploader = S3Uploader() with ( patch.dict(sys.modules, {"boto3": None}), @@ -90,6 +99,7 @@ def test_error_message_mentions_boto3(self): ) def test_error_message_mentions_install_extra(self): + """Point to the omnibioai-tool-runtime install extra in the missing-dependency error message.""" uploader = S3Uploader() with ( patch.dict(sys.modules, {"boto3": None}), @@ -106,24 +116,30 @@ def test_error_message_mentions_install_extra(self): class TestSessionConstruction: + """upload_bytes must build a boto3 Session honoring the configured aws_profile.""" def test_session_created(self): + """Create exactly one boto3 Session per upload.""" mock_boto3, mock_session, _ = _call(S3Uploader()) mock_boto3.Session.assert_called_once() def test_no_profile_session_has_no_profile_kwarg(self): + """Omit the profile_name kwarg from Session() when aws_profile is None.""" mock_boto3, _, _ = _call(S3Uploader(aws_profile=None)) assert "profile_name" not in mock_boto3.Session.call_args.kwargs def test_profile_passed_to_session(self): + """Pass aws_profile through to Session() as profile_name.""" mock_boto3, _, _ = _call(S3Uploader(aws_profile="staging")) assert mock_boto3.Session.call_args.kwargs["profile_name"] == "staging" def test_empty_string_profile_not_passed(self): # Empty string is falsy so should not be forwarded + """Omit the profile_name kwarg from Session() when aws_profile is an empty (falsy) string.""" mock_boto3, _, _ = _call(S3Uploader(aws_profile="")) assert "profile_name" not in mock_boto3.Session.call_args.kwargs def test_session_client_called_with_s3(self): + """Request the 's3' client from the constructed Session.""" _, mock_session, _ = _call(S3Uploader()) mock_session.client.assert_called_once_with("s3") @@ -134,37 +150,46 @@ def test_session_client_called_with_s3(self): class TestPutObject: + """upload_bytes must forward bucket/key/data/content_type to boto3's put_object unchanged.""" def test_put_object_called(self): + """Call put_object exactly once per upload.""" _, _, mock_s3 = _call(S3Uploader()) mock_s3.put_object.assert_called_once() def test_bucket_passed(self): + """Forward the given bucket as put_object's Bucket kwarg.""" _, _, mock_s3 = _call(S3Uploader(), bucket="target-bucket") assert mock_s3.put_object.call_args.kwargs["Bucket"] == "target-bucket" def test_key_passed(self): + """Forward the given key as put_object's Key kwarg.""" _, _, mock_s3 = _call(S3Uploader(), key="runs/run-1/results.json") assert mock_s3.put_object.call_args.kwargs["Key"] == "runs/run-1/results.json" def test_data_passed_as_body(self): + """Forward the given data bytes as put_object's Body kwarg unmodified.""" payload = b'{"result": 99}' _, _, mock_s3 = _call(S3Uploader(), data=payload) assert mock_s3.put_object.call_args.kwargs["Body"] == payload def test_content_type_passed(self): + """Forward the given content_type as put_object's ContentType kwarg.""" _, _, mock_s3 = _call(S3Uploader(), content_type="text/plain") assert mock_s3.put_object.call_args.kwargs["ContentType"] == "text/plain" def test_default_content_type_json(self): + """Forward application/json as ContentType when that is the given content_type.""" _, _, mock_s3 = _call(S3Uploader(), content_type="application/json") assert mock_s3.put_object.call_args.kwargs["ContentType"] == "application/json" def test_body_is_bytes(self): + """Pass the Body kwarg through to put_object as bytes, not a str.""" payload = b"binary-payload" _, _, mock_s3 = _call(S3Uploader(), data=payload) assert isinstance(mock_s3.put_object.call_args.kwargs["Body"], bytes) def test_returns_none(self): + """upload_bytes returns None (the S3 call result is not surfaced to the caller).""" mock_boto3, mock_session, mock_s3 = _mock_boto3() uploader = S3Uploader() with patch.dict(sys.modules, {"boto3": mock_boto3}): @@ -174,6 +199,7 @@ def test_returns_none(self): assert result is None def test_all_four_kwargs_present(self): + """Always supply Bucket, Key, Body, and ContentType together to put_object.""" _, _, mock_s3 = _call(S3Uploader()) kw = mock_s3.put_object.call_args.kwargs for key in ("Bucket", "Key", "Body", "ContentType"): @@ -186,7 +212,9 @@ def test_all_four_kwargs_present(self): class TestEndToEnd: + """Full upload_bytes flow, from constructor through the mocked boto3 call chain, without a profile and with one.""" def test_full_flow_no_profile(self): + """A profile-less uploader creates a bare Session() and uploads the exact bucket/key/body/content-type given.""" mock_boto3, mock_session, mock_s3 = _mock_boto3() uploader = S3Uploader(aws_profile=None) with patch.dict(sys.modules, {"boto3": mock_boto3}): @@ -206,6 +234,7 @@ def test_full_flow_no_profile(self): ) def test_full_flow_with_profile(self): + """A profiled uploader creates Session(profile_name=...) and uploads the exact bucket/key/body/content-type given.""" mock_boto3, mock_session, mock_s3 = _mock_boto3() uploader = S3Uploader(aws_profile="dev") with patch.dict(sys.modules, {"boto3": mock_boto3}): diff --git a/tests/test_tools_echo_test.py b/tests/test_tools_echo_test.py index 8f366fc..7381dd4 100644 --- a/tests/test_tools_echo_test.py +++ b/tests/test_tools_echo_test.py @@ -1,3 +1,6 @@ +"""End-to-end subprocess test of the echo_test tool's local-mode (no RESULT_URI) execution. + +Developer: Manish Kumar """ import json import os import subprocess @@ -5,6 +8,7 @@ def test_echo_test_local_mode_no_result_uri(): + """Reject leaking the raw echoed input value into process stdout even in local mode with no RESULT_URI configured.""" env = dict(os.environ) env["TOOL_ID"] = "echo_test" env["RUN_ID"] = "local123" diff --git a/tests/test_upload_dispatch.py b/tests/test_upload_dispatch.py index 3a16b5d..b118ca9 100644 --- a/tests/test_upload_dispatch.py +++ b/tests/test_upload_dispatch.py @@ -1,3 +1,6 @@ +"""Dispatch of upload_to_result_uri() to the correct cloud uploader implementation based on RESULT_URI scheme. + +Developer: Manish Kumar """ # tests/test_upload_dispatch.py from __future__ import annotations @@ -9,6 +12,7 @@ class _S3Spy: + """Fake S3Uploader that records upload_bytes() calls instead of touching AWS.""" def __init__(self, aws_profile=None): self.aws_profile = aws_profile self.calls = [] @@ -26,6 +30,7 @@ def upload_bytes(self, *, bucket: str, key: str, data: bytes, content_type: str) class _AzureSpy: + """Fake AzureBlobUploader that records upload_bytes() calls instead of touching Azure.""" def __init__(self, account_name: str, auth: str = "managed_identity", connection_string=None): self.account_name = account_name self.auth = auth @@ -49,6 +54,7 @@ def upload_bytes( def test_upload_dispatch_to_s3(monkeypatch: pytest.MonkeyPatch): + """Route an s3:// RESULT_URI to S3Uploader with the parsed bucket/key and given aws_profile.""" _s3_spy = _S3Spy(aws_profile="prof1") # Patch constructor used inside upload_to_result_uri @@ -79,6 +85,7 @@ def _ctor(aws_profile=None): def test_upload_dispatch_to_azureblob(monkeypatch: pytest.MonkeyPatch): + """Route an azureblob:// RESULT_URI to AzureBlobUploader with the parsed account/container/blob_path and given auth settings.""" created = {} def _ctor(account_name: str, auth: str = "managed_identity", connection_string=None): @@ -113,6 +120,7 @@ def _ctor(account_name: str, auth: str = "managed_identity", connection_string=N def test_upload_unsupported_scheme_raises(): + """Reject an unsupported RESULT_URI scheme before any uploader is constructed.""" with pytest.raises(ValueError): mod.upload_to_result_uri(result_uri="ftp://bucket/key", data=b"x") diff --git a/tests/test_upload_result.py b/tests/test_upload_result.py index 15029e1..056a836 100644 --- a/tests/test_upload_result.py +++ b/tests/test_upload_result.py @@ -5,6 +5,8 @@ pytest tests/test_upload_result.py -v pytest tests/test_upload_result.py \ --cov=omni_tool_runtime/upload_result --cov-report=term-missing -v + +Developer: Manish Kumar """ from __future__ import annotations @@ -29,16 +31,21 @@ class TestNormalizeResultUri: + """_normalize_result_uri() fills in a results.json filename on prefix-style URIs and rejects blank RESULT_URI values.""" def test_already_ends_in_json_unchanged(self): + """Leave a RESULT_URI already ending in .json unchanged.""" assert _normalize_result_uri("s3://bucket/results.json") == "s3://bucket/results.json" def test_trailing_slash_replaced_with_results_json(self): + """Append results.json when the RESULT_URI ends with a trailing slash.""" assert _normalize_result_uri("s3://bucket/prefix/") == "s3://bucket/prefix/results.json" def test_prefix_without_trailing_slash_appended(self): + """Append /results.json when the RESULT_URI is a bare prefix with no filename or trailing slash.""" assert _normalize_result_uri("s3://bucket/prefix") == "s3://bucket/prefix/results.json" def test_multiple_trailing_slashes_normalized(self): + """Collapse multiple trailing slashes before appending results.json, without corrupting the scheme's own "//".""" result = _normalize_result_uri("s3://bucket/prefix///") assert result.endswith("/results.json") # Only check the path portion — the scheme "s3://" legitimately contains // @@ -47,27 +54,33 @@ def test_multiple_trailing_slashes_normalized(self): def test_uppercase_json_extension_unchanged(self): # .JSON suffix counts as ending with .json (case-insensitive check) + """Treat a .JSON suffix as already having a JSON extension (case-insensitive) and leave it unchanged.""" assert _normalize_result_uri("s3://bucket/out.JSON") == "s3://bucket/out.JSON" def test_other_json_extension_kept(self): + """Leave a URI already ending in .json (non-s3 scheme) unchanged.""" assert ( _normalize_result_uri("azureblob://acct/cont/blob.json") == "azureblob://acct/cont/blob.json" ) def test_empty_string_raises_runtime_error(self): + """Reject an empty RESULT_URI with a clear "RESULT_URI not set" error.""" with pytest.raises(RuntimeError, match="RESULT_URI not set"): _normalize_result_uri("") def test_whitespace_only_raises_runtime_error(self): + """Reject a whitespace-only RESULT_URI with a clear "RESULT_URI not set" error.""" with pytest.raises(RuntimeError, match="RESULT_URI not set"): _normalize_result_uri(" ") def test_none_raises_runtime_error(self): + """Reject a None RESULT_URI with a clear "RESULT_URI not set" error.""" with pytest.raises(RuntimeError, match="RESULT_URI not set"): _normalize_result_uri(None) def test_strips_leading_whitespace(self): + """Strip leading whitespace from RESULT_URI before normalizing.""" result = _normalize_result_uri(" s3://b/p") assert result == "s3://b/p/results.json" @@ -78,36 +91,45 @@ def test_strips_leading_whitespace(self): class TestParseS3: + """_parse_s3() splits an s3:// URI into (bucket, key) and rejects malformed or wrong-scheme URIs.""" def test_returns_bucket_and_key(self): + """Split a well-formed s3:// URI into its bucket and key.""" bucket, key = _parse_s3("s3://my-bucket/path/to/results.json") assert bucket == "my-bucket" assert key == "path/to/results.json" def test_simple_key(self): + """Split an s3:// URI with a single-segment key into bucket and key.""" bucket, key = _parse_s3("s3://bucket/results.json") assert bucket == "bucket" and key == "results.json" def test_deep_key_path(self): + """Preserve a multi-segment key path when splitting an s3:// URI.""" _, key = _parse_s3("s3://bucket/a/b/c/results.json") assert key == "a/b/c/results.json" def test_wrong_scheme_raises(self): + """Reject a non-s3:// URI passed to _parse_s3.""" with pytest.raises(ValueError, match="Not an s3 URI"): _parse_s3("azureblob://acct/cont/blob.json") def test_missing_bucket_raises(self): + """Reject an s3:// URI with an empty bucket component.""" with pytest.raises(ValueError, match="Invalid s3 URI"): _parse_s3("s3:///key/results.json") def test_missing_key_raises(self): + """Reject an s3:// URI with no key after the bucket.""" with pytest.raises(ValueError, match="Invalid s3 URI"): _parse_s3("s3://bucket/") def test_http_scheme_raises(self): + """Reject an https:// URI even if it looks like an S3 virtual-hosted URL.""" with pytest.raises(ValueError, match="Not an s3 URI"): _parse_s3("https://bucket.s3.amazonaws.com/key") def test_leading_slash_stripped_from_key(self): + """Never leave a leading slash on the extracted S3 key.""" _, key = _parse_s3("s3://bucket/results.json") assert not key.startswith("/") @@ -118,7 +140,9 @@ def test_leading_slash_stripped_from_key(self): class TestParseAzureblob: + """_parse_azureblob() splits an azureblob:// URI into (account, container, blob_path) and rejects malformed or wrong-scheme URIs.""" def test_returns_account_container_blob(self): + """Split a well-formed azureblob:// URI into account, container, and blob path.""" account, container, blob_path = _parse_azureblob( "azureblob://myaccount/mycontainer/path/to/results.json" ) @@ -127,30 +151,37 @@ def test_returns_account_container_blob(self): assert blob_path == "path/to/results.json" def test_simple_blob_path(self): + """Split an azureblob:// URI with a single-segment blob path.""" _, _, blob_path = _parse_azureblob("azureblob://acct/cont/results.json") assert blob_path == "results.json" def test_deep_blob_path(self): + """Preserve a multi-segment blob path when splitting an azureblob:// URI.""" _, _, blob_path = _parse_azureblob("azureblob://acct/cont/a/b/c/results.json") assert blob_path == "a/b/c/results.json" def test_wrong_scheme_raises(self): + """Reject a non-azureblob:// URI passed to _parse_azureblob.""" with pytest.raises(ValueError, match="Not an azureblob URI"): _parse_azureblob("s3://bucket/key") def test_missing_container_raises(self): + """Reject an azureblob:// URI with no container/blob segment after the account.""" with pytest.raises(ValueError, match="Invalid azureblob URI"): _parse_azureblob("azureblob://acct/results.json") def test_empty_account_raises(self): + """Reject an azureblob:// URI with an empty account component.""" with pytest.raises(ValueError): _parse_azureblob("azureblob:///container/blob.json") def test_leading_slash_stripped_from_path(self): + """Never leave a leading slash on the extracted blob path.""" _, _, blob_path = _parse_azureblob("azureblob://acct/cont/results.json") assert not blob_path.startswith("/") def test_container_extracted_correctly(self): + """Extract a hyphenated container name exactly as given.""" _, container, _ = _parse_azureblob("azureblob://acct/my-container/blob.json") assert container == "my-container" @@ -161,25 +192,31 @@ def test_container_extracted_correctly(self): class TestUploadArgValidation: + """upload_to_result_uri() enforces that exactly one of content/data is usable and that RESULT_URI resolves to a supported scheme.""" def test_no_content_or_data_raises_type_error(self): + """Reject a call that supplies neither content nor data.""" with pytest.raises(TypeError, match="content="): upload_to_result_uri(result_uri="s3://b/k.json") def test_content_none_and_data_none_raises(self): + """Reject a call where both content and data are explicitly None.""" with pytest.raises(TypeError): upload_to_result_uri(result_uri="s3://b/k.json", content=None, data=None) def test_empty_result_uri_raises_runtime_error(self): + """Reject an empty RESULT_URI before attempting any upload.""" with pytest.raises(RuntimeError, match="RESULT_URI not set"): upload_to_result_uri(result_uri="", content=b"x") def test_unsupported_scheme_raises_value_error(self): + """Reject a parsed scheme outside the supported s3/azureblob/gs set.""" with patch(f"{MOD}.parse_result_uri") as mock_parse: mock_parse.return_value = MagicMock(scheme="gcs") with pytest.raises(ValueError, match="Unsupported RESULT_URI scheme"): upload_to_result_uri(result_uri="gcs://bucket/key.json", content=b"x") def test_content_preferred_over_data(self): + """Prefer the content kwarg over the legacy data kwarg when both are given.""" mock_uploader = MagicMock() with ( patch(f"{MOD}.parse_result_uri") as mock_parse, @@ -195,6 +232,7 @@ def test_content_preferred_over_data(self): assert mock_uploader.upload_bytes.call_args.kwargs["data"] == b"preferred" def test_data_used_when_content_is_none(self): + """Fall back to the legacy data kwarg when content is not given.""" mock_uploader = MagicMock() with ( patch(f"{MOD}.parse_result_uri") as mock_parse, @@ -214,7 +252,10 @@ def test_data_used_when_content_is_none(self): class TestUploadS3: + """upload_to_result_uri() dispatches s3:// URIs to S3Uploader with the correct bucket/key/content/aws_profile.""" + def _call(self, uri="s3://my-bucket/results.json", content=b"payload", **kw): + """Drive upload_to_result_uri() through a mocked S3Uploader for a given URI/content/kwargs.""" mock_uploader = MagicMock() with ( patch(f"{MOD}.parse_result_uri") as mock_parse, @@ -225,43 +266,53 @@ def _call(self, uri="s3://my-bucket/results.json", content=b"payload", **kw): return mock_cls, mock_uploader def test_s3_uploader_instantiated(self): + """Instantiate S3Uploader exactly once for an s3:// RESULT_URI.""" mock_cls, _ = self._call() mock_cls.assert_called_once() def test_upload_bytes_called(self): + """Call S3Uploader.upload_bytes exactly once.""" _, mock_uploader = self._call() mock_uploader.upload_bytes.assert_called_once() def test_bucket_passed_correctly(self): + """Pass the exact bucket parsed from the RESULT_URI to upload_bytes.""" _, mock_uploader = self._call(uri="s3://target-bucket/results.json") assert mock_uploader.upload_bytes.call_args.kwargs["bucket"] == "target-bucket" def test_key_passed_correctly(self): + """Pass the exact key parsed from the RESULT_URI to upload_bytes.""" _, mock_uploader = self._call(uri="s3://bucket/path/to/results.json") assert mock_uploader.upload_bytes.call_args.kwargs["key"] == "path/to/results.json" def test_payload_passed_correctly(self): + """Pass the given content bytes through to upload_bytes unmodified.""" _, mock_uploader = self._call(content=b'{"ok": true}') assert mock_uploader.upload_bytes.call_args.kwargs["data"] == b'{"ok": true}' def test_default_content_type_is_json(self): + """Default content_type to application/json when not given.""" _, mock_uploader = self._call() assert mock_uploader.upload_bytes.call_args.kwargs["content_type"] == "application/json" def test_custom_content_type_forwarded(self): + """Forward an explicitly given content_type to upload_bytes.""" _, mock_uploader = self._call(content_type="text/plain") assert mock_uploader.upload_bytes.call_args.kwargs["content_type"] == "text/plain" def test_aws_profile_passed_to_uploader(self): + """Pass an explicitly given aws_profile through to the S3Uploader constructor.""" mock_cls, _ = self._call(aws_profile="my-profile") assert mock_cls.call_args.kwargs["aws_profile"] == "my-profile" def test_aws_profile_none_when_not_set(self): + """Leave aws_profile as None when neither the argument nor AWS_PROFILE is set.""" with patch.dict("os.environ", {}, clear=True): mock_cls, _ = self._call() assert mock_cls.call_args.kwargs["aws_profile"] is None def test_aws_profile_from_env_when_not_passed(self): + """Fall back to the AWS_PROFILE environment variable when aws_profile is not passed explicitly.""" with ( patch(f"{MOD}.parse_result_uri") as mock_parse, patch(f"{MOD}.S3Uploader") as mock_cls, @@ -273,6 +324,7 @@ def test_aws_profile_from_env_when_not_passed(self): assert mock_cls.call_args.kwargs["aws_profile"] == "env-profile" def test_explicit_aws_profile_overrides_env(self): + """Prefer an explicitly given aws_profile over the AWS_PROFILE environment variable.""" with ( patch(f"{MOD}.parse_result_uri") as mock_parse, patch(f"{MOD}.S3Uploader") as mock_cls, @@ -288,10 +340,12 @@ def test_explicit_aws_profile_overrides_env(self): assert mock_cls.call_args.kwargs["aws_profile"] == "explicit-profile" def test_prefix_uri_normalized_to_results_json(self): + """Normalize a prefix-style s3:// RESULT_URI to a results.json key before uploading.""" _, mock_uploader = self._call(uri="s3://bucket/run-outputs") assert mock_uploader.upload_bytes.call_args.kwargs["key"].endswith("results.json") def test_azure_uploader_not_called_for_s3(self): + """Never construct an AzureBlobUploader when the resolved scheme is s3.""" with ( patch(f"{MOD}.parse_result_uri") as mock_parse, patch(f"{MOD}.S3Uploader", return_value=MagicMock()), @@ -308,9 +362,11 @@ def test_azure_uploader_not_called_for_s3(self): class TestUploadAzureBlob: + """upload_to_result_uri() dispatches azureblob:// URIs to AzureBlobUploader with the correct account/container/blob_path/auth.""" _URI = "azureblob://myaccount/mycontainer/path/results.json" def _call(self, uri=None, content=b"payload", **kw): + """Drive upload_to_result_uri() through a mocked AzureBlobUploader for a given URI/content/kwargs.""" uri = uri or self._URI mock_uploader = MagicMock() with ( @@ -323,30 +379,37 @@ def _call(self, uri=None, content=b"payload", **kw): return mock_cls, mock_uploader def test_azure_uploader_instantiated(self): + """Instantiate AzureBlobUploader exactly once for an azureblob:// RESULT_URI.""" mock_cls, _ = self._call() mock_cls.assert_called_once() def test_upload_bytes_called(self): + """Call AzureBlobUploader.upload_bytes exactly once.""" _, mock_uploader = self._call() mock_uploader.upload_bytes.assert_called_once() def test_container_passed_correctly(self): + """Pass the exact container parsed from the RESULT_URI to upload_bytes.""" _, mock_uploader = self._call() assert mock_uploader.upload_bytes.call_args.kwargs["container"] == "mycontainer" def test_blob_path_passed_correctly(self): + """Pass the exact blob path parsed from the RESULT_URI to upload_bytes.""" _, mock_uploader = self._call() assert mock_uploader.upload_bytes.call_args.kwargs["blob_path"] == "path/results.json" def test_payload_passed_correctly(self): + """Pass the given content bytes through to upload_bytes unmodified.""" _, mock_uploader = self._call(content=b'{"result": 1}') assert mock_uploader.upload_bytes.call_args.kwargs["data"] == b'{"result": 1}' def test_default_content_type_is_json(self): + """Default content_type to application/json when not given.""" _, mock_uploader = self._call() assert mock_uploader.upload_bytes.call_args.kwargs["content_type"] == "application/json" def test_custom_content_type_forwarded(self): + """Forward an explicitly given content_type to upload_bytes.""" _, mock_uploader = self._call(content_type="application/octet-stream") assert ( mock_uploader.upload_bytes.call_args.kwargs["content_type"] @@ -354,14 +417,17 @@ def test_custom_content_type_forwarded(self): ) def test_account_name_passed_to_uploader(self): + """Pass the account parsed from the RESULT_URI as account_name to the AzureBlobUploader constructor.""" mock_cls, _ = self._call() assert mock_cls.call_args.kwargs["account_name"] == "myaccount" def test_default_auth_is_managed_identity(self): + """Default the Azure auth mode to managed_identity when not overridden.""" mock_cls, _ = self._call() assert mock_cls.call_args.kwargs["auth"] == "managed_identity" def test_explicit_auth_connection_string_passed(self): + """Pass an explicitly requested connection_string auth mode through to the uploader.""" mock_cls, _ = self._call( azure_auth="connection_string", azure_connection_string="DefaultEndpointsProtocol=https;...", @@ -369,6 +435,7 @@ def test_explicit_auth_connection_string_passed(self): assert mock_cls.call_args.kwargs["auth"] == "connection_string" def test_connection_string_passed_to_uploader(self): + """Pass an explicitly given azure_connection_string through to the uploader.""" conn = "DefaultEndpointsProtocol=https;AccountName=x;..." mock_cls, _ = self._call( azure_auth="connection_string", @@ -377,6 +444,7 @@ def test_connection_string_passed_to_uploader(self): assert mock_cls.call_args.kwargs["connection_string"] == conn def test_env_connection_string_used_when_not_passed(self): + """Fall back to the AZURE_STORAGE_CONNECTION_STRING environment variable when azure_connection_string is not passed.""" conn = "DefaultEndpointsProtocol=https;AccountName=env;..." mock_uploader = MagicMock() with ( @@ -402,6 +470,7 @@ def test_env_connection_string_upgrades_auth_to_connection_string(self): assert mock_cls.call_args.kwargs["auth"] == "connection_string" def test_env_azure_auth_overrides_default(self): + """Fall back to the AZURE_AUTH environment variable to select the auth mode when not passed explicitly.""" mock_uploader = MagicMock() with ( patch(f"{MOD}.parse_result_uri") as mock_parse, @@ -413,6 +482,7 @@ def test_env_azure_auth_overrides_default(self): assert mock_cls.call_args.kwargs["auth"] == "connection_string" def test_s3_uploader_not_called_for_azure(self): + """Never construct an S3Uploader when the resolved scheme is azureblob.""" with ( patch(f"{MOD}.parse_result_uri") as mock_parse, patch(f"{MOD}.AzureBlobUploader", return_value=MagicMock()), @@ -424,10 +494,12 @@ def test_s3_uploader_not_called_for_azure(self): mock_s3.assert_not_called() def test_prefix_uri_normalized(self): + """Normalize a prefix-style azureblob:// RESULT_URI to a results.json blob path before uploading.""" _, mock_uploader = self._call(uri="azureblob://myaccount/mycontainer/run-outputs") assert mock_uploader.upload_bytes.call_args.kwargs["blob_path"].endswith("results.json") def test_returns_none(self): + """upload_to_result_uri() returns None on a successful Azure upload.""" with ( patch(f"{MOD}.parse_result_uri") as mock_parse, patch(f"{MOD}.AzureBlobUploader", return_value=MagicMock()), @@ -444,9 +516,12 @@ def test_returns_none(self): class TestUploadGCS: + """upload_to_result_uri() dispatches gs:// URIs to google-cloud-storage's blob.upload_from_string with the correct bucket/blob/content.""" + _URI = "gs://my-bucket/path/results.json" def _make_gcs_storage_mock(self): + """Build a mocked google.cloud.storage module with a chained Client/bucket/blob mock.""" mock_blob = MagicMock() mock_bucket = MagicMock() mock_bucket.blob.return_value = mock_blob @@ -457,6 +532,7 @@ def _make_gcs_storage_mock(self): return mock_storage, mock_blob def _call(self, uri=None, content=b"payload", **kw): + """Drive upload_to_result_uri() through a mocked google-cloud-storage module for a given URI/content/kwargs.""" uri = uri or self._URI mock_storage, mock_blob = self._make_gcs_storage_mock() mock_gcloud = MagicMock(storage=mock_storage) @@ -472,24 +548,29 @@ def _call(self, uri=None, content=b"payload", **kw): return mock_storage, mock_blob def test_gcs_upload_called(self): + """Call blob.upload_from_string exactly once for a gs:// RESULT_URI.""" _, mock_blob = self._call() mock_blob.upload_from_string.assert_called_once() def test_gcs_upload_content_passed(self): + """Pass the given content bytes through to upload_from_string unmodified.""" _, mock_blob = self._call(content=b"hello-gcs") args, _ = mock_blob.upload_from_string.call_args assert args[0] == b"hello-gcs" def test_gcs_upload_content_type_is_json(self): + """Default the GCS upload content_type to application/json.""" _, mock_blob = self._call() _, kwargs = mock_blob.upload_from_string.call_args assert kwargs.get("content_type") == "application/json" def test_gcs_upload_uses_correct_bucket(self): + """Look up the exact bucket name parsed from the gs:// RESULT_URI.""" mock_storage, _ = self._call(uri="gs://target-bucket/results.json") mock_storage.Client.return_value.bucket.assert_called_with("target-bucket") def test_gcs_upload_uses_correct_blob_path(self): + """Look up the exact blob path parsed from the gs:// RESULT_URI.""" mock_storage, _ = self._call(uri="gs://bucket/path/to/results.json") mock_storage.Client.return_value.bucket.return_value.blob.assert_called_with( "path/to/results.json" @@ -510,6 +591,7 @@ def test_gcs_missing_package_raises_runtime_error(self): upload_to_result_uri(result_uri=self._URI, content=b"x") def test_gcs_invalid_uri_empty_bucket_raises(self): + """Reject a gs:// RESULT_URI with an empty bucket component.""" mock_storage, _ = self._make_gcs_storage_mock() mock_gcloud = MagicMock(storage=mock_storage) with ( diff --git a/tests/test_workflow_runner.py b/tests/test_workflow_runner.py index 994c517..a1e82d5 100644 --- a/tests/test_workflow_runner.py +++ b/tests/test_workflow_runner.py @@ -4,6 +4,8 @@ mocked. These tests exercise the runner's command, environment, staging, result, and failure contracts without requiring workflow engines or cloud credentials. + +Developer: Manish Kumar """ from __future__ import annotations @@ -19,6 +21,7 @@ def test_json_env_returns_default_for_invalid_or_non_object(monkeypatch): + """Fall back to the given default when an env-var JSON payload is invalid or not a JSON object.""" monkeypatch.setenv("INPUTS_JSON", "not-json") assert runner._load_json_env("INPUTS_JSON", {"fallback": True}) == {"fallback": True} @@ -27,6 +30,7 @@ def test_json_env_returns_default_for_invalid_or_non_object(monkeypatch): def test_download_local_file_and_file_uri(tmp_path): + """Copy a local path or file:// URI to the destination path, creating parent directories as needed.""" source = tmp_path / "source.txt" source.write_text("payload") destination = tmp_path / "nested" / "copy.txt" @@ -40,6 +44,7 @@ def test_download_local_file_and_file_uri(tmp_path): def test_download_rejects_missing_and_unsupported_uris(tmp_path): + """Reject a missing local path, an unsupported URI scheme, and a malformed s3:// URI with distinct error messages.""" with pytest.raises(RuntimeError, match="Local path not found"): runner._download_uri_to_path(str(tmp_path / "missing"), tmp_path / "out") with pytest.raises(RuntimeError, match="Unsupported download URI scheme"): @@ -49,6 +54,7 @@ def test_download_rejects_missing_and_unsupported_uris(tmp_path): def test_download_s3_uses_bucket_key_and_destination(tmp_path, monkeypatch): + """Download an s3:// URI via boto3's download_file with the exact bucket, key, and destination path.""" client = MagicMock() monkeypatch.setitem(sys.modules, "boto3", SimpleNamespace(client=lambda name: client)) @@ -61,6 +67,7 @@ def test_download_s3_uses_bucket_key_and_destination(tmp_path, monkeypatch): def test_extract_tgz_and_run_command_boundaries(tmp_path, monkeypatch): + """Extract a bundle via `tar -xzf` and run a command as either a shell string or an argv list, propagating the subprocess return code.""" check_call = MagicMock() run = MagicMock(return_value=SimpleNamespace(returncode=4)) monkeypatch.setattr(runner.subprocess, "check_call", check_call) @@ -78,6 +85,7 @@ def test_extract_tgz_and_run_command_boundaries(tmp_path, monkeypatch): def test_upload_rejects_bad_s3_and_unknown_schemes(monkeypatch): + """Reject a malformed s3:// upload target and an unsupported RESULT_URI scheme.""" monkeypatch.setitem(sys.modules, "boto3", SimpleNamespace(client=lambda name: MagicMock())) with pytest.raises(RuntimeError, match="Bad S3 URI"): runner._upload_uri("s3://bucket", b"data") @@ -86,6 +94,7 @@ def test_upload_rejects_bad_s3_and_unknown_schemes(monkeypatch): def test_s3_put_file_uses_upload_file(tmp_path, monkeypatch): + """Upload a local file to S3 via boto3's upload_file with the exact bucket, key, and local path.""" client = MagicMock() monkeypatch.setitem(sys.modules, "boto3", SimpleNamespace(client=lambda name: client)) local = tmp_path / "input.txt" @@ -96,6 +105,7 @@ def test_s3_put_file_uses_upload_file(tmp_path, monkeypatch): def test_upload_local_and_s3_boundaries(tmp_path, monkeypatch): + """Write result bytes to a local path or upload them to S3 with the given content type, depending on the target URI scheme.""" local = tmp_path / "results.json" runner._upload_uri(str(local), b"local") assert local.read_bytes() == b"local" @@ -109,6 +119,7 @@ def test_upload_local_and_s3_boundaries(tmp_path, monkeypatch): def test_upload_azure_requires_valid_uri_and_connection(monkeypatch): + """Upload to azureblob:// via a connection-string-authenticated BlobServiceClient, rejecting a URI missing container/blob or a missing AZURE_STORAGE_CONNECTION_STRING.""" blob_service = MagicMock() azure_blob = SimpleNamespace(BlobServiceClient=blob_service) monkeypatch.setitem(sys.modules, "azure.storage.blob", azure_blob) @@ -125,6 +136,7 @@ def test_upload_azure_requires_valid_uri_and_connection(monkeypatch): def test_command_and_nextflow_helpers(): + """RESULT_URI normalization, S3 bucket/prefix extraction, Nextflow-command detection, and -profile injection each behave correctly across their documented edge cases.""" assert runner._normalize_result_uri("s3://bucket/run/results.json") == ( "s3://bucket/run/results.json", "s3://bucket/run/outputs.json", ) @@ -152,6 +164,7 @@ def test_command_and_nextflow_helpers(): def test_patch_nextflow_for_aws_uses_result_uri_fallback(monkeypatch): + """Derive the S3 work-dir for AWS Batch from RESULT_URI when S3_RESULTS_BUCKET/PREFIX are unset.""" monkeypatch.delenv("S3_RESULTS_BUCKET", raising=False) monkeypatch.delenv("S3_RESULTS_PREFIX", raising=False) command, extra_env = runner._patch_nextflow_for_aws( @@ -164,6 +177,7 @@ def test_patch_nextflow_for_aws_uses_result_uri_fallback(monkeypatch): def test_patch_nextflow_for_aws_honors_configured_bucket_and_existing_work_dir(monkeypatch): + """Prefer configured S3_RESULTS_BUCKET/PREFIX over RESULT_URI and leave an explicit -work-dir untouched.""" monkeypatch.setenv("S3_RESULTS_BUCKET", "configured") monkeypatch.setenv("S3_RESULTS_PREFIX", "/prefix/") command, extra_env = runner._patch_nextflow_for_aws( @@ -176,6 +190,7 @@ def test_patch_nextflow_for_aws_honors_configured_bucket_and_existing_work_dir(m def test_stage_inputs_rewrites_nested_duplicate_paths(tmp_path, monkeypatch): + """Upload each distinct local input path to S3 once and rewrite all occurrences (including nested duplicates) to the same S3 key, recording a stage manifest.""" source = tmp_path / "input.fastq" source.write_text("reads") (tmp_path / "exec").mkdir() @@ -198,6 +213,7 @@ def test_stage_inputs_rewrites_nested_duplicate_paths(tmp_path, monkeypatch): def test_stage_inputs_requires_bucket(tmp_path): + """Reject staging inputs to S3 when the target bucket is empty.""" with pytest.raises(RuntimeError, match="S3 bucket is empty"): runner._stage_and_rewrite_inputs_to_s3( {}, bucket="", base_prefix="runs", run_id="r1", exec_root=tmp_path, @@ -205,6 +221,7 @@ def test_stage_inputs_requires_bucket(tmp_path): def test_stage_inputs_preserves_non_string_values(tmp_path, monkeypatch): + """Leave non-string input values (numbers, booleans) untouched and unrecorded in the staging manifest.""" monkeypatch.setattr(runner, "_s3_put_file", MagicMock()) rewritten, manifest = runner._stage_and_rewrite_inputs_to_s3( {"count": 3, "enabled": True}, @@ -215,6 +232,7 @@ def test_stage_inputs_preserves_non_string_values(tmp_path, monkeypatch): def test_input_and_parameter_helpers_preserve_existing_values(tmp_path): + """Local-path detection, file-path resolution, content hashing, and Nextflow/CLI argument helpers each behave correctly for present and absent values.""" local = tmp_path / "input.txt" local.write_text("x") assert runner._looks_like_local_path(str(local)) @@ -238,6 +256,7 @@ def test_input_and_parameter_helpers_preserve_existing_values(tmp_path): def test_apply_aws_env_does_not_overwrite_existing_values(): + """Derive AWS Batch queue/region child-env vars from workflow inputs without overwriting an already-set AWS_REGION.""" child_env = {"AWS_REGION": "existing"} runner._apply_aws_env_from_inputs( child_env, {"aws_queue": " queue ", "aws_region": " us-east-1 "} @@ -249,6 +268,7 @@ def test_apply_aws_env_does_not_overwrite_existing_values(): def test_main_local_success_writes_and_uploads_results(tmp_path, monkeypatch): + """A successful local Nextflow run uploads both the outputs file and a results.json reporting ok: true.""" uploads = [] executed = {} @@ -275,6 +295,7 @@ def fake_run(cmd, cwd, env): def test_main_failure_records_exit_code_and_upload_error(tmp_path, monkeypatch): + """A nonzero tool exit code is recorded in results.json, and a failed outputs upload is captured as outputs_upload_error rather than crashing the runner.""" uploads = [] monkeypatch.setenv("RUN_ID", "failed") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) @@ -301,6 +322,7 @@ def test_main_failure_records_exit_code_and_upload_error(tmp_path, monkeypatch): ], ) def test_main_selects_legacy_engine_commands(tmp_path, monkeypatch, engine, workflow, expected): + """Select the correct default command (nextflow/snakemake/cwltool) for each legacy engine name given only a workflow file.""" captured = {} monkeypatch.setenv("RUN_ID", f"{engine}-run") monkeypatch.setenv("RESULT_URI", str(tmp_path / f"{engine}.json")) @@ -314,6 +336,7 @@ def test_main_selects_legacy_engine_commands(tmp_path, monkeypatch, engine, work def test_main_bundle_mode_downloads_entrypoint_and_input_json(tmp_path, monkeypatch): + """Bundle mode downloads and extracts the workflow bundle, downloads input_json_uri, and builds a command referencing the resolved entrypoint and AWS params.""" captured = {} monkeypatch.setenv("RUN_ID", "bundle-run") monkeypatch.setenv("RESULT_URI", str(tmp_path / "bundle-results.json")) @@ -348,6 +371,7 @@ def fake_extract(tgz_path, destination): def test_main_bundle_mode_rejects_missing_entrypoint(tmp_path, monkeypatch): + """Reject bundle mode when the declared workflow_entrypoint is not present after extraction.""" monkeypatch.setenv("RUN_ID", "bad-bundle") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) monkeypatch.setenv("WORK_ROOT", str(tmp_path / "work")) @@ -363,6 +387,7 @@ def test_main_bundle_mode_rejects_missing_entrypoint(tmp_path, monkeypatch): def test_main_copies_local_bundle_and_uploads_normalized_outputs(tmp_path, monkeypatch): + """A local bundle path is copied into the exec root, and both raw and normalized outputs are uploaded and reflected in results.json.""" bundle = tmp_path / "bundle" bundle.mkdir() (bundle / "config.json").write_text("config") @@ -391,6 +416,7 @@ def fake_run(cmd, cwd, env): def test_main_removes_existing_local_bundle_before_copy(tmp_path, monkeypatch): + """Remove a stale bundle directory from a prior run before copying in the current run's local bundle.""" bundle = tmp_path / "bundle" bundle.mkdir() (bundle / "new.txt").write_text("new") @@ -417,6 +443,7 @@ def fake_run(cmd, cwd, env): def test_main_uses_command_str_and_shlex_for_nextflow(tmp_path, monkeypatch): + """Parse a shell-quoted command_str via shlex into the exact argv passed to the runner.""" captured = {} monkeypatch.setenv("RUN_ID", "string-command") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) @@ -430,6 +457,7 @@ def test_main_uses_command_str_and_shlex_for_nextflow(tmp_path, monkeypatch): def test_main_records_normalized_output_upload_failure(tmp_path, monkeypatch): + """A failed normalized-output upload is captured as outputs_normalized_upload_error without failing the overall run.""" uploads = [] monkeypatch.setenv("RUN_ID", "normalized-failure") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) @@ -453,6 +481,7 @@ def fake_upload(uri, data, content_type="application/json"): def test_main_omits_oversized_outputs_from_result(tmp_path, monkeypatch): + """Replace an oversized outputs payload with a note pointing to outputs_uri instead of inlining it into results.json.""" uploads = [] monkeypatch.setenv("RUN_ID", "large-output") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) @@ -472,6 +501,7 @@ def fake_run(cmd, cwd, env): def test_main_catches_process_exception_and_parses_malformed_outputs(tmp_path, monkeypatch): + """A subprocess-launch exception is recorded as results['error'], and malformed outputs.json content is reported as a parse error rather than propagating.""" uploads = [] monkeypatch.setenv("RUN_ID", "exception") monkeypatch.setenv("RESULT_URI", str(tmp_path / "results.json")) @@ -492,6 +522,7 @@ def fail_run(cmd, cwd, env): def test_main_aws_stages_inputs_and_patches_nextflow(tmp_path, monkeypatch): + """AWS mode stages local file inputs to S3, patches the Nextflow command for awsbatch with a run-scoped work-dir, and forwards queue/region into the child environment.""" source = tmp_path / "reads.fastq" source.write_text("reads") uploads = [] @@ -524,6 +555,7 @@ def fake_run(cmd, cwd, env): def test_main_requires_run_id(monkeypatch): + """Reject running the workflow when RUN_ID is not set.""" monkeypatch.delenv("RUN_ID", raising=False) with pytest.raises(RuntimeError, match="RUN_ID is required"): runner.main()