chore(trainer): add unit tests for discovery tools#82
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
🎉 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:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
Signed-off-by: Aastha Mahajan <amahajan1_be23@thapar.edu>
ad8efd4 to
a1e885d
Compare
There was a problem hiding this comment.
Pull request overview
Adds discovery-tool unit coverage for issue #68.
Changes:
- Tests job/runtime discovery, filtering, errors, and empty results.
- Mocks package lookup and namespace policy enforcement.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_list_training_jobs_caps_limit_at_max(): | ||
| client = MagicMock() | ||
| client.list_jobs.return_value = [_fake_job(f"job-{i}") for i in range(5)] | ||
| with patch( | ||
| "kubeflow_mcp.trainer.api.discovery.get_trainer_client_for_namespace", | ||
| return_value=client, | ||
| ): | ||
| result = list_training_jobs(limit=10_000) | ||
|
|
||
| # All 5 fake jobs returned; the cap only affects slicing, not a crash, | ||
| # so this mainly documents that an oversized limit doesn't error. | ||
| assert result["success"] is True | ||
| assert result["data"]["total"] == 5 |
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_get_training_job_success(): |
|
Waiting for test scaffold to merge first : |
aastha-m22
changed the title
Part of #68 — discovery section.
Covers list_training_jobs, get_training_job, list_runtimes, get_runtime:
Real cluster calls and packages-via-pod polling are out of scope per the issue;
_fetch_packages_via_pod is mocked directly in the one test that exercises
include_packages=True.
Ready for review. I haven't heard back on the issue thread yet, so happy to adjust scope if this overlaps with anything already in progress.
Testing
pytest tests/unit/trainer/test_discovery.py -v → 23 passed
ruff check / ruff format --check → clean