-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test/e2e: Preserve the existing environment when using exec.Command #76
test/e2e: Preserve the existing environment when using exec.Command #76
Conversation
Signed-off-by: timflannagan <[email protected]>
Waiting for CI feedback just in case. /hold |
@timflannagan: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
alright cool, looks like these changes still work in CI. /hold cancel |
@timflannagan Do we not care about the two failures that we're seeing for CI here? Changes look good but want to check first. |
Nope I think it's fine to merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, in that case this looks good.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: timflannagan, tylerslaton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
These changes only affect the internal testing suite. Adding the required labels. /label qe-approved |
[ART PR BUILD NOTIFIER] This PR has been included in build ose-cluster-platform-operators-manager-container-v4.12.0-202305022015.p0.gd40fae8.assembly.stream for distgit ose-cluster-platform-operators-manager. |
Ensure we're preserving the existing environment before running any exec.Commands. I noticed locally when running the e2e suite that the underlying bash script wasn't able to find 'oc' despite that executable being in my $PATH, and the
which oc
was outputting strange results when run in isolation. Explicitly specifying the existing environment viaos.Environ()
which should include the $PATH variable helped fix this pathing issue.Signed-off-by: timflannagan [email protected]