Skip to content

test: add a new test for mcp named query with default server#1365

Merged
dbasunag merged 3 commits intoopendatahub-io:mainfrom
dbasunag:mcp_named_queries
Apr 7, 2026
Merged

test: add a new test for mcp named query with default server#1365
dbasunag merged 3 commits intoopendatahub-io:mainfrom
dbasunag:mcp_named_queries

Conversation

@dbasunag
Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag commented Apr 7, 2026

Pull Request

Summary

Related Issues

  • Fixes:
  • JIRA: RHOAIENG-56783

Please review and indicate how it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Tests
    • Added a new test for MCP server filter options that asserts no model-specific "named queries" are present; test is marked as expected-to-fail due to a known issue (RHOAIENG-56783).
    • Non-invasive: no other test logic or assertions were modified.

Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/hold', '/build-push-pr-image', '/cherry-pick', '/lgtm', '/verified', '/wip'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 10b6866d-622b-42a2-a20d-78b2011ecc9b

📥 Commits

Reviewing files that changed from the base of the PR and between 5806d37 and 4eeff76.

📒 Files selected for processing (1)
  • tests/model_registry/mcp_servers/config/test_named_queries.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/model_registry/mcp_servers/config/test_named_queries.py

📝 Walkthrough

Walkthrough

A new pytest class, TestMCPServerFilterOptionsNamedQueries, was added to tests/model_registry/mcp_servers/config/test_named_queries.py. It contains an xfail test that calls mcp_servers/filter_options, logs namedQueries (default {}) and asserts it is empty; xfail references RHOAIENG-56783.

Changes

Cohort / File(s) Summary
MCP Server Filter Options Test
tests/model_registry/mcp_servers/config/test_named_queries.py
Added TestMCPServerFilterOptionsNamedQueries with test_filter_options_no_named_queries() that calls mcp_servers/filter_options, reads/logs namedQueries, asserts it is absent/empty, and is marked xfail referencing RHOAIENG-56783.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a new test for MCP named queries with default server behavior.
Description check ✅ Passed The description follows the required template structure with Summary, Related Issues (JIRA ticket included), and testing verification. The Summary section is minimal but the core JIRA reference is present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/model_registry/mcp_servers/config/test_named_queries.py (1)

103-103: Docstring should use Given-When-Then format.

The test docstring is too terse and doesn't follow the Given-When-Then structure required by project standards.

Proposed fix
-        """Validate that MCP filter_options does not return any namedQueries."""
+        """Validate that MCP filter_options does not return any namedQueries.
+
+        Given: A default MCP server deployment with the filter_options endpoint available.
+        When: The mcp_servers/filter_options endpoint is queried.
+        Then: The response should not contain any namedQueries field or the field should be empty.
+        """

As per coding guidelines: "each test/class must have a docstring (Google format, Given-When-Then if feasible)" (CONSTITUTION) and "use Given-When-Then in the docstring" (AGENTS).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/model_registry/mcp_servers/config/test_named_queries.py` at line 103,
Replace the terse docstring """Validate that MCP filter_options does not return
any namedQueries.""" with a Given-When-Then formatted docstring for the test
that validates MCP filter_options behavior: state the Given (e.g., given an MCP
server or configuration without named queries), the When (when calling
filter_options on the MCP), and the Then (then assert that no namedQueries are
returned). Update the test function's docstring (the one referencing "MCP
filter_options" / "namedQueries") to follow that structure in Google style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/model_registry/mcp_servers/config/test_named_queries.py`:
- Around line 94-96: Add the required pytest markers to the
TestMCPServerFilterOptionsNamedQueries class so it follows project standards:
add the component marker (e.g., `@pytest.mark.model_registry`) and the appropriate
tier marker from pytest.ini (e.g., `@pytest.mark.unit` or `@pytest.mark.integration`
as applicable); you can mirror the style used by TestMCPServerNamedQueries
(which uses `@pytest.mark.usefixtures`("mcp_servers_configmap_patch")) but you do
not need to add that fixture if it's not required—just ensure the class
TestMCPServerFilterOptionsNamedQueries has the component and tier markers
applied.

---

Nitpick comments:
In `@tests/model_registry/mcp_servers/config/test_named_queries.py`:
- Line 103: Replace the terse docstring """Validate that MCP filter_options does
not return any namedQueries.""" with a Given-When-Then formatted docstring for
the test that validates MCP filter_options behavior: state the Given (e.g.,
given an MCP server or configuration without named queries), the When (when
calling filter_options on the MCP), and the Then (then assert that no
namedQueries are returned). Update the test function's docstring (the one
referencing "MCP filter_options" / "namedQueries") to follow that structure in
Google style.
🪄 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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: fcbad118-e282-42ea-8f4c-1a0529dedcde

📥 Commits

Reviewing files that changed from the base of the PR and between d434772 and 5806d37.

📒 Files selected for processing (1)
  • tests/model_registry/mcp_servers/config/test_named_queries.py

Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Copy Markdown
Contributor

@SB159 SB159 left a comment

Choose a reason for hiding this comment

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

/lgtm

@dbasunag dbasunag merged commit 2a8c802 into opendatahub-io:main Apr 7, 2026
10 checks passed
@dbasunag dbasunag deleted the mcp_named_queries branch April 7, 2026 15:37
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants