fix: make --dry-run useful (#256)

* chore: remove include_curr_dir option

The `include_curr_dir` option is only for strict_mode in python standard runner.
Few people will ever use the strict mode.
Realistically, no one will ever make `include_curr_dir` False because it increases the change
that ATS won't work (and it doesn't work on enough siturations to add one more).

I'm keeping the `strict_mode`, but making the `include_curr_dir` always True.
The default was already True, but now there's no choice.

* fix: make --dry-run useful

This makes the --dry-run in label-analysis actually do something useful.
It prints the tests you need to run _with_ the tool options to make ats run correctly.

In the case of pytest this means that it prints `--cov-context=test` (we need this to get labels)
and then the list of labels to run, spearated by spaces.

I couldn't find a way to actually export an ENV variable safely so I'm just printing to stdout
The small problem is that there's other output with it, so I decided to add "ATS_TESTS_TO_RUN="
to make grep-ing easier. Just for good measure throw it in a file as well.

So in thory if you get ATS_TESTS_TO_RUN from your CI you can do
`pytest <YOUR NORMAL OPTIONS> <ATS_TESTS_TO_RUN>` and that should work to run all the ATS tests.

closes codecov/engineering-team#470

* feat: only print dry-run to file if path specified

Introducing `--dry-run-output-path` as the filepath to print dry-run output into.
Only works if `--dry-run` is specified.

ALso updating help text for `--dry-run`

Author: giovanni-guidini

codecov_cli/commands/labelanalysis.py

@@ -1,7 +1,7 @@
-import json
 import logging
+import pathlib
 import time
-from typing import List
+from typing import List, Optional
 
 import click
 import requests
@@ -55,9 +55,16 @@
 @click.option(
     "--dry-run",
     "dry_run",
-    help="Userful during setup. This will run the label analysis, but will print the result to stdout and terminate instead of calling the runner.process_labelanalysis_result",
+    help='Print list of tests to run and options that need to be added to the test runner as a space-separated list to stdout. Format is ATS_TESTS_TO_RUN="<options> <test_1> <test_2> ... <test_n>"',
     is_flag=True,
 )
