PR #13318: fix(tests): make test_modal_terminal actually assert, not return booleans

• Repository: NousResearch/hermes-agent
• Author: Sanjays2402
• State: open | merged: False
• Link: https://github.com/NousResearch/hermes-agent/pull/13318

Description (problem / solution / changelog)

Summary

Closes #13286.

Six of the tests in tests/integration/test_modal_terminal.py used:

success = <condition>
...
return success

Pytest treats a returned value as a warning, not a failure. Every one of these tests has been reported as PASSED regardless of whether Modal actually worked — the file was effectively a no-op under pytest, masking real Modal terminal regressions.

The fix

• Replace return success / return isolated / return requirements_met with assert <value>, <diagnostic message>. The diagnostic includes exit code and error text so a CI failure is actionable, e.g.:

  assert success, f"Simple command failed: exit={result_json.get('exit_code')}, error={result_json.get('error')}"

• Replace the TERMINAL_ENV != modal → return False in test_modal_requirements() with pytest.skip(...), which is the correct signal when a precondition isn't met. The test is no longer misreported as PASSED when Modal isn't configured — it's cleanly skipped.

• Keep the imperative main() runner usable (for the python tests/integration/test_modal_terminal.py usage the module docstring advertises). Rewrote it to tolerate AssertionError / pytest.skip.Exception and classify each test as PASSED / FAILED / SKIPPED in the summary.

Diff

-    if config['env_type'] != 'modal':
-        print(...)
-        return False
+    if config['env_type'] != 'modal':
+        print(...)
+        pytest.skip(f"TERMINAL_ENV='{config['env_type']}', skipping Modal-specific test")
...
-    return success
+    assert success, f"Simple command failed: exit={result_json.get('exit_code')}, error={result_json.get('error')}"

One file changed, +52 / −29 lines.

Verification

• python3 -m py_compile tests/integration/test_modal_terminal.py → clean.
• Walked the file top to bottom, confirmed all six test bodies now end with an assert carrying a descriptive message, and that test_modal_requirements() uses pytest.skip() instead of return False when TERMINAL_ENV != "modal".
• main() still works as a script runner: any pytest.skip.Exception or AssertionError raised by a test function is caught, classified, and reported in the summary — matching the previous script-style behavior as closely as possible while letting pytest do the right thing.

Not in scope

• #13287 (test_web_tools.py collects 0 tests) — addressed by a different PR / decision about whether to rename or add pytest markers.
• Reworking these tests into proper @pytest.mark.modal fixtures with conftest auto-skip — would be a larger refactor; this PR intentionally stays minimal and just makes the existing tests actually assert.

Linked Issue

• Closes #13286

Changed files

• tests/integration/test_modal_terminal.py (modified, +52/-29)

PR #13325: fix(tests): assert modal terminal integration failures

• Repository: NousResearch/hermes-agent
• Author: sgaofen
• State: open | merged: False
• Link: https://github.com/NousResearch/hermes-agent/pull/13325

Description (problem / solution / changelog)

Summary

• Fixes #13286
• Converts Modal terminal integration tests from boolean-returning pytest tests to assertion-based tests
• Keeps the manual script flow available through boolean helper functions and skips cleanly when Modal is not configured

Root Cause

Pytest ignores truthy/falsy return values from test functions, so logical Modal failures could still pass under pytest collection.

Tests

• uv run --frozen --python 3.11 --extra dev pytest -o addopts='' --collect-only tests/integration/test_modal_terminal.py (6 tests collected)
• uv run --frozen --python 3.11 --extra dev pytest -o addopts='' tests/integration/test_modal_terminal.py -q (6 skipped, Modal not configured locally)
• git diff --check -- tests/integration/test_modal_terminal.py

Changed files

• tests/integration/test_modal_terminal.py (modified, +218/-144)