-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix CMD.exe activation test #13159
Fix CMD.exe activation test #13159
Conversation
self.expect(".*\n") | ||
else: | ||
self.expect("%s\n" % value) | ||
self.expect(f"{value}\r?\n") |
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.
Auto-handle \r\n
and \n
.
prefix, charizard, _ = shell_wrapper_integration | ||
conda_bat = Path(CONDA_PACKAGE_ROOT, "shell", "condabin", "conda.bat") | ||
monkeypatch.setenv("PATH", STRIPPED_PATH) |
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.
monkeypatch.setenv
instead of env_vars
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.
Don't we use conda env_vars because it resets the context? Will that be a problem compared to monkeypatch?
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.
I've been trying to move away from env_var(s)?
in general
In this case resetting the context has no impact since the context doesn't do anything with PATH
Description
Looks like git moved on the GitHub runner (?) (or perhaps it changed on conda-forge?) so the CMD.exe test started to fail. Fixed the issue by being more careful with how we "reset" the
PATH
for the CMD.exe test.Merging this to the 23.9.x release branch since we need this fix in order to merge #13155 (we will also need this fix for any 23.9.x patches and the upcoming 23.10.x special release).
Checklist - did you ...
Add a file to thenews
directory (using the template) for the next release's release notes?Add / update necessary tests?Add / update outdated documentation?