Add unit tests for osism/tasks/__init__.py

[berendt]

Closes #2353

Adds unit test coverage for the shared Celery-worker helpers in osism/tasks/__init__.py, which previously had no dedicated test module.

Change set (single commit)

876ab59 — Add unit tests for osism/tasks/init.py

Introduces tests/unit/tasks/test_init.py (775 lines) covering the module-level helpers that back the Celery workers — none of which are Celery tasks themselves, so they are exercised directly without a broker:

• HOST_PATTERN — the host-extraction regex: parametrized match/no-match cases, including the pinned ok: [src -> dst] delegation capture and documented gaps (fatal: lines, leading whitespace).
• Config — the static flags (task_default_queue, task_create_missing_queues, broker_connection_retry_on_startup, enable_utc) and the complete task_routes queue mapping.
• get_container_version
• log_play_execution
• run_ansible_in_environment and run_command — both subprocess runners, driven by a make_process helper that fakes Popen.poll/stdout.readline/wait.
• handle_task — the CLI-side wait/revoke helper.

Notes for the reviewer

Two current behaviors are pinned as-is (characterization tests, not endorsements):

• run_command with locking=True creates and releases a redlock but never calls acquire().
• Tuple arguments are not joined, because run_ansible_in_environment uses an exact-type list check.

Config.broker_url / Config.result_backend precedence and task_track_started are already pinned by tests/unit/tasks/test_config.py (which reloads osism.settings / osism.tasks to exercise import-time resolution); this module deliberately does not duplicate them. Because test_config.py reloads osism.tasks, everything under test is accessed as a module attribute (tasks.Config, tasks.get_container_version, …) rather than imported by name, so a binding cannot go stale depending on test order. Loguru output is invisible to pytest's caplog, so the loguru_logs fixture from tests/conftest.py is used for every "warning/error logged" assertion.

No divergence from the issue: the change is test-only and adds no production code.

---

Implemented by planwerk-agent 4c14036 with Claude:claude-opus-4-8

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The single 775-line test_init.py file covers many distinct behaviors; consider splitting it into smaller, focused test modules (e.g., per helper or concern) to keep future maintenance and navigation manageable.
• The patched_path fixture replaces osism.tasks.Path with a simple lambda that only preserves the basename, which could break if the production code starts using more complex Path operations; consider wrapping/monkeypatching with a real Path-like object instead of a plain lambda to make these tests more resilient to implementation changes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The single 775-line `test_init.py` file covers many distinct behaviors; consider splitting it into smaller, focused test modules (e.g., per helper or concern) to keep future maintenance and navigation manageable.
- The `patched_path` fixture replaces `osism.tasks.Path` with a simple lambda that only preserves the basename, which could break if the production code starts using more complex `Path` operations; consider wrapping/monkeypatching with a real `Path`-like object instead of a plain lambda to make these tests more resilient to implementation changes.

## Individual Comments

### Comment 1
<location path="tests/unit/tasks/test_init.py" line_range="638-643" />
<code_context>
+    assert passed is not env
+
+
+def test_run_command_accumulates_output_and_publishes(command_mocks):
+    command_mocks.popen.return_value = make_process(["out-1\n", "out-2\n"], rc=0)
+    result = tasks.run_command("req-1", "echo", {})
+    assert result == "out-1\nout-2\n"
+    assert command_mocks.push.call_count == 2
+    command_mocks.finish.assert_called_once_with("req-1", rc=0)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a `run_command` test for non-zero exit codes to ensure the failure path is covered and `finish_task_output` is called with the correct `rc`.

Existing tests only cover the success path (`rc=0`) and don't exercise a non-zero return code. Please add a test, similar to `test_run_command_accumulates_output_and_publishes` but using `make_process([...], rc=1)`, asserting that the aggregated output is returned and `finish_task_output` is called with `rc=1` to lock in the failure behavior.

Suggested implementation:

```python
def test_run_command_merges_env_with_os_environ(command_mocks, monkeypatch):
    monkeypatch.setenv("INHERITED_KEY", "inherited")
    env = {"INJECTED_KEY": "injected"}
    tasks.run_command("req-1", "echo", env, ignore_env=False)
    passed = command_mocks.popen.call_args.kwargs["env"]
    assert passed["INJECTED_KEY"] == "injected"
    assert passed["INHERITED_KEY"] == "inherited"
    assert passed is not env


def test_run_command_accumulates_output_and_publishes(command_mocks):
    command_mocks.popen.return_value = make_process(["out-1\n", "out-2\n"], rc=0)
    result = tasks.run_command("req-1", "echo", {})
    assert result == "out-1\nout-2\n"
    assert command_mocks.push.call_count == 2
    command_mocks.finish.assert_called_once_with("req-1", rc=0)


def test_run_command_failure_aggregates_output_and_finishes_with_non_zero_rc(command_mocks):
    command_mocks.popen.return_value = make_process(["err-1\n", "err-2\n"], rc=1)
    result = tasks.run_command("req-2", "echo", {})
    assert result == "err-1\nerr-2\n"
    assert command_mocks.push.call_count == 2
    command_mocks.finish.assert_called_once_with("req-2", rc=1)



"""Unit tests for the module-level helpers in :mod:`osism.tasks`.

```

If the existing test names or fixtures differ slightly (e.g. the helper is named `finish_task_output` instead of `finish` on `command_mocks`), adjust the new test to use the same mock attribute and naming conventions as `test_run_command_accumulates_output_and_publishes`. The new test should mirror the success-path test exactly, changing only the `rc`, the request id, and optionally the output strings.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}