Make tests use a temporary directory for conda environments #549
Conversation
Previously they used the current conda prefix, but this is dangerous (there's a risk a broken test could break the dev environment), and the current conda prefix could contain things that break the tests. This currently works by parameterizing in the fixture itself (currently just for two basic envs, one without pip packages and one with). But in the future we can do this parameterization on a per-test level with indirect parameterization. See https://docs.pytest.org/en/latest/example/parametrize.html#indirect-parametrization Currently, the test_generate_conda_export() test fails here. The reason is that it creates an export which cannot actually be read by the CondaSpecification object. The reason for this is that the "name" of the environment that gets generated is just the same as the "prefix". This fails a validation check, because the name is guarded by a regular expression that disallows paths. However, this appears to be a bug in conda-store, or maybe even an architectural issue. The "name" of a conda environment, as defined by "conda env export" at least, is only really defined if that environment is in the envs directory. I checked, and even env export yamls generated by conda-store itself (in standalone mode) end up having the prefix for the name, rather than the supposed actual environment name. At best, it means that this test is just wrong and it shouldn't actually check the generated YAML file by trying to load it into a CondaSpecification. I don't know if conda actually stores the environment "name" anywhere in the environment. It seems to me that this data should be stored separately by conda-store, especially since the environment path for environments created by conda-store is not actually the same as the environment name (the path contains a hash).
This is built on conda-incubator#549 but I've made in a separate PR because I'm not sure if there will be other issues here and I don't want to block that PR on this (but at the same time, tests won't pass on Mac without the changes from that PR). See conda-incubator#513 and conda-incubator#507
There was a problem hiding this comment.
I don't know if conda actually stores the environment "name" anywhere in the environment. It seems to me that this data should be stored separately by conda-store, especially since the environment path for environments created by conda-store is not actually the same as the environment name (the path contains a hash).
Yeah like you said conda doesn't store the name anywhere.
It takes the envs_dir director and any folder in there is assumed to be the name. It is possible for there to be collisions in the environment names.
I chose that the CondaSpecification does not store the name since I felt this was redundant (already stored in the db) and did not influence the solve. I agree that we may want to revisit this later on how we store the actual environment specification.
I'm fine with this PR as is
| @pytest.fixture | ||
| def current_prefix(): | ||
| return pathlib.Path(os.environ["CONDA_PREFIX"]) | ||
| @pytest.fixture( |
There was a problem hiding this comment.
Love it. This is a great way I think to be able to test more complete environment in the future.
|
Just to be clear, |
@asmeurer I think you are completely right.
Yeah I'd go this route just so that we can be sure that export is still generating valid environment yamls. |
|
OK, I've made that change. |
The environment name produced by conda env export will not be the actual "name" of the environment unless that environment is inside of a conda envs directory. Otherwise it is just the prefix, which fails the CondaSpecification validation for the name. We work around this by just hotfixing the name in the test before parsing the result.
This is built on conda-incubator#549 but I've made in a separate PR because I'm not sure if there will be other issues here and I don't want to block that PR on this (but at the same time, tests won't pass on Mac without the changes from that PR). See conda-incubator#513 and conda-incubator#507
* Add a macOS worker to the CI This is built on #549 but I've made in a separate PR because I'm not sure if there will be other issues here and I don't want to block that PR on this (but at the same time, tests won't pass on Mac without the changes from that PR). See #513 and #507 * Trigger build * Try not using mamba to fix macos CI * Add a separate macos environment file without conda-docker * Use the macos environment file for macos on CI * Try using mamba on macos again * Revert "Try using mamba on macos again" This reverts commit 031574a. * Install mamba in the dev environment * Go back to using mamba but with the correct syntax this time Revert "Revert "Try using mamba on macos again"" This reverts commit 6cf2877. * Fix CI Revert "Go back to using mamba but with the correct syntax this time" This reverts commit b82c0c8. * Only test docker on Linux
Previously they used the current conda prefix, but this is dangerous (there's a risk a broken test could break the dev environment), and the current conda prefix could contain things that break the tests.
This currently works by parameterizing in the fixture itself (currently just for two basic envs, one without pip packages and one with). But in the future we can do this parameterization on a per-test level with indirect parameterization. See
https://docs.pytest.org/en/latest/example/parametrize.html#indirect-parametrization
Currently, the test_generate_conda_export() test fails here. The reason is that it creates an export which cannot actually be read by the CondaSpecification object.
The reason for this is that the "name" of the environment that gets generated is just the same as the "prefix". This fails a validation check, because the name is guarded by a regular expression that disallows paths.
However, this appears to be a bug in conda-store, or maybe even an architectural issue. The "name" of a conda environment, as defined by "conda env export" at least, is only really defined if that environment is in the envs directory. I checked, and even env export yamls generated by conda-store itself (in standalone mode) end up having the prefix for the name, rather than the supposed actual environment name.
At best, it means that this test is just wrong and it shouldn't actually check the generated YAML file by trying to load it into a CondaSpecification.
I don't know if conda actually stores the environment "name" anywhere in the environment. It seems to me that this data should be stored separately by conda-store, especially since the environment path for environments created by conda-store is not actually the same as the environment name (the path contains a hash).
This is related to issue #513.
Description
This pull request:
Pull request checklist
Additional information