pipelines
pipelines copied to clipboard
test: Fix failing SDK test by replacing mock.patch with subprocess for actual build
Description of your changes: Resolves: https://github.com/kubeflow/pipelines/issues/11038
- Removed the mock.patch on subprocess.run.
- Added a direct call to subprocess.run to build the KFP package.
- Introduced error handling with a helpful error message if the package build fails.
- Kept the rest of the test logic intact.
The original mock.patch.object wasn’t working because it prevented the real build from happening, which was necessary for the test to pass. Replacing it with a direct call to subprocess.run ensures the KFP package is actually built, resolving the issue. This approach allows the test to reflect the real build environment and provides clearer error reporting if something goes wrong during the build process.
SDK test GHA passed on this PR: https://github.com/kubeflow/pipelines/actions/runs/11284347350/job/31385382544?pr=11285 Alternatively, you could test locally by running the following command from the repo's root directory:
pytest sdk/python/kfp --ignore=sdk/python/kfp/deprecated --cov=kfp
Checklist:
- [x] You have signed off your commits
- [x] The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.
[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 connor-mccarthy for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
ready for review. cc: @chensun @gregsheremeta @connor-mccarthy @hbelmiro
what was the mock.patch.object supposed to be doing, why wasn't it working, and how does calling a build via subprocess act as a valid replacement? (The commit message and PR should explain this, too.)
New changes are detected. LGTM label has been removed.
what was the
mock.patch.objectsupposed to be doing, why wasn't it working, and how does calling a build via subprocess act as a valid replacement? (The commit message and PR should explain this, too.)
Done, added some details on this in the commit message and the PR description.
/rerun-all