kustomize
kustomize copied to clipboard
Missing error case coverage in api/internal/target/kusttarget.go
Discovered during review of the remote loader's integration suite revamp.
We are going to lift the code freeze on the remote loader, but will require test coverage improvements along with or ahead of any changes to accumulateResources specifically going forward. This affects Kustomization loading for both remote and local targets.
Part of the problem here is that accumulateResources is making very uneducated guesses about why it got the two possible errors back from functions it called. @annasong20 points out that we could expose new error types in ifc.Loader and take advantage of those to make better decisions on which error to return if they both fail (as well as whether or not to even try the second load in the first place, although we'll need the enhanced coverage of current behaviour in place before we can improve it).
/assign @mightyguava
@KnVerey: GitHub didn't allow me to assign the following users: mightyguava.
Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide
In response to this:
Discovered during review of the remote loader's integration suite revamp.
We are going to lift the code freeze on the remote loader, but will require test coverage improvements along with or ahead of any changes to
accumulateResourcesspecifically going forward. This affects Kustomization loading for both remote and local targets.Part of the problem here is that accumulateResources is making very uneducated guesses about why it got the two possible errors back from functions it called. @annasong20 points out that we could expose new error types in
ifc.Loaderand take advantage of those to make better decisions on which error to return if they both fail (as well as whether or not to even try the second load in the first place, although we'll need the enhanced coverage of current behaviour in place before we can improve it)./assign @mightyguava
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
lol
/assign @mightyguava
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
Hi @mightyguava, are you still working on this? If not, can I assign it to myself? It is currently blocking #4348.
/assign
@annasong20 can we close this now that your PR is merged? Or is there more work to do here?
@natasha41575 Still need to add test cases for remote repos, local files, and local directories. Will add a comment in the last test PR that automatically closes this issue.
/remove-lifecycle rotten
/unassign I haven't had time to work on this in the past few months. Anyone should feel free to pick it up!
/assign might have bandwidth to work on this in the next week or so