Introducing `conda_cli`, `tmp_env`, and `path_factory` fixtures #12592

kenodegard · 2023-04-13T20:59:26Z

Description

We have two different run subcommand implementations that effectively do the same thing. Each adds unnecessary complexities that we are better off avoiding all together.

from conda.testing.integration import run_command, Commands
from pathlib import Path

def test_info_envs():
    out, err, code = run_commands(Commands.INFO, None, "--envs")

def test_create(tmp_path: Path):
    out, err, code = run_commands(Commands.CREATE, tmp_path, "--yes")

vs.

from conda.testing.helpers import run_inprocess_conda_command
from pathlib import Path

def test_info_envs():
    out, err, code = run_inprocess_conda_command("conda info --envs")

def test_create(tmp_path: Path):
    out, err, code = run_inprocess_conda_command(f"conda create --prefix {tmp_path}" --yes")

Here we propose rewriting these into a pytest fixture:

from conda.testing.integration import CondaCLIFixture
from pathlib import Path

def test_info_envs(conda_cli: CondaCLIFixture):
    out, err, code = conda_cli("info", "--envs")

def test_create(conda_cli: CondaCLIFixture, tmp_path: Path):
    out, err, code = conda_cli("create", "--prefix", tmp_path, "--yes")

In addition to the new conda_cli fixture we also implemented:

tmp_env fixture to replace conda.testing.integration.make_temp_env
path_factory fixture which does tmp_path / uuid.uuid4().hex for us

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

dholth · 2023-04-14T13:49:37Z

conda/cli/main_remove.py

            )
        if not isfile(join(prefix, "conda-meta", "history")):
            raise DirectoryNotACondaEnvironmentError(prefix)
-        print("\nRemove all packages in environment %s:\n" % prefix, file=sys.stderr)
+        print(f"\nRemove all packages in environment {prefix}:\n")


changes stderr to stdout

correct, IMO the text printed does not belong in stderr:

$ conda create -n scratch python --yes ... $ conda remove -n scratch --all --yes > /dev/null Remove all packages in environment /Users/kodegard/.conda/arm64/23.1.0/3.10/envs/scratch:

$ conda create -n scratch python --yes ... $ conda remove -n scratch --all 2> /dev/null ## Package Plan ## environment location: /Users/kodegard/.conda/arm64/23.1.0/3.10/envs/scratch The following packages will be REMOVED: bzip2-1.0.8-h620ffc9_4 ca-certificates-2023.01.10-hca03da5_0 certifi-2022.12.7-py311hca03da5_0 libffi-3.4.2-hca03da5_6 ncurses-6.4-h313beb8_0 openssl-1.1.1t-h1a28f6b_0 pip-23.0.1-py311hca03da5_0 python-3.11.2-hc0d8a6c_0 readline-8.2-h1a28f6b_0 setuptools-65.6.3-py311hca03da5_0 sqlite-3.41.2-h80987f9_0 tk-8.6.12-hb8d0fd4_0 tzdata-2023c-h04d1e81_0 wheel-0.38.4-py311hca03da5_0 xz-5.2.10-h80987f9_1 zlib-1.2.13-h5a0b063_0 Preparing transaction: done Verifying transaction: done Executing transaction: done

yeah that seemed to be a bug

conda/testing/__init__.py

dholth · 2023-04-14T13:51:49Z

tests/test_create.py

@@ -1361,7 +1361,7 @@ def test_conda_run_1(self):
            use_restricted_unicode=False, name=str(uuid4())[:7]
        ) as prefix:
            output, error, rc = run_command(Commands.RUN, prefix, "echo", "hello")
-            assert output == "hello" + os.linesep
+            assert output == f"hello{os.linesep}\n"


If os.linesep is \n are we looking for \n\n now

not entirely sure why this change was necessary but the change was test driven:

Windows failure

Linux failure

Question: is the bash shell introducing the \n

conda run likes to spit out an extra trailing newline, you can test this locally:

$ conda run -n base python --version Python 3.10.8 $ conda run -n base echo hello hello

dholth · 2023-04-14T13:52:38Z

conda/cli/main_info.py

-        stdout_logger = getLogger("conda.stdoutlog")
-        stdout_logger.info(get_main_info_str(info_dict))
-        stdout_logger.info("\n")
+        print(get_main_info_str(info_dict) + "\n")


Interesting way to send messages presumably to stdout with stdout_logger instead of print. I wonder if that flexibility was ever used.

I'm curious about this change, the conda.stdoutlog logger behaves the same as print()?

It's effectively the same for the CLI.

