Skip to content

HF: Add more negative tests#997

Merged
dbasunag merged 2 commits intoopendatahub-io:mainfrom
dbasunag:hf_negative
Jan 9, 2026
Merged

HF: Add more negative tests#997
dbasunag merged 2 commits intoopendatahub-io:mainfrom
dbasunag:hf_negative

Conversation

@dbasunag
Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag commented Jan 8, 2026

Description

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • Tests
    • Expanded negative tests for model catalog configuration validation to cover additional invalid wildcard scenarios and organization-specific wildcard rejection.
    • Updated test cases and expected error messages to reflect stricter validation for unsupported patterns (e.g., lone or mixed wildcards and organization-containing patterns).
    • Adjusted test parametrization and removed prior conditional expectation markers; no runtime or public API changes.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 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', '/wip', '/lgtm', '/verified'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

Expanded negative tests for HuggingFace catalog source configurations by adding multiple invalid wildcard scenarios and updating expected error messages; changes are limited to test parametrization and assertions (no runtime logic modifications).

Changes

Cohort / File(s) Summary
Test parameterization for HuggingFace pattern validation
tests/model_registry/model_catalog/catalog_config/test_model_catalog_negative.py
Replaced ibm-granite/* with abc-random*, removed an xfail and renamed a test case to focus on invalid organization, and added new invalid wildcard cases: *, */*, RedHatAI/, *RedHatAI*. Updated expected error messages to: "failed to expand model patterns: wildcard pattern is not supported - Hugging Face requires a specific organization".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'HF: Add more negative tests' accurately describes the main change: expanding negative test coverage for HuggingFace catalog configurations with additional invalid wildcard scenarios.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac13ac6 and 09cf931.

📒 Files selected for processing (1)
  • tests/model_registry/model_catalog/catalog_config/test_model_catalog_negative.py
🔇 Additional comments (4)
tests/model_registry/model_catalog/catalog_config/test_model_catalog_negative.py (4)

78-88: LGTM! Clear error message improvement.

The updated error message clearly communicates the requirement for HuggingFace catalogs.


89-102: LGTM! Good test case refinement.

The pattern change from ibm-granite/* to abc-random* and the corresponding test ID rename better reflect the intent of testing invalid organization wildcards. The updated error message is clear and consistent with the other new test cases.


162-179: Test method structure is sound.

The test logic correctly validates that invalid catalog configurations result in error states with appropriate error messages. The use of substring matching (in operator on line 177) is appropriate for verifying error content.


103-158: Verify error messages against model-registry application behavior.

The new test cases provide comprehensive coverage of various invalid wildcard patterns (*, */*, RedHatAI/, *RedHatAI*). The test structure and integration methodology are sound—the tests query the model-registry REST API to verify that invalid patterns produce the expected error messages.

However, the error messages must be verified against the actual model-registry application output. These errors are generated by the model-registry service, not defined in this test repository. Run the tests against the actual model-registry application to confirm all expected error messages match the implementation, particularly:

  • Whether all four wildcard scenarios (lines 103–158) produce the exact error messages specified
  • Whether the error message for RedHatAI/ accurately describes the validation failure (this pattern has an organization but lacks a model name, so the message about "requiring a specific organization" may need clarification)

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.

❤️ Share

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

@dbasunag dbasunag enabled auto-merge (squash) January 9, 2026 13:18
Copy link
Copy Markdown
Contributor

@sheltoncyril sheltoncyril 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

@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

@dbasunag dbasunag merged commit 4592fa9 into opendatahub-io:main Jan 9, 2026
9 checks passed
@dbasunag dbasunag deleted the hf_negative branch January 9, 2026 14:31
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 9, 2026

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

mwaykole pushed a commit to mwaykole/opendatahub-tests that referenced this pull request Jan 23, 2026
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