test: add direct unit tests for Mimir mixin (#881) by Piyushtiwari919 · Pull Request #1040 · Tracer-Cloud/opensre

Piyushtiwari919 · 2026-04-28T13:36:58Z

Introduces a DummyMimirClient with a mocked _make_request method to isolate and test the MimirMixin without making real HTTP requests. Coverage includes PromQL query string construction (plain and service-filtered), result-series JSON normalization, and error envelope handling for missing configurations and network exceptions.

Fixes #881

Describe the changes you have made in this PR -

Added tests/services/test_grafana_mimir.py.
Created a DummyMimirClient class to attach the mixin and mock the internal _make_request network call using MagicMock.
Added 4 focused unit tests to verify:
- Plain metric queries: Verifies base PromQL string construction (e.g., cpu_usage_total).
- Service-filtered queries: Verifies correct bracket injection for service names (e.g., {service_name="backend-api"}).
- Result normalization: Verifies that deeply nested, messy Grafana JSON responses are properly flattened into the clean OpenSRE dictionary format.
- Error handling: Verifies early exit logic for unconfigured clients and safe failure envelopes for network exceptions.
CI/CD & Linter Fixes: Proactively addressed strict linter and CodeQL requirements by explicitly calling super().__init__() in the dummy class, adding proper -> None return type hints, and ensuring no unused imports remain in the file.

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

No, I wrote all the code myself
Yes, I used AI assistance (continue below)

If you used AI assistance:

I have reviewed every single line of the AI-generated code
I can explain the purpose and logic of each function/component I added
I have tested edge cases and understand how the code handles them
I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

What problem does your code solve? The MimirMixin builds dynamic PromQL queries and parses complex Grafana API responses, but lacked direct test coverage. This created a risk for silent failures if the data shape changed or network calls failed.
Alternative approaches: The alternative would be writing integration tests against a live Grafana Mimir instance. This is an anti-pattern for unit tests as it makes the CI/CD pipeline slow, flaky, and dependent on live API keys.
Why this implementation? Using MagicMock acts as a spy, allowing us to intercept the _make_request call to assert exactly what URL and parameters were sent. Injecting a fake JSON response allows us to test the for-loop parsing logic safely. This approach exactly mirrors the existing LokiMixin testing pattern, maintaining architectural consistency across the repository.
Key functions: DummyMimirClient sets up the mock environment. The four test_query_mimir_* functions handle the exact Acceptance Criteria from the issue: plain queries, filtered queries, data normalization, and exception catching.

Checklist before requesting a review

I have added proper PR title and linked to the issue
I have performed a self-review of my code
I can explain the purpose of every function, class, and logic block I added
I understand why my changes work and have tested them thoroughly
I have considered potential edge cases and how my code handles them
If it is a core feature, I have added thorough tests
My code follows the project's style guidelines and conventions

- Implements DummyMimirClient to mock internal requests without real network calls. - Validates PromQL query construction for both plain metrics and service-filtered queries. - Verifies result-series normalization of raw Grafana API JSON responses. - Covers edge cases including not-configured clients and exception-handling error envelopes. Closes Tracer-Cloud#881

greptile-apps · 2026-04-28T13:39:12Z

Greptile Summary

Adds tests/services/test_grafana_mimir.py with a DummyMimirClient harness and five focused unit tests covering MimirMixin query construction, result normalization, unconfigured early-exit, and exception handling. The test logic and assertions are all correct and accurately reflect the mimir.py source.

P1: Two pairs of consecutive top-level test functions are missing the required 2-blank-line separator (ruff E302) — make lint covers tests/ and this would fail CI.

Confidence Score: 4/5

Safe to merge after fixing the two missing blank-line separators that would fail ruff/CI lint.

All test assertions are logically correct against the mimir.py source. The only blocking issue is the ruff E302 lint violation (zero blank lines between two pairs of top-level functions) which would fail make lint in CI. Once fixed this is straightforward to merge.

tests/services/test_grafana_mimir.py — blank lines before test_query_mimir_result_normalization (line 43) and test_query_mimir_not_configured (line 71).

Important Files Changed

Filename	Overview
tests/services/test_grafana_mimir.py	New test file covering MimirMixin — logic and assertions are correct, but two pairs of consecutive top-level functions lack the required 2-blank-line separator (ruff E302, would fail CI lint), and `from __future__ import annotations` is missing per code-style conventions.

Sequence Diagram

sequenceDiagram
    participant Test as Test Function
    participant DC as DummyMimirClient
    participant MM as MimirMixin.query_mimir()
    participant BDU as _build_datasource_url (MagicMock)
    participant MR as _make_request (MagicMock)

    Test->>DC: DummyMimirClient(is_configured=True)
    Test->>MR: set return_value / side_effect
    Test->>MM: query_mimir(metric_name, service_name?)
    MM->>MM: check self.is_configured
    MM->>BDU: _build_datasource_url(mimir_datasource_uid, "/api/v1/query")
    BDU-->>MM: "https://fake-grafana.com/api/v1/query"
    MM->>MR: _make_request(url, params={query})
    MR-->>MM: fake_api_response
    MM-->>Test: {success, metrics, total_series, query, account_id}
    Test->>MR: assert_called_once_with(...)

_{Reviews (1): Last reviewed commit: "test: add direct unit tests for Mimir mi..." | Re-trigger Greptile}

greptile-apps · 2026-04-28T13:39:16Z

+    client._make_request.return_value = {"data": {"result": []}}
+
+    client.query_mimir("cpu_usage_total", service_name="backend-api")
+


Missing blank lines between top-level functions (E302)

Two top-level test functions are defined with zero blank lines separating them, violating PEP 8 / ruff rule E302 (expected 2 blank lines). make lint runs ruff check tests/, so this would fail CI.

test_query_mimir_service_filtered → test_query_mimir_result_normalization (line 43): 0 blank lines

test_query_mimir_result_normalization → test_query_mimir_not_configured (line 71): 0 blank lines

Add two blank lines before each of those function definitions.

Context Used: Code style and conventions (source)

muddlebee · 2026-04-28T14:28:11Z

+    # Requirement: Exception cases / error envelope
+    assert result["success"] is False
+    assert "Network timeout" in result["error"]
+    assert result["metrics"] == []


Only the generic Exception path is covered. MimirMixin has a second branch (lines 69-71) that fires when the exception carries a .response attribute, rewriting error_msg to 'Mimir query failed: <status_code>' and capturing response.text. Add a test using a mock exception with a .response stub to cover that path — otherwise a status-code-parsing regression is invisible.

Missed the hasattr block. I've added a new test with a mocked .response to cover the status code parsing. Thanks for the review!

muddlebee · 2026-04-28T14:28:21Z

+
+    metric_data = result["metrics"][0]
+    assert metric_data["metric"]["instance"] == "server-1"
+    assert metric_data["value"][1] == "95.5"


The success response from query_mimir also includes account_id (set to self.account_id). Missing assertion here means a future refactor could drop or rename that field silently. Add: assert result["account_id"] == "test_acc_123"

I've updated the test to include the account_id assertion so it doesn't get dropped in a future refactor. Just pushed!

muddlebee · 2026-04-28T14:28:33Z

+    client._make_request.return_value = {"data": {"result": []}}
+
+    client.query_mimir("cpu_usage_total", service_name="backend-api")
+


The greptile E302 report (missing blank lines between top-level functions) appears to be a false positive — the diff shows two blank lines between every top-level function definition. No action needed here, but worth running make lint locally to confirm before merge.

…ount_id in normalized response

github-actions · 2026-04-28T15:33:52Z

🎉 MERGED! @Piyushtiwari919 just shipped something. The diff gods are pleased. 🙌

👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

greptile-apps Bot reviewed Apr 28, 2026

View reviewed changes

Piyushtiwari919 added 2 commits April 28, 2026 19:09

style: apply ruff formatting to mimir tests

18f6385

fix: add missing from __future__ import annotations

5c3de8c

muddlebee reviewed Apr 28, 2026

View reviewed changes

test: add coverage for exception with response attribute & assert acc…

d53d584

…ount_id in normalized response

Piyushtiwari919 requested a review from muddlebee April 28, 2026 15:32

muddlebee merged commit 1653e37 into Tracer-Cloud:main Apr 28, 2026
7 checks passed

Piyushtiwari919 deleted the test/mimir-tests branch April 28, 2026 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add direct unit tests for Mimir mixin (#881)#1040

test: add direct unit tests for Mimir mixin (#881)#1040
muddlebee merged 4 commits intoTracer-Cloud:mainfrom
Piyushtiwari919:test/mimir-tests

Piyushtiwari919 commented Apr 28, 2026

Uh oh!

greptile-apps Bot commented Apr 28, 2026

Uh oh!

greptile-apps Bot Apr 28, 2026

Uh oh!

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

Piyushtiwari919 Apr 28, 2026

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

Piyushtiwari919 Apr 28, 2026

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 28, 2026 •

edited by muddlebee

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		client._make_request.return_value = {"data": {"result": []}}

		client.query_mimir("cpu_usage_total", service_name="backend-api")

Conversation

Piyushtiwari919 commented Apr 28, 2026

Describe the changes you have made in this PR -

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Checklist before requesting a review

Uh oh!

greptile-apps Bot commented Apr 28, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piyushtiwari919 Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piyushtiwari919 Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited by muddlebee Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 28, 2026 •

edited by muddlebee

Loading