Skip to content

test: add verification for filtering mcp server by source label#1285

Merged
dbasunag merged 2 commits intoopendatahub-io:mainfrom
fege:source_label_mcp
Mar 24, 2026
Merged

test: add verification for filtering mcp server by source label#1285
dbasunag merged 2 commits intoopendatahub-io:mainfrom
fege:source_label_mcp

Conversation

@fege
Copy link
Copy Markdown
Contributor

@fege fege commented Mar 24, 2026

Pull Request

Summary

Related Issues

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 comprehensive tests for source label filtering of MCP servers
  • Added validation for unlabeled/null source handling
  • Updated model search tests with stricter filtering assertions

@github-actions
Copy link
Copy Markdown

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

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

Pull request introduces testing infrastructure for MCP server filtering by source labels, including an unlabeled source variant. Changes add a new test fixture that patches the MCP catalog ConfigMap with three server YAML configurations, a new test module validating source label filtering behavior, supporting constants, and updates to existing model search tests to validate null/unlabeled source accounting.

Changes

Cohort / File(s) Summary
Test Fixtures & Configuration
tests/model_registry/mcp_servers/config/conftest.py
Added mcp_source_label_configmap_patch fixture (class-scoped) that patches ConfigMap with three MCP server YAML entries and three catalog source definitions (two labeled, one unlabeled). Follows existing fixture control flow pattern: resource patch → pod wait → API availability check → yield → teardown wait.
Source Label Test Module
tests/model_registry/mcp_servers/config/test_source_label.py
Added TestMCPServerSourceLabel test class with two test methods: test_mcp_server_source_label validates filtering by individual/combined source labels and asserts size accounting (unfiltered = labeled sum + null), and test_mcp_server_invalid_source_label (tier3) verifies invalid labels return size 0.
MCP Source Constants
tests/model_registry/mcp_servers/constants.py
Added third unlabeled MCP catalog source definition: MCP_CATALOG_SOURCE3_* constants plus YAML catalog path/content. Added computed sets EXPECTED_MCP_SOURCE3_SERVER_NAMES and EXPECTED_ALL_MCP_SERVER_NAMES_WITH_UNLABELED.
Model Search Test Updates
tests/model_registry/model_catalog/search/test_model_search.py
Renamed variables for clarity (redhat\_ai\_filter\_moldels\_size → redhat\_ai\_filter\_models\_size), added null label size assertions, changed assertion logic to explicitly validate size accounting: labeled sum must equal combined labeled, and unfiltered must equal labeled sum + null size.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description lacks a Summary section explaining the changes and their necessity; only template placeholders remain. The JIRA reference is provided but implementation details are absent. Complete the Summary section with concrete details about what was added (new test fixtures, test cases, constants) and why source label filtering verification is needed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately and concisely describes the primary changeset: adding verification tests for MCP server filtering by source label.

✏️ 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: 2

🤖 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/conftest.py`:
- Around line 83-86: The fixture currently blindly extends
current_data["mcp_catalogs"] with MCP_CATALOG_SOURCE, MCP_CATALOG_SOURCE2,
MCP_CATALOG_SOURCE3 which can create duplicates; update the logic around
current_data["mcp_catalogs"] to first ensure it's a list, compute the existing
set (from current_data["mcp_catalogs"]), and only append/extend those
MCP_CATALOG_* values that are not already present (e.g., filter the list
[MCP_CATALOG_SOURCE, MCP_CATALOG_SOURCE2, MCP_CATALOG_SOURCE3] by membership in
the existing set before extending) so duplicates are avoided.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: a8858fba-33d8-498a-a271-76abc8f6544c

📥 Commits

Reviewing files that changed from the base of the PR and between f6eb75b and 5230d95.

📒 Files selected for processing (4)
  • tests/model_registry/mcp_servers/config/conftest.py
  • tests/model_registry/mcp_servers/config/test_source_label.py
  • tests/model_registry/mcp_servers/constants.py
  • tests/model_registry/model_catalog/search/test_model_search.py

@dbasunag dbasunag merged commit de2f59c into opendatahub-io:main Mar 24, 2026
11 checks passed
@github-actions
Copy link
Copy Markdown

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

@fege fege deleted the source_label_mcp branch March 24, 2026 16:34
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.

3 participants