Skip to content

🧪 [testing improvement: observability interface tests] #2606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SatoryKono merged 2 commits into from
Apr 3, 2026
Merged

🧪 [testing improvement: observability interface tests] #2606

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfec62f
🧪 [testing improvement: observability interface tests]
google-labs-jules[bot] Apr 2, 2026
74ea869
🧪 [testing improvement: observability interface tests]
google-labs-jules[bot] Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions tests/unit/interfaces/test_observability.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_start_metrics_server_success():


def test_start_metrics_server_failure():
"""Simulate server failure and verify graceful handling.
"""Simulate server failure and verify graceful handling via interface.

The server is designed to catch exceptions and return False (fail_fast=False by default)
rather than raising exceptions, to allow pipelines to continue without metrics.
Expand All @@ -46,11 +46,40 @@ def test_start_metrics_server_failure():
"bioetl.infrastructure.observability.server.start_http_server",
side_effect=RuntimeError("Failed"),
):
# Server catches exceptions and returns False for graceful degradation
result = obs_server.start_metrics_server(port=8000, fail_fast=False)
# Interface should catch exceptions and return False for graceful degradation
result = observability.start_metrics_server(port=8000, fail_fast=False)
Comment on lines +49 to +50
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify exception-handling ownership in the test comment.

At Line 49, the comment says the interface catches exceptions, but exception handling happens in bioetl.infrastructure.observability.server.start_metrics_server; the interface delegates. Please update wording to avoid architectural confusion.

Suggested wording fix 
-        # Interface should catch exceptions and return False for graceful degradation
+        # Interface delegates to infrastructure, which handles exceptions and returns
+        # False when fail_fast=False (graceful degradation).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Interface should catch exceptions and return False for graceful degradation
result = observability.start_metrics_server(port=8000, fail_fast=False)
# Interface delegates to infrastructure, which handles exceptions and returns
# False when fail_fast=False (graceful degradation).
result = observability.start_metrics_server(port=8000, fail_fast=False)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/interfaces/test_observability.py` around lines 49 - 50, Update the
test comment to clarify that exception handling is performed by the server
implementation and not the interface: replace the line stating "Interface should
catch exceptions and return False for graceful degradation" with wording that
the test verifies the interface delegates to
bioetl.infrastructure.observability.server.start_metrics_server and that when
that function raises and fail_fast=False the interface should propagate the
server's graceful-failure behavior (return False); reference the test call
observability.start_metrics_server(port=8000, fail_fast=False) and the server
function bioetl.infrastructure.observability.server.start_metrics_server to make
the ownership clear.

assert result is False


def test_start_metrics_server_fail_fast_propagation():
"""Verify that fail_fast=True propagates MetricsServerError via interface."""
with mock.patch(
"bioetl.infrastructure.observability.server.start_http_server",
side_effect=RuntimeError("Failed"),
):
with pytest.raises(domain_exceptions.MetricsServerError):
observability.start_metrics_server(port=8000, fail_fast=True)


def test_start_metrics_server_default_arguments():
"""Verify that default arguments are correctly passed through the interface."""
with mock.patch(
"bioetl.composition.observability_api.start_metrics_server",
return_value=True,
) as mock_impl:
result = observability.start_metrics_server()

assert result is True
mock_impl.assert_called_once_with(
port=8000,
addr="0.0.0.0",
fail_fast=False,
retry_count=3,
retry_delay=1.0,
logger=None,
)


def test_interface_re_exports_from_composition_observability_api():
"""Verify interface delegates start_metrics_server via composition API.

Expand Down
Loading