Test frozen #7
Test frozen #7
Conversation
| extra_files: | ||
| - frozen.json: conda-meta/frozen | ||
| - frozen.json: envs/default/conda-meta/frozen | ||
| - frozen.json: envs/env2/conda-meta/frozen |
There was a problem hiding this comment.
This environment doesn't exist in your installer. It should also have a linefeed at the end of the file.
tests/test_examples.py
Outdated
| install_dir / "conda-meta" / "frozen", | ||
| install_dir / "envs" / "default" / "conda-meta" / "frozen", | ||
| "freeze_base": install_dir / "conda-meta" / "frozen", | ||
| "freeze_env": install_dir / "envs" / "env1" / "conda-meta" / "frozen", |
There was a problem hiding this comment.
This removes the extra_files method from the test.
tests/test_examples.py
Outdated
|
|
||
| assert expected_frozen_paths == actual_frozen_paths, ( | ||
| f"Expected: {sorted(str(p) for p in expected_frozen_paths)}\n" | ||
| assert set(expected_frozen_paths.values()) == actual_frozen_paths, ( |
There was a problem hiding this comment.
You don't need to handcraft your message here. assert set(...) == set(...) already has a very nice diff that will be easier to read in the log than comparing "Expected" and "Found" (where comparisons will have to be done "by hand").
tests/test_examples.py
Outdated
| ) | ||
|
|
||
| for method, path in expected_frozen_paths.items(): | ||
| actual_config = json.load(open(path)) |
There was a problem hiding this comment.
| actual_config = json.load(open(path)) | |
| actual_config = json.loads(path.read_text()) |
open outside of a context manager doesn't close the file handle until garbage collection. File handles should be closed when files are no longer needed to avoid resource leaks.
There was a problem hiding this comment.
You may also want to use a different variable name than path because it is too similar to the Path object.
| - conda | ||
| freeze_env: | ||
| conda: | ||
| "message": "This environment is frozen." |
There was a problem hiding this comment.
| "message": "This environment is frozen." | |
| message: This environment is frozen. |
| conda: | ||
| "message": "This environment is frozen." | ||
|
|
||
| extra_files: |
There was a problem hiding this comment.
Currently, your construct.yaml file is the one that's broken. It would be much nicer if you committed the one that's working to the repository and then add this later for the negative test case.
tests/test_examples.py
Outdated
| shutil.copytree(str(example_path), str(input_path)) | ||
|
|
||
| with open(input_path / "construct.yaml") as f: | ||
| modified_config = YAML().load(f) | ||
| modified_config.pop("extra_files", None) | ||
| with open(input_path / "construct.yaml", "w") as f: | ||
| YAML().dump(modified_config, f) |
There was a problem hiding this comment.
I would put this before the context manager. That way, the code inside the context manager is simpler and you don't have to execute two different branches.
Right now, there is no reason for you to even have the same context manager because you're doing different things any. So, the pseudocode would be:
context = pytest.raises() if has_conflicts else nullcontext()
shutil.copytree(example_path, input_path)
read config
if has_conflicts:
modify config
with context:
You don't need the as c if you're not using it.
Confirmed having both
freeze_baseand creating a frozen marker file in thebaseenv viaextra_filesgeneratesRuntimeError: Frozen marker files found from both 'freeze_base / freeze_env' and 'extra_files' for the following environments: base. See here