I had to change it because the logging was resulting in the output getting collected differently so the new conda_cli didn't work correctly for run commands.

Per my understanding, an effort was previously undertaken to replace all print (to both stdout and stderr) with this kind of logging logic. My best guess is this was to potentially allow third-party tools (like navigator) to better redirect/interface with the text streams. This effort was never completed and the logging system has become over complicated as a result. (I'll ping the navigator team to check that this won't cause issues)

sven6002 · 2023-04-14T15:35:19Z

tests/conftest.py

@@ -5,6 +5,8 @@

 import pytest

+from conda.testing import conda_cli, path_factory, tmp_env
+


should this file should be under changes committed?

Importing these fixtures here makes these fixtures available to all tests.

As for this extra newline, isort + black handles this formatting, if we remove it, it'll be re-added.

sven6002 · 2023-04-14T15:37:22Z

tests/cli/test_subcommands.py

+            "ca-certificates",
+            "--yes",
+        )
+        assert out


assert out is not None I think is a better usage of assertion in this section.

conda_cli returns tuple[str, str, int], out will always be not None so that would be a redundant check

what I'm asserting here is whether anything was printed to stdout so an empty string is considered a failure

travishathaway · 2023-04-14T15:55:48Z

conda/testing/__init__.py

@@ -156,3 +165,94 @@ def conda_check_versions_aligned():
        )
        with open(version_file, "w") as fh:
            fh.write(version_from_git)
+
+
+@dataclass


Why is this a dataclass?

Originally I had:

class CondaCLIFixture: def __init__(self, capsys: CaptureFixture): self.capsys = capsys

Which is what a dataclass does 🤷🏼‍♂️

jezdez

Some questions, but generally this is a big step forward

jezdez · 2023-04-27T07:58:09Z

conda/cli/main_run.py

@@ -55,4 +55,4 @@ def execute(args, parser):
        log = getLogger(__name__)
        log.warning(f"CONDA_TEST_SAVE_TEMPS :: retaining main_run script {script}")

-    return response
+    return response.rc


That is odd, wasn't the return code returned before? Might make things tricky for VS Code etc.

Apparently no, we've been returning the entire Response object (which happens to be a custom NamedTuple?)

