Skip to content

Add ONNX model format testcase in MLServer test-suite#1080

Merged
dbasunag merged 1 commit intoopendatahub-io:mainfrom
Snomaan6846:RHOAIENG-47134
Mar 10, 2026
Merged

Add ONNX model format testcase in MLServer test-suite#1080
dbasunag merged 1 commit intoopendatahub-io:mainfrom
Snomaan6846:RHOAIENG-47134

Conversation

@Snomaan6846
Copy link
Copy Markdown
Contributor

@Snomaan6846 Snomaan6846 commented Feb 4, 2026

Signed-off-by: Snomaan6846 syedali@redhat.com

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

Description

Added ONNX model format testcase in MLServer test-suite.

Jira-Link : https://issues.redhat.com/browse/RHOAIENG-47134

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
    • Added ONNX model format test coverage for MLServer basic model deployment scenarios, including REST and raw deployment configurations.
    • Expanded parameterized tests to include ONNX cases, added ONNX-specific test constants and a new snapshot capturing ONNX inference output for deterministic validation.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 4, 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', '/verified', '/cherry-pick', '/lgtm', '/wip'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 10, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The pull request adds ONNX model format support to the MLServer basic model deployment test suite by introducing a new ONNX snapshot JSON, adding an ONNX test parameter to the parameterized test, and defining ONNX-specific REST input constants and a MODEL_CONFIGS entry.

Changes

Cohort / File(s) Summary
Test Snapshot
tests/model_serving/model_runtime/mlserver/basic_model_deployment/__snapshots__/TestMLServerModels.test_mlserver_model_inference[onnx-raw-deployment].json
Adds a new snapshot JSON for ONNX raw deployment describing a model id onnx, version v1.0.0, one FP32 output predict with shape [1,1] and value -137.39060974121094.
Test Parameterization
tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py
Adds a pytest.param entry for ModelFormat.ONNX to the existing parameterized test cases (mirrors other model formats).
Test Constants / Configs
tests/model_serving/model_runtime/mlserver/constant.py
Introduces ONNX_REST_INPUT_QUERY constant and adds a ModelFormat.ONNX entry to MODEL_CONFIGS with model_version: "v1.0.0", rest_query: ONNX_REST_INPUT_QUERY, and output_type: OutputType.DETERMINISTIC.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. While it identifies the core change and provides a Jira link, it omits required template sections including Related Issues details, testing methodology specifics, and additional requirements checklist. Complete the PR description template by detailing testing approach under 'How it has been tested', explicitly addressing whether this introduces new test images or Jenkins markers, and ensuring all required checkboxes are properly evaluated.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely describes the main change: adding an ONNX model format testcase to the MLServer test suite.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

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.

🧹 Nitpick comments (1)
tests/model_serving/model_runtime/mlserver/constant.py (1)

80-90: Missing type annotation for consistency.

Other REST input query constants (SKLEARN_REST_INPUT_QUERY, XGBOOST_REST_INPUT_QUERY, LIGHTGBM_REST_INPUT_QUERY) have explicit dict[str, Any] type annotations. Add it here for consistency.

Proposed fix
-ONNX_REST_INPUT_QUERY = {
+ONNX_REST_INPUT_QUERY: dict[str, Any] = {
     "id": "onnx",
     "inputs": [
         {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/model_serving/model_runtime/mlserver/constant.py` around lines 80 - 90,
The constant ONNX_REST_INPUT_QUERY should have an explicit type annotation for
consistency with other constants; update its declaration to include the type
dict[str, Any] (i.e., annotate ONNX_REST_INPUT_QUERY: dict[str, Any]) while
keeping the existing value unchanged so it matches SKLEARN_REST_INPUT_QUERY,
XGBOOST_REST_INPUT_QUERY, and LIGHTGBM_REST_INPUT_QUERY.
🤖 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/model_serving/model_runtime/mlserver/constant.py`:
- Around line 80-90: The constant ONNX_REST_INPUT_QUERY should have an explicit
type annotation for consistency with other constants; update its declaration to
include the type dict[str, Any] (i.e., annotate ONNX_REST_INPUT_QUERY: dict[str,
Any]) while keeping the existing value unchanged so it matches
SKLEARN_REST_INPUT_QUERY, XGBOOST_REST_INPUT_QUERY, and
LIGHTGBM_REST_INPUT_QUERY.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: e24203e0-9042-4152-94d6-099567738e49

📥 Commits

Reviewing files that changed from the base of the PR and between 4628796 and cd10e3d.

📒 Files selected for processing (3)
  • tests/model_serving/model_runtime/mlserver/basic_model_deployment/__snapshots__/test_mlserver_basic_model_deployment/TestMLServerModels.test_mlserver_model_inference[onnx-raw-deployment].json
  • tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py
  • tests/model_serving/model_runtime/mlserver/constant.py

Copy link
Copy Markdown
Contributor

@brettmthompson brettmthompson left a comment

Choose a reason for hiding this comment

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

/lgtm

Signed-off-by: Snomaan6846 <syedali@redhat.com>

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@dbasunag dbasunag merged commit 17d4fd2 into opendatahub-io:main Mar 10, 2026
7 of 8 checks passed
@Snomaan6846 Snomaan6846 deleted the RHOAIENG-47134 branch March 10, 2026 15:46
@github-actions
Copy link
Copy Markdown

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