-
Notifications
You must be signed in to change notification settings - Fork 29
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 finding mamba on windows #135
Fix finding mamba on windows #135
Conversation
Good catch @ericpre - thanks a lot for that. To make it more robust, I propose to change a bit the following code: - which = "which"
+ cmd = ["which", "mamba"]
if sys.platform == "win32":
- which = "where"
+ cmd =["where", "mamba.exe"]
process = Popen(
- [which, "mamba"], stdout=PIPE, stderr=PIPE, encoding="utf-8"
+ cmd, stdout=PIPE, stderr=PIPE, encoding="utf-8"
) So we are sure to capture an executable on Windows and not the shell script. Regarding testing, the tests are stored in the Then in that file, the test functions are all function named def test_EnvManager_manager():
try:
import mamba
except ImportError:
expected = "conda"
else:
expected = "mamba"
manager = EnvManager("", None)
assert Path(manager.manager).stem == expected Let me known if you have questions. |
Thanks, your test is god and simple! My issue was mainly with getting a gator/mamba_gator/tests/test_api.py Lines 606 to 613 in 96a711f
|
This allows to quickly see if mamba was found
165cdae
to
095cd35
Compare
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.
Thanks @ericpre and sorry for the delay - your PR highlights that the CI was actually not testing Mamba has intended.
The following fails on windows:
with the error:
Because
output
is'C:\\Users\\Eric\\HyperSpy-bundle\\Scripts\\mamba.exe\nC:\\Users\\Eric\\HyperSpy-bundle\\condabin\\mamba.bat\n'
I have tried to add a test but I don't understand how the test suite works.