Merged
Conversation
tmikula-dev
requested review from
lsulak,
oto-macenauer-absa and
petr-pokorny-absa
as code owners
WalkthroughAdded Dependabot configuration and a GitHub Actions workflow to auto-approve and attempt auto-merge Dependabot PRs; raised pytest coverage threshold from 80% to 90%; and extended/rewrote multiple unit tests for handlers, token handling, trace logging, and Postgres writer health checks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dependabot as "Dependabot"
participant Repo as "Repository (master)"
participant GitHubActions as "GitHub Actions\n(dependabot_auto)"
participant FetchMeta as "dependabot/fetch-metadata"
participant GHCLI as "gh (CLI)"
Note over Dependabot,Repo: Dependabot opens PRs (weekly)
Dependabot->>Repo: open PR (version/security update)
Repo->>GitHubActions: trigger workflow on PR opened/synchronize
GitHubActions->>FetchMeta: run action to fetch Dependabot metadata
FetchMeta-->>GitHubActions: outputs.update-type, pr_url
GitHubActions->>GHCLI: run `gh pr review --approve` with pr_url
alt update-type is version or security
GitHubActions->>GHCLI: run `gh pr merge --auto --squash` (continue-on-error)
GHCLI-->>GitHubActions: merge attempted / may fail
end
GHCLI-->>Repo: PR approved and (maybe) auto-merged
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
6-18:⚠️ Potential issue | 🟡 MinorUpdate stale
pytest-unittarget description.The target now enforces 90%, but the inline help still says
threshold >= 80%.Suggested change
-pytest-unit: ## Run unit tests with coverage (threshold >= 80%) +pytest-unit: ## Run unit tests with coverage (threshold >= 90%)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 6 - 18, The pytest-unit target's inline help text is stale (mentions "threshold >= 80%") while MIN_COVERAGE is set to 90; update the comment for the pytest-unit Makefile target to reflect the current threshold (e.g., change "threshold >= 80%" to "threshold >= 90%") so the target description matches the MIN_COVERAGE value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dependabot_auto.yml:
- Around line 4-5: The workflow currently triggers on pull_request types
["opened","synchronize"] and misses the "reopened" event; update the
pull_request types array in the Dependabot workflow (pull_request: types) to
include "reopened" so Dependabot PRs that are closed and later reopened will be
picked up for auto-approval/auto-merge.
- Around line 29-32: The step "Enable auto-merge for Dependabot PRs" currently
silences failures via continue-on-error: true; change this so auto-merge errors
are surfaced: remove continue-on-error and instead capture the gh pr merge exit
status/output (the command run: gh pr merge --auto --squash "$PR_URL") and on
non-zero exit either fail the job or write a clear error message to the workflow
logs (e.g., echo the gh output and exit 1) so merge-setup failures
(protection/review constraints) are not ignored.
In `@tests/unit/handlers/test_handler_api.py`:
- Around line 39-44: The test function
test_load_api_definition_empty_file_raises should use the pytest-mock fixture
instead of unittest.mock.patch: change the test signature to accept the mocker
fixture and replace patch("builtins.open", mock_open(read_data="")) with
mocker.patch("builtins.open", mock_open(read_data="")); keep the rest of the
test (HandlerApi instantiation and pytest.raises check) unchanged so the test
still asserts RuntimeError with message "API specification initialization
failed".
In `@tests/unit/handlers/test_handler_token.py`:
- Around line 107-108: Tests use unittest.mock.patch directly; update them to
use pytest-mock's mocker fixture for consistency: replace calls like patch(...)
and patch.object(...) with mocker.patch(...) and mocker.patch.object(...).
Specifically modify uses around token_handler.with_public_keys_queried and the
other occurrences called out (lines referencing
token_handler._refresh_keys_if_needed, with_public_keys_queried, and similar
mocks at the indicated ranges) so they call mocker.patch / mocker.patch.object
and return the same mocked object/behavior; ensure the mocker fixture is
accepted by the test functions (add a mocker parameter if missing).
In `@tests/unit/writers/test_writer_postgres.py`:
- Around line 388-398: In test_check_health_load_config_exception replace
monkeypatch.setattr with pytest-mock's mocker.patch.object: add the mocker
fixture to the test signature, call mocker.patch.object(writer,
"_load_db_config", side_effect=ValueError("secret fetch failed")) to simulate
the exception, and then call writer.check_health() as before; reference
WriterPostgres._load_db_config and writer.check_health when making the change.
---
Outside diff comments:
In `@Makefile`:
- Around line 6-18: The pytest-unit target's inline help text is stale (mentions
"threshold >= 80%") while MIN_COVERAGE is set to 90; update the comment for the
pytest-unit Makefile target to reflect the current threshold (e.g., change
"threshold >= 80%" to "threshold >= 90%") so the target description matches the
MIN_COVERAGE value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c2edbaf-6334-4178-926f-58cfcf387af5
📒 Files selected for processing (8)
.github/dependabot.yml.github/workflows/dependabot_auto.ymlDEVELOPER.mdMakefiletests/unit/handlers/test_handler_api.pytests/unit/handlers/test_handler_token.pytests/unit/utils/test_trace_logging.pytests/unit/writers/test_writer_postgres.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/writers/test_writer_postgres.py (1)
384-384: Use expected-first boolean assertions for consistency.Line 384 and Line 396 use
assert healthy/assert not healthy; this diverges from the test assertion convention used in this repo.Suggested change
- assert healthy + assert True == healthy @@ - assert not healthy + assert False == healthyAs per coding guidelines,
tests/**/*.pyshould use the assert pattern:assert expected == actual.Also applies to: 396-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/writers/test_writer_postgres.py` at line 384, The test uses bare boolean assertions; change the two occurrences of "assert healthy" and "assert not healthy" in tests/unit/writers/test_writer_postgres.py to the repo's expected-first style by asserting the expected literal against the variable (e.g., assert True == healthy and assert False == healthy) so the assertion reads expected == actual and matches project test conventions.tests/unit/conftest.py (1)
112-115: Prefermocker.patchhere to match unit-test mocking conventions.Line 112 currently patches via
unittest.mock.patch(throughstart_patch). Intests/unit/**, this should usemocker.patch(...)for consistency with the project’s unit-test pattern.Suggested change
-@pytest.fixture(scope="module") -def event_gate_module(): +@pytest.fixture(scope="module") +def event_gate_module(mocker): @@ - mock_kafka_producer = start_patch("src.writers.writer_kafka.Producer") - mock_producer_instance = MagicMock() - mock_producer_instance.flush.return_value = 0 # 0 pending → flush loop breaks immediately - mock_kafka_producer.return_value = mock_producer_instance + mock_kafka_producer = mocker.patch("src.writers.writer_kafka.Producer") + mock_producer_instance = mock_kafka_producer.return_value + mock_producer_instance.flush.return_value = 0 # 0 pending → flush loop breaks immediatelyAs per coding guidelines:
tests/unit/**/*.py: Usemocker.patch("module.dependency")ormocker.patch.object(Class, "method")for mocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/conftest.py` around lines 112 - 115, Replace the use of start_patch("src.writers.writer_kafka.Producer") with the pytest mocker API: call mocker.patch("src.writers.writer_kafka.Producer") to create mock_kafka_producer, keep mock_producer_instance = MagicMock() and its flush.return_value = 0, and set mock_kafka_producer.return_value = mock_producer_instance so the Producer instantiation in tests returns the MagicMock; this aligns with the project convention of using mocker.patch rather than start_patch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/conftest.py`:
- Around line 112-115: Replace the use of
start_patch("src.writers.writer_kafka.Producer") with the pytest mocker API:
call mocker.patch("src.writers.writer_kafka.Producer") to create
mock_kafka_producer, keep mock_producer_instance = MagicMock() and its
flush.return_value = 0, and set mock_kafka_producer.return_value =
mock_producer_instance so the Producer instantiation in tests returns the
MagicMock; this aligns with the project convention of using mocker.patch rather
than start_patch.
In `@tests/unit/writers/test_writer_postgres.py`:
- Line 384: The test uses bare boolean assertions; change the two occurrences of
"assert healthy" and "assert not healthy" in
tests/unit/writers/test_writer_postgres.py to the repo's expected-first style by
asserting the expected literal against the variable (e.g., assert True ==
healthy and assert False == healthy) so the assertion reads expected == actual
and matches project test conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5963362-d8c2-4959-aaf7-8f6aab31b420
📒 Files selected for processing (4)
tests/unit/conftest.pytests/unit/handlers/test_handler_api.pytests/unit/handlers/test_handler_token.pytests/unit/writers/test_writer_postgres.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/handlers/test_handler_api.py
- tests/unit/handlers/test_handler_token.py
oto-macenauer-absa
approved these changes
Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This pull request introduces automated dependency management with Dependabot, raises code coverage standards, and significantly expands test coverage for critical modules. The main themes are infrastructure automation, improved code quality requirements, and enhanced unit testing for error handling and edge cases.
Release Notes
Related
Closes #114
Summary by CodeRabbit
Chores
Tests
Documentation