+@click.option(
+    "--dry-run-output-path",
+    "dry_run_output_path",
+    help="Prints the dry-run list into dry_run_output_path (in addition to stdout)",
+    type=pathlib.Path,
+    default=None,
+)
 @click.pass_context
 def label_analysis(
     ctx: click.Context,
@@ -67,6 +74,7 @@ def label_analysis(
     runner_name: str,
     max_wait_time: str,
     dry_run: bool,
+    dry_run_output_path: Optional[pathlib.Path],
 ):
     enterprise_url = ctx.obj.get("enterprise_url")
     logger.debug(
@@ -137,7 +145,12 @@ def label_analysis(
         # Retry it
         eid = _send_labelanalysis_request(payload, url, token_header)
         if eid is None:
-            _fallback_to_collected_labels(requested_labels, runner, dry_run=dry_run)
+            _fallback_to_collected_labels(
+                requested_labels,
+                runner,
+                dry_run=dry_run,
+                dry_run_output_path=dry_run_output_path,
+            )
             return
 
     has_result = False
@@ -157,23 +170,33 @@ def label_analysis(
             if not dry_run:
                 runner.process_labelanalysis_result(request_result)
             else:
-                _dry_run_output(LabelAnalysisRequestResult(request_result))
+                _dry_run_output(
+                    LabelAnalysisRequestResult(request_result),
+                    runner,
+                    dry_run_output_path,
+                )
             return
         if resp_json["state"] == "error":
             logger.error(
                 "Request had problems calculating",
                 extra=dict(extra_log_attributes=dict(resp_json=resp_json)),
             )
             _fallback_to_collected_labels(
-                collected_labels=requested_labels, runner=runner, dry_run=dry_run
+                collected_labels=requested_labels,
+                runner=runner,
+                dry_run=dry_run,
+                dry_run_output_path=dry_run_output_path,
             )
             return
         if max_wait_time and (time.monotonic() - start_wait) > max_wait_time:
             logger.error(
                 f"Exceeded max waiting time of {max_wait_time} seconds. Running all tests.",
             )
             _fallback_to_collected_labels(
-                collected_labels=requested_labels, runner=runner, dry_run=dry_run
+                collected_labels=requested_labels,
+                runner=runner,
+                dry_run=dry_run,
+                dry_run_output_path=dry_run_output_path,
             )
             return
         logger.info("Waiting more time for result...")
@@ -285,27 +308,31 @@ def _send_labelanalysis_request(payload, url, token_header):
     return eid
 
 
-def _dry_run_output(result: LabelAnalysisRequestResult):
-    logger.info(
-        "Not executing tests because '--dry-run' is on. List of labels selected for running below."
-    )
-    logger.info("")
-    logger.info("Label groups:")
-    logger.info(
-        "- absent_labels: Set of new labels found in HEAD that are not present in BASE"
+def _dry_run_output(
+    result: LabelAnalysisRequestResult,
+    runner: LabelAnalysisRunnerInterface,
+    dry_run_output_path: Optional[pathlib.Path],
+):
+    labels_to_run = list(
+        set(
+            result.absent_labels
+            + result.global_level_labels
+            + result.present_diff_labels
+        )
     )
-    logger.info("- present_diff_labels: Set of labels affected by the git diff")
-    logger.info("- global_level_labels: Set of labels that possibly touch global code")
-    logger.info("- present_report_labels: Set of labels previously uploaded")
-    logger.info("")
-    logger.info(json.dumps(result))
+    output = runner.dry_run_runner_options + sorted(labels_to_run)
+    if dry_run_output_path is not None:
+        with open(dry_run_output_path, "w") as fd:
+            fd.write(" ".join(output) + "\n")
+    click.echo(f"ATS_TESTS_TO_RUN=\"{' '.join(output)}\"")
 
 
 def _fallback_to_collected_labels(
     collected_labels: List[str],
     runner: LabelAnalysisRunnerInterface,
     *,
     dry_run: bool = False,
+    dry_run_output_path: Optional[pathlib.Path] = None,
 ) -> dict:
     logger.info("Trying to fallback on collected labels")
     if collected_labels:
@@ -321,6 +348,8 @@ def _fallback_to_collected_labels(
         if not dry_run:
             return runner.process_labelanalysis_result(fake_response)
         else:
-            return _dry_run_output(LabelAnalysisRequestResult(fake_response))
+            return _dry_run_output(
+                LabelAnalysisRequestResult(fake_response), runner, dry_run_output_path
+            )
     logger.error("Cannot fallback to collected labels because no labels were collected")
     raise click.ClickException("Failed to get list of labels to run")

codecov_cli/runners/python_standard_runner.py

@@ -97,6 +97,9 @@ def _execute_pytest_subprocess(
 
 
 class PythonStandardRunner(LabelAnalysisRunnerInterface):
+
+    dry_run_runner_options = ["--cov-context=test"]
+
     def __init__(self, config_params: Optional[dict] = None) -> None:
         super().__init__()
         if config_params is None:

codecov_cli/runners/types.py

@@ -1,4 +1,4 @@
-from typing import List
+from typing import Dict, List
 
 
 # This is supposed to be a TypedDict,
@@ -23,7 +23,8 @@ def global_level_labels(self) -> List[str]:
 
 
 class LabelAnalysisRunnerInterface(object):
-    params = None
+    params: Dict = None
+    dry_run_runner_options: List[str] = []
 
     def collect_tests(self) -> List[str]:
         raise NotImplementedError()

tests/commands/test_invoke_labelanalysis.py

@@ -1,4 +1,5 @@
 import json
+from pathlib import Path
 
 import click
 import pytest
@@ -122,10 +123,13 @@ def test_labelanalysis_help(self, mocker, fake_ci_provider):
             "  --max-wait-time INTEGER       Max time (in seconds) to wait for the label",
             "                                analysis result before falling back to running",
             "                                all tests. Default is to wait forever.",
-            "  --dry-run                     Userful during setup. This will run the label",
-            "                                analysis, but will print the result to stdout",
-            "                                and terminate instead of calling the",
-            "                                runner.process_labelanalysis_result",
+            "  --dry-run                     Print list of tests to run and options that need",
+            "                                to be added to the test runner as a space-",
+            "                                separated list to stdout. Format is",
+            '                                ATS_TESTS_TO_RUN="<options> <test_1> <test_2>',
+            '                                ... <test_n>"',
+            "  --dry-run-output-path PATH    Prints the dry-run list into dry_run_output_path",
+            "                                (in addition to stdout)",
             "  -h, --help                    Show this message and exit.",
             "",
         ]
@@ -258,21 +262,90 @@ def test_invoke_label_analysis_dry_run(self, get_labelanalysis_deps, mocker):
                 json={"state": "finished", "result": label_analysis_result},
             )
             cli_runner = CliRunner()
-            result = cli_runner.invoke(
-                cli,
-                [
-                    "label-analysis",
-                    "--token=STATIC_TOKEN",
-                    f"--base-sha={FAKE_BASE_SHA}",
-                    "--dry-run",
+            with cli_runner.isolated_filesystem():
+                result = cli_runner.invoke(
+                    cli,
+                    [
+                        "label-analysis",
+                        "--token=STATIC_TOKEN",
+                        f"--base-sha={FAKE_BASE_SHA}",
+                        "--dry-run",
+                    ],
+                    obj={},
+                )
+                mock_get_runner.assert_called()
+                fake_runner.process_labelanalysis_result.assert_not_called()
+        print(result.output)
+        assert result.exit_code == 0
+        assert (
+            'ATS_TESTS_TO_RUN="--labels test_absent test_global test_in_diff'
+            in result.output
+        )
+
+    def test_invoke_label_analysis_dry_run_with_output_path(
+        self, get_labelanalysis_deps, mocker
+    ):
+        mock_get_runner = get_labelanalysis_deps["mock_get_runner"]
+        fake_runner = get_labelanalysis_deps["fake_runner"]
+
+        label_analysis_result = {
+            "present_report_labels": ["test_present"],
+            "absent_labels": ["test_absent"],
+            "present_diff_labels": ["test_in_diff"],
+            "global_level_labels": ["test_global"],
+        }
+
+        with responses.RequestsMock() as rsps:
+            rsps.add(
+                responses.POST,
+                "https://api.codecov.io/labels/labels-analysis",
+                json={"external_id": "label-analysis-request-id"},
+                status=201,
+                match=[
+                    matchers.header_matcher({"Authorization": "Repotoken STATIC_TOKEN"})
                 ],
-                obj={},
             )
-            mock_get_runner.assert_called()
-            fake_runner.process_labelanalysis_result.assert_not_called()
+            rsps.add(
+                responses.PATCH,
+                "https://api.codecov.io/labels/labels-analysis/label-analysis-request-id",
+                json={"external_id": "label-analysis-request-id"},
+                status=201,
+                match=[
+                    matchers.header_matcher({"Authorization": "Repotoken STATIC_TOKEN"})
+                ],
+            )
+            rsps.add(
+                responses.GET,
+                "https://api.codecov.io/labels/labels-analysis/label-analysis-request-id",
+                json={"state": "finished", "result": label_analysis_result},
+            )
+            cli_runner = CliRunner()
+            with cli_runner.isolated_filesystem():
+                result = cli_runner.invoke(
+                    cli,
+                    [
+                        "label-analysis",
+                        "--token=STATIC_TOKEN",
+                        f"--base-sha={FAKE_BASE_SHA}",
+                        "--dry-run",
+                        "--dry-run-output-path=ats_output_path",
+                    ],
+                    obj={},
+                )
+                labels_file = Path("ats_output_path")
+                assert labels_file.exists() and labels_file.is_file()
+                with open(labels_file, "r") as fd:
+                    assert fd.readlines() == [
+                        "--labels test_absent test_global test_in_diff\n"
+                    ]
+                mock_get_runner.assert_called()
+                fake_runner.process_labelanalysis_result.assert_not_called()
         print(result.output)
         assert result.exit_code == 0
-        assert json.dumps(label_analysis_result) in result.output
+        assert (
+            'ATS_TESTS_TO_RUN="--labels test_absent test_global test_in_diff'
+            in result.output
+        )
 
     def test_fallback_to_collected_labels(self, mocker):
         mock_runner = mocker.MagicMock()
@@ -367,7 +440,9 @@ def test_fallback_dry_run(self, get_labelanalysis_deps, mocker, use_verbose_opti
                     "absent_labels": collected_labels,
                     "present_diff_labels": [],
                     "global_level_labels": [],
-                }
+                },
+                fake_runner,
+                None,
             )
         assert result.exit_code == 0
 

tests/factory.py

@@ -75,6 +75,8 @@ def get_fallback_value(self, fallback_field: FallbackFieldEnum) -> Optional[str]
 
 
 class FakeRunner(LabelAnalysisRunnerInterface):
+    dry_run_runner_options = ["--labels"]
+
     def __init__(self, collect_tests_response: List[str]) -> None:
         super().__init__()
         self.collect_tests_response = collect_tests_response