feat: add test infrastructure and initial test coverage#71
Merged
evaline-ju merged 4 commits intomainfrom Mar 19, 2026
Merged
Conversation
Add shared conftest.py fixtures and tests for three previously uncovered critical paths in src/server.py: - tests/__init__.py: package marker - tests/conftest.py: shared mock_envoy_modules and mock_manager fixtures, eliminating duplication across test modules - tests/test_tool_pre_invoke.py: 4 tests for getToolPreInvokeResponse (continue, modified args, blocked with violation, payload naming) - tests/test_prompt_pre_fetch.py: 3 tests for getPromptPreFetchResponse (continue, modified args, blocked) - tests/test_helpers.py: 6 tests for helper functions (set_result_in_body, get_modified_response, create_mcp_immediate_error_response with/without violation and mcp_error_code) All 22 tests (13 new + 9 existing) pass. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
evaline-ju
reviewed
Mar 13, 2026
Contributor
evaline-ju
left a comment
evaline-ju
left a comment
There was a problem hiding this comment.
Thanks for more tests, @pdettori ! I see that the conftest may have overlap with existing server tests https://github.com/kagenti/plugins-adapter/blob/main/tests/test_server.py . Perhaps we can help refactor usage with the conftest instead of duplication? It might be worth organizing the existing tests a bit for navigability since already existent "tool_post_invoke" tests aren't really discoverable w.r.t. these other "hook" (prompt_pre_fetch and tool_pre_invoke) new test files
…lity Remove unused `import pytest` from test_helpers.py (ruff F401 fix). Rename test_server.py → test_tool_post_invoke.py so all hook tests share a consistent naming convention (test_tool_pre_invoke, test_tool_post_invoke, test_prompt_pre_fetch). Remove the three locally-duplicated fixtures (mock_envoy_modules, mock_manager, sample_tool_result_body) from the renamed file and promote sample_tool_result_body to conftest.py, where it is now shared across all post-invoke test cases. No test logic changed — all 22 tests still pass. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
pdettori
commented
Mar 13, 2026
Collaborator
Author
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Thanks for the feedback @evaline-ju!
Both issues are addressed in the latest commit:
- CI lint failure: removed the unused
import pytestfromtest_helpers.py(ruff F401) - Fixture duplication / navigability: renamed
test_server.py→test_tool_post_invoke.pyso all hook tests now follow a consistent naming convention alongsidetest_tool_pre_invoke.pyandtest_prompt_pre_fetch.py. The three locally-duplicated fixtures (mock_envoy_modules,mock_manager,sample_tool_result_body) have been removed from that file andsample_tool_result_bodypromoted toconftest.pyfor shared use.
All 22 tests pass.
pdettori
commented
Mar 13, 2026
Collaborator
Author
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Thanks for the feedback @evaline-ju!
Both issues are addressed in the latest commit:
- CI lint failure: removed the unused
import pytestfromtest_helpers.py(ruff F401) - Fixture duplication / navigability: renamed
test_server.py→test_tool_post_invoke.pyso all hook tests now follow a consistent naming convention alongsidetest_tool_pre_invoke.pyandtest_prompt_pre_fetch.py. The three locally-duplicated fixtures (mock_envoy_modules,mock_manager,sample_tool_result_body) have been removed from that file andsample_tool_result_bodypromoted toconftest.pyfor shared use.
All 22 tests pass.
evaline-ju
reviewed
Mar 16, 2026
Contributor
evaline-ju
left a comment
evaline-ju
left a comment
There was a problem hiding this comment.
Thanks for the refactor update! A couple more minor comments for consideration
…hen assertion - Move _make_result helper to conftest.py as make_hook_result, removing duplication between test_tool_pre_invoke.py and test_prompt_pre_fetch.py - Strengthen test_get_modified_response to verify serialized JSON body content instead of only checking response is not None Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Remove unused json import from conftest.py, fix import sort order in test_tool_pre_invoke.py and test_prompt_pre_fetch.py. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
evaline-ju
approved these changes
Mar 19, 2026
Contributor
evaline-ju
left a comment
evaline-ju
left a comment
There was a problem hiding this comment.
Thanks for the updates!
/lgtm
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.
Summary
Phase 3 of repo orchestration — adds test infrastructure and coverage for previously untested critical paths.
New files:
tests/__init__.py: package markertests/conftest.py: sharedmock_envoy_modulesandmock_managerfixtures, eliminating duplicationtests/test_tool_pre_invoke.py: 4 tests forgetToolPreInvokeResponse(previously 0 coverage)tests/test_prompt_pre_fetch.py: 3 tests forgetPromptPreFetchResponse(previously 0 coverage)tests/test_helpers.py: 6 tests for helper functions (previously 0 coverage)set_result_in_body: mutates and replaces argumentsget_modified_response: returns BodyResponsecreate_mcp_immediate_error_response: default code, violation reason, mcp_error_code overrideTest plan
test_server.pystill pass (no regressions)