refactor(test): mutate test manifests structurally instead of using sed#896
Open
chw120 wants to merge 2 commits into
Open
refactor(test): mutate test manifests structurally instead of using sed#896chw120 wants to merge 2 commits into
chw120 wants to merge 2 commits into
Conversation
Address Barney's feedback in PR 755 by replacing fragile string replacements/sed mutations with structured structural YAML parsing and path-based updates. Specifically: - Add a robust Python utility dev/tools/mutate_yaml.py to parse and update template and router manifests structurally. - Modify run-test-kind.sh to use mutate_yaml.py on the fly without modifying the source files in-place. - Update test_e2e_python_sdk.py to use structured yaml.safe_load_all and path-based updates.
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: chw120 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 |
k8s-ci-robot
added
the
cncf-cla: yes
✅ Deploy Preview for agent-sandbox canceled.
|
k8s-ci-robot
added
the
size/L
There was a problem hiding this comment.
Pull request overview
This PR refactors test manifest mutation to use structured YAML parsing rather than string replacement, improving robustness for Python SDK E2E and kind gateway test setup.
Changes:
- Adds
dev/tools/mutate_yaml.pyfor path-based YAML manifest mutation. - Updates
run-test-kind.shto pipe mutated manifests directly tokubectl apply. - Updates Python E2E router deployment setup to mutate YAML documents structurally.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
dev/tools/mutate_yaml.py |
Adds a CLI helper for mutating SandboxTemplate and sandbox-router manifests. |
clients/python/agentic-sandbox-client/gateway-kind/run-test-kind.sh |
Replaces in-place sed mutations with the new YAML mutator during kind setup. |
test/e2e/clients/python/test_e2e_python_sdk.py |
Replaces router manifest string replacements with structured YAML updates. |
janetkuo
added
ready-for-review
and removed
action-required: resolve-copilot-comments
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
This PR addresses feedback from Barney in PR 755 by replacing fragile string replacements and
sedmutations with robust structural YAML parsing and path-based updates.Specifically, the following changes were made:
dev/tools/mutate_yaml.pyhas been introduced to parse and update template and router manifests structurally.run-test-kind.shnow usesmutate_yaml.pyto modify manifests on the fly without changing source files in-place.test_e2e_python_sdk.pyhas been updated to useyaml.safe_load_allfor structured path-based updates.Which issue(s) this PR is related to
Follow up on #755 (comment)
Release Note