Fix xonsh - #4366
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4366 +/- ##
==========================================
+ Coverage 55.38% 55.40% +0.02%
==========================================
Files 243 243
Lines 30374 30383 +9
Branches 3257 3256 -1
==========================================
+ Hits 16822 16835 +13
+ Misses 13549 13545 -4
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # activate → manually delete var → deactivate | ||
| s = [ | ||
| f"{alias} activate test_unset_env", | ||
| "del $MAMBA_UNSET_TEST", | ||
| f"{alias} deactivate", | ||
| ] |
There was a problem hiding this comment.
Could we also try to use the -p flag to verify the support of this flag after the fix?
There was a problem hiding this comment.
Hmm not sure I get what you mean.
So shell activate was using -p in mamba.xsh while it's supposed to be a positional argument. The fix is to call that command properly in the script, and the test for that is to check there is no warning anymore:
assert "does not contain any filesystem separator" not in stderr
|
|
||
| prefix = tmp_root_prefix / "envs" / "test_unset_env" | ||
| state_file = prefix / "conda-meta" / "state" | ||
| state_file.write_text(helpers.json.dumps({"env_vars": {"MAMBA_UNSET_TEST": "hello"}})) |
There was a problem hiding this comment.
Also do you think that it is possible to test the case where MAMBA_UNSET_TEST:
- is set initially to some value (e.g. "hi")
- is reset by the activation of
test_unset_envto "hello" - is deleted under this environment
- is reset to the original value ("hi") after the environment is deactivated?
There was a problem hiding this comment.
I think that would be testing restoring environment variables behavior with activation and deactivation, and therefore is a more general test.
I suggest to rather open an issue to check the existence of such tests and add them if not.
|
I propose to first release 2.9.0 before merging this PR and other ones. |
2 participants
Description
Fix #4235
Type of Change
Checklist
pre-commit run --alllocally in the source folder and confirmed that there are no linter errors.