This should only impact downstream projects if they invoke conda run via Python (I'll ping the VSCode community to confirm)

This change ensures that all of the execute functions defined for our subcommands consistently return an int exit code (a good change IMO)

jezdez · 2023-04-27T08:03:03Z

conda/cli/main_info.py

-        stdout_logger = getLogger("conda.stdoutlog")
-        stdout_logger.info(get_main_info_str(info_dict))
-        stdout_logger.info("\n")
+        print(get_main_info_str(info_dict) + "\n")


I'm curious about this change, the conda.stdoutlog logger behaves the same as print()?

travishathaway

@kenodegard,

Looking good so far. Thanks for the very clear examples on the description of the pull request. That made this a lot easier to review.

Before we merge this, could we please update this article in the documentation?

https://docs.conda.io/projects/conda/en/stable/dev-guide/writing-tests/integration-tests.html

Shouldn't be too difficult.

documentation has been updated as requested

Combine `conda.testing.integration.run_commands` and `conda.testing.helpers.run_inprocess_conda_command` into a new `conda_cli` fixture. Define new `tmp_env` fixture to replace `conda.testing.integration.make_temp_env`. Define new `path_factory` fixture to simplify doing `tmp_path / uuid.uuid4().hex`.

kenodegard added 2 commits April 13, 2023 14:14

Implement path_factory, run, and tmp_env

3abfcc3

Add simple tests for all subcommands

5a37e5e

kenodegard requested a review from travishathaway April 13, 2023 20:59

kenodegard self-assigned this Apr 13, 2023

kenodegard requested a review from a team as a code owner April 13, 2023 20:59

kenodegard requested a review from sven6002 April 13, 2023 20:59

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 13, 2023

kenodegard force-pushed the combine-runs branch from 93a80b8 to c4929bd Compare April 13, 2023 21:05

Rename run fixture to conda_cli

aa01917

kenodegard force-pushed the combine-runs branch from c4929bd to aa01917 Compare April 13, 2023 21:07

kenodegard mentioned this pull request Apr 13, 2023

Add doctor subcommand plugin #12124

Merged

10 tasks

kenodegard added 3 commits April 13, 2023 16:14

Include missing import

5da3ed3

Fix deprecation decorator bug

42b6945

Use print instead of conda.stdout/stderr logging

1ea577c

kenodegard force-pushed the combine-runs branch from 527844d to 1ea577c Compare April 13, 2023 22:17

kenodegard added 6 commits April 13, 2023 22:24

Update run_command to handle conda run

4b4150b

Add typing for new fixtures

d83c981

Fix broken test

4468a38

List packages in temporary env

8576028

Add news

e1bbb31

Include fixture for test_list

8e10655

dholth reviewed Apr 14, 2023

View reviewed changes

sven6002 reviewed Apr 14, 2023

View reviewed changes

travishathaway reviewed Apr 14, 2023

View reviewed changes

kenodegard changed the title ~~Introducing simplified subcommand run fixture~~ Introducing conda_cli, tmp_env, and path_factory fixtures Apr 14, 2023

kenodegard added 2 commits April 17, 2023 10:00

Missing type hint

b4ec93a

Remove conda check and add missing docstrings

405629a

kenodegard requested review from sven6002 and travishathaway April 23, 2023 20:13

kenodegard requested a review from dholth April 23, 2023 20:13

beeankha previously approved these changes Apr 26, 2023

View reviewed changes

jezdez previously approved these changes Apr 27, 2023

View reviewed changes

travishathaway previously requested changes Apr 27, 2023

View reviewed changes

kenodegard dismissed stale reviews from jezdez and beeankha via c4824c6 April 27, 2023 17:47

kenodegard force-pushed the combine-runs branch from c4824c6 to 8f663ba Compare April 27, 2023 18:04

Add conda_cli, tmp_env, and path_factory to docs

7178464

kenodegard force-pushed the combine-runs branch from 8f663ba to 7178464 Compare April 27, 2023 19:27

kenodegard added 3 commits April 27, 2023 18:41

Minor edits

826c13e

Remove line references

310385c

Merge branch 'main' into combine-runs

e0c3577

jezdez requested a review from travishathaway May 2, 2023 09:30

jezdez approved these changes May 2, 2023

View reviewed changes

travishathaway added the source::anaconda created by members of Anaconda, Inc. label May 4, 2023

Merge branch 'main' into combine-runs

13249b8

beeankha approved these changes May 16, 2023

View reviewed changes

kenodegard merged commit 21232ba into conda:main May 16, 2023
91 checks passed

kenodegard deleted the combine-runs branch May 16, 2023 16:07

This was referenced May 16, 2023

Deprecate conda.testing.integration.get_conda_list_tuple #12676

Merged

Deprecate conda.testing.integration.make_temp_env in favor of conda.testing.tmp_env #12680

Closed

Convert tests/test_create.py to pytest-style #12682

Merged

sven6002 mentioned this pull request May 22, 2023

Manual QA For 23.5.x release #12663

Closed

6 tasks

github-actions bot added the locked [bot] locked due to inactivity label May 17, 2024

github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing `conda_cli`, `tmp_env`, and `path_factory` fixtures #12592

Introducing `conda_cli`, `tmp_env`, and `path_factory` fixtures #12592

kenodegard commented Apr 13, 2023 •

edited

Loading

dholth Apr 14, 2023

kenodegard Apr 14, 2023

jezdez Apr 27, 2023

dholth Apr 14, 2023

kenodegard Apr 14, 2023

sven6002 Apr 14, 2023

kenodegard Apr 14, 2023

dholth Apr 14, 2023

jezdez Apr 27, 2023

kenodegard Apr 27, 2023

sven6002 Apr 14, 2023

kenodegard Apr 14, 2023 •

edited

Loading

sven6002 Apr 14, 2023 •

edited

Loading

kenodegard Apr 14, 2023

travishathaway Apr 14, 2023

kenodegard Apr 14, 2023

jezdez left a comment

jezdez Apr 27, 2023

kenodegard Apr 27, 2023 •

edited

Loading

jezdez Apr 27, 2023

travishathaway left a comment

		@@ -5,6 +5,8 @@

		import pytest

		from conda.testing import conda_cli, path_factory, tmp_env

Introducing conda_cli, tmp_env, and path_factory fixtures #12592

Introducing conda_cli, tmp_env, and path_factory fixtures #12592

Conversation

kenodegard commented Apr 13, 2023 • edited Loading

Description

Checklist - did you ...

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenodegard Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

sven6002 Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jezdez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenodegard Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travishathaway left a comment

Choose a reason for hiding this comment

Introducing `conda_cli`, `tmp_env`, and `path_factory` fixtures #12592

Introducing `conda_cli`, `tmp_env`, and `path_factory` fixtures #12592

kenodegard commented Apr 13, 2023 •

edited

Loading

kenodegard Apr 14, 2023 •

edited

Loading

sven6002 Apr 14, 2023 •

edited

Loading

kenodegard Apr 27, 2023 •

edited

Loading