Skip to content

chore(trainer): add unit tests for platform runtime tools#97

Open
Solaris-star wants to merge 2 commits into
kubeflow:mainfrom
Solaris-star:chore/trainer-platform-unit-tests
Open

chore(trainer): add unit tests for platform runtime tools#97
Solaris-star wants to merge 2 commits into
kubeflow:mainfrom
Solaris-star:chore/trainer-platform-unit-tests

Conversation

@Solaris-star

Copy link
Copy Markdown
Contributor

Summary

Part of #68 — platform section.

Adds unit coverage for runtime CRUD tools with mocked CustomObjects API:

patch_runtime

  • missing patch / invalid top-level keys
  • preview when confirmed=False
  • success when confirmed
  • not-found + generic Kubernetes error

create_runtime

  • missing spec / invalid top-level keys
  • preview body construction
  • success when confirmed
  • generic error

delete_runtime

  • preview lists dependent TrainJobs
  • preview with no dependents
  • success when confirmed
  • not-found + generic error

Testing

uv run pytest -q tests/unit/trainer/test_platform.py
# 16 passed

Part of kubeflow#68 platform slice.

Cover patch_runtime/create_runtime/delete_runtime preview and confirmed
paths with mocked CustomObjects API, including validation and error wrapping.

Signed-off-by: Solaris-star <solaris@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 19, 2026 11:41
@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign electronic-waste for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

Copy link
Copy Markdown

🎉 Welcome to the Kubeflow MCP Server! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
  • Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds unit coverage for Trainer platform runtime CRUD tools using mocked Kubernetes APIs.

Changes:

  • Tests validation and preview behavior.
  • Tests confirmed mutations and Kubernetes errors.
  • Tests dependent TrainJob handling during deletion.

"kubeflow_mcp.trainer.api.platform.mcp_utils.get_custom_objects_api",
return_value=api,
):
result = patch_runtime("torch-tune", patch=patch_body, confirmed=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this. The shared Pattern C scaffold in #6 is not on main yet, so this PR cannot import it without duplicating that open PR. In 7e536dd I tightened the patch_runtime confirmed-path coverage to verify the complete Kubernetes API invocation, including group, version, plural, payload/name, and request timeout. Once #6 lands, I can rebase and convert this file to its shared fixtures.

"kubeflow_mcp.trainer.api.platform.mcp_utils.get_custom_objects_api",
return_value=api,
):
result = create_runtime("my-runtime", spec=spec, confirmed=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this. The shared Pattern C scaffold in #6 is not on main yet, so this PR cannot import it without duplicating that open PR. In 7e536dd I tightened the create_runtime confirmed-path coverage to verify the complete Kubernetes API invocation, including group, version, plural, payload/name, and request timeout. Once #6 lands, I can rebase and convert this file to its shared fixtures.

"kubeflow_mcp.trainer.api.platform.mcp_utils.get_custom_objects_api",
return_value=api,
):
result = delete_runtime("torch-tune", confirmed=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this. The shared Pattern C scaffold in #6 is not on main yet, so this PR cannot import it without duplicating that open PR. In 7e536dd I tightened the delete_runtime confirmed-path coverage to verify the complete Kubernetes API invocation, including group, version, plural, payload/name, and request timeout. Once #6 lands, I can rebase and convert this file to its shared fixtures.

@Solaris-star

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I agree the confirmed runtime paths should use the shared Pattern C utilities. The relevant mock_k8s_apis and verify_tool_* harness currently exists only in #6, which remains open and is not present on main. Copying those fixtures into this PR would duplicate the scaffold.

I will rebase after #6 lands, convert the confirmed create/patch/delete cases to the shared fixtures, and assert the complete Kubernetes SDK invocations before requesting maintainer review.

Signed-off-by: Solaris-star <820622658@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants