From e8ff3d12aa67365bd8b1bc98fdc35ea4455a9594 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 13:29:40 +0000 Subject: [PATCH 1/2] fix(lib): give local dry-run services an id so rendering works `blackfish run --dry-run` crashed with `AttributeError: 'NoneType' object has no attribute 'hex'` for local profiles: the local branch built its service without an `id`, unlike the Slurm branch, and `render_job_script` reads `self.id.hex`. Since a dry-run service is never flushed to the database, nothing else assigns one. Add `id=uuid4()` to the local constructors in both `text_generation` and `speech_recognition`, matching the Slurm branch. The dry-run tests missed this because they patched the service class, so the real renderer never ran. Drop those patches and assert on the script body rather than only the metadata echoed above it. `mock_config` now carries a real `CONTAINER_PROVIDER`, since the local templates branch on `provider == "docker"` and would otherwise render an empty script. Closes #494 --- .../cli/services/speech_recognition.py | 1 + .../blackfish/cli/services/text_generation.py | 1 + lib/tests/cli/conftest.py | 5 ++ lib/tests/cli/test_cli_run.py | 50 +++++++------------ 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/lib/src/blackfish/cli/services/speech_recognition.py b/lib/src/blackfish/cli/services/speech_recognition.py index c7c20353..39b79b53 100644 --- a/lib/src/blackfish/cli/services/speech_recognition.py +++ b/lib/src/blackfish/cli/services/speech_recognition.py @@ -209,6 +209,7 @@ def run_speech_recognition( if dry_run: service = SpeechRecognition( + id=uuid4(), name=name, model=repo_id, profile=profile.name, diff --git a/lib/src/blackfish/cli/services/text_generation.py b/lib/src/blackfish/cli/services/text_generation.py index d9327c78..4783c56a 100644 --- a/lib/src/blackfish/cli/services/text_generation.py +++ b/lib/src/blackfish/cli/services/text_generation.py @@ -212,6 +212,7 @@ def run_text_generation( if dry_run: service = TextGeneration( + id=uuid4(), name=name, model=repo_id, profile=profile.name, diff --git a/lib/tests/cli/conftest.py b/lib/tests/cli/conftest.py index 583d52fb..fec19caf 100644 --- a/lib/tests/cli/conftest.py +++ b/lib/tests/cli/conftest.py @@ -3,6 +3,8 @@ from click.testing import CliRunner from unittest.mock import patch +from blackfish.server.config import ContainerProvider + @pytest.fixture() def cli_runner() -> CliRunner: @@ -18,4 +20,7 @@ def mock_config(): mock_config.HOME_DIR = ( Path(__file__).parent.parent / "tests", ) # "/tmp/blackfish-test" + # A real provider, so job scripts rendered from this config aren't + # empty: the templates branch on `provider == "docker"`. + mock_config.CONTAINER_PROVIDER = ContainerProvider.Docker yield mock_config diff --git a/lib/tests/cli/test_cli_run.py b/lib/tests/cli/test_cli_run.py index 9ec0598a..b40a375f 100644 --- a/lib/tests/cli/test_cli_run.py +++ b/lib/tests/cli/test_cli_run.py @@ -8,7 +8,7 @@ import shlex import pytest import requests -from unittest.mock import patch, Mock, MagicMock +from unittest.mock import patch, Mock from blackfish.cli.__main__ import main from blackfish.server.models.profile import LocalProfile, SlurmProfile @@ -99,9 +99,6 @@ def test_dry_run_local_profile(self, cli_runner, mock_config, local_profile): patch( "blackfish.cli.services.text_generation.get_model_dir" ) as mock_get_model_dir, - patch( - "blackfish.cli.services.text_generation.TextGeneration" - ) as mock_service_class, ): mock_deserialize.return_value = local_profile mock_get_models.return_value = ["openai/gpt-2"] @@ -109,16 +106,15 @@ def test_dry_run_local_profile(self, cli_runner, mock_config, local_profile): mock_get_latest.return_value = "abc123" mock_get_model_dir.return_value = "/path/to/model" - mock_service = MagicMock() - mock_service.image = "text_generation" - mock_service.render_job_script.return_value = "#!/bin/bash\necho test" - mock_service_class.return_value = mock_service - result = cli_runner.invoke(main, cmd) + assert result.exit_code == 0, result.exception assert "Rendering job script" in result.output assert "model: openai/gpt-2" in result.output assert "profile: default" in result.output + # The script itself, not just the echoed metadata above it. + script = result.output.split("> image_ref:")[-1] + assert "docker run" in script def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): """Test dry run with SlurmProfile renders job script.""" @@ -147,9 +143,6 @@ def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): patch( "blackfish.cli.services.text_generation.get_model_dir" ) as mock_get_model_dir, - patch( - "blackfish.cli.services.text_generation.TextGeneration" - ) as mock_service_class, ): mock_deserialize.return_value = slurm_profile mock_get_models.return_value = ["openai/gpt-2"] @@ -157,17 +150,16 @@ def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): mock_get_latest.return_value = "abc123" mock_get_model_dir.return_value = "/path/to/model" - mock_service = MagicMock() - mock_service.scheduler = "slurm" - mock_service.render_job_script.return_value = "#!/bin/bash\n#SBATCH" - mock_service_class.return_value = mock_service - result = cli_runner.invoke(main, cmd) + assert result.exit_code == 0, result.exception assert "Rendering job script" in result.output assert "model: openai/gpt-2" in result.output assert "profile: cluster" in result.output assert "host: hpc.example.com" in result.output + # The script itself, not just the echoed metadata above it. + script = result.output.split("> image_ref:")[-1] + assert "#SBATCH" in script def test_success_local_profile(self, cli_runner, mock_config, local_profile): """Test successful API call with LocalProfile.""" @@ -627,9 +619,6 @@ def test_dry_run_local_profile(self, cli_runner, mock_config, local_profile): patch( "blackfish.cli.services.speech_recognition.get_model_dir" ) as mock_get_model_dir, - patch( - "blackfish.cli.services.speech_recognition.SpeechRecognition" - ) as mock_service_class, ): mock_deserialize.return_value = local_profile mock_get_models.return_value = ["openai/whisper-tiny"] @@ -637,16 +626,15 @@ def test_dry_run_local_profile(self, cli_runner, mock_config, local_profile): mock_get_latest.return_value = "abc123" mock_get_model_dir.return_value = "/path/to/models/whisper-tiny" - mock_service = MagicMock() - mock_service.image = "speech_recognition" - mock_service.render_job_script.return_value = "#!/bin/bash\necho test" - mock_service_class.return_value = mock_service - result = cli_runner.invoke(main, cmd) + assert result.exit_code == 0, result.exception assert "Rendering job script" in result.output assert "model: openai/whisper-tiny" in result.output assert "profile: default" in result.output + # The script itself, not just the echoed metadata above it. + script = result.output.split("> image_ref:")[-1] + assert "docker run" in script def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): """Test dry run with SlurmProfile renders job script.""" @@ -675,9 +663,6 @@ def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): patch( "blackfish.cli.services.speech_recognition.get_model_dir" ) as mock_get_model_dir, - patch( - "blackfish.cli.services.speech_recognition.SpeechRecognition" - ) as mock_service_class, ): mock_deserialize.return_value = slurm_profile mock_get_models.return_value = ["openai/whisper-tiny"] @@ -685,17 +670,16 @@ def test_dry_run_slurm_profile(self, cli_runner, mock_config, slurm_profile): mock_get_latest.return_value = "abc123" mock_get_model_dir.return_value = "/path/to/models/whisper-tiny" - mock_service = MagicMock() - mock_service.scheduler = "slurm" - mock_service.render_job_script.return_value = "#!/bin/bash\n#SBATCH" - mock_service_class.return_value = mock_service - result = cli_runner.invoke(main, cmd) + assert result.exit_code == 0, result.exception assert "Rendering job script" in result.output assert "model: openai/whisper-tiny" in result.output assert "profile: cluster" in result.output assert "host: hpc.example.com" in result.output + # The script itself, not just the echoed metadata above it. + script = result.output.split("> image_ref:")[-1] + assert "#SBATCH" in script def test_success_local_profile(self, cli_runner, mock_config, local_profile): """Test successful API call with LocalProfile.""" From 4c32fca09740b7cab8b027bc5fc20dcf7122ca90 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 13:48:17 +0000 Subject: [PATCH 2/2] chore(lib): update coverage badge to 72% The dry-run tests now exercise the real job-script renderer and templates instead of a mocked service, which covers a few more lines. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011Aj5BVf85qjSaKT9kJxap8 --- lib/docs/assets/img/coverage.svg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/docs/assets/img/coverage.svg b/lib/docs/assets/img/coverage.svg index ffd257bd..f5af1dbe 100644 --- a/lib/docs/assets/img/coverage.svg +++ b/lib/docs/assets/img/coverage.svg @@ -15,7 +15,7 @@ coverage coverage - 71% - 71% + 72% + 72%