Update --dry-run output (#272)

related to codecov/engineering-team#555

These changes update the output of `--dry-run` option so that:
1. Users can get a list of tests that are being skipped
2. Users have the option to parse the output from JSON

> ⚠️
>
>  The JSON output doesn't include the runner options in the tests to run list.
>  Instead there's the 'runner_options' key with that info.
>  This is to make sharding the list of tests easier

It's considerably more output, but given that we don't have clear guidelines on the
best way to do things I think it's better to provide all options.
Eventually adoption will converge to some way or another and we prioritize that.

Author: giovanni-guidini

codecov_cli/commands/labelanalysis.py

@@ -1,3 +1,4 @@
+import json
 import logging
 import pathlib
 import time
@@ -55,13 +56,23 @@
 @click.option(
     "--dry-run",
     "dry_run",
-    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>"',
+    help=(
+        "Print list of tests to run AND tests skipped (and options that need to be added to the test runner) to stdout. "
+        + "Also prints the same information in JSON format. "
+        + "JSON will have keys 'ats_tests_to_run', 'ats_tests_to_skip' and 'runner_options'. "
+        + "List of tests to run is prefixed with ATS_TESTS_TO_RUN= "
+        + "List of tests to skip is prefixed with ATS_TESTS_TO_SKIP="
+    ),
     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)",
+    help=(
+        "Prints the dry-run list (ATS_TESTS_TO_RUN) into dry_run_output_path (in addition to stdout)\n"
+        + "AND prints ATS_TESTS_TO_SKIP into dry_run_output_path_skipped\n"
+        + "AND prints dry-run JSON output into dry_run_output_path.json"
+    ),
     type=pathlib.Path,
     default=None,
 )
@@ -313,18 +324,50 @@ def _dry_run_output(
     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
-        )
+    labels_to_run = set(
+        result.absent_labels + result.global_level_labels + result.present_diff_labels
+    )
+    labels_skipped = set(result.present_report_labels) - labels_to_run
+    # If the test label can contain spaces and dashes the test runner might
+    # interpret it as an option and not a label
+    # So we wrap it in doublequotes just to be extra sure
+    labels_run_wrapped_double_quotes = sorted(
+        map(lambda l: '"' + l + '"', labels_to_run)
+    )
+    labels_skip_wrapped_double_quotes = sorted(
+        map(lambda l: '"' + l + '"', labels_skipped)
+    )
+
+    output_as_dict = dict(
+        runner_options=runner.dry_run_runner_options,
+        ats_tests_to_run=labels_run_wrapped_double_quotes,
+        ats_tests_to_skip=labels_skip_wrapped_double_quotes,
     )
-    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)}\"")
+            fd.write(
+                " ".join(
+                    runner.dry_run_runner_options + labels_run_wrapped_double_quotes
+                )
+                + "\n"
+            )
+        with open(str(dry_run_output_path) + "_skipped", "w") as fd:
+            fd.write(
+                " ".join(
+                    runner.dry_run_runner_options + labels_skip_wrapped_double_quotes
+                )
+                + "\n"
+            )
+        with open(str(dry_run_output_path) + ".json", "w") as fd:
+            fd.write(json.dumps(output_as_dict) + "\n")
+
+    click.echo(json.dumps(output_as_dict))
+    click.echo(
+        f"ATS_TESTS_TO_RUN={' '.join(runner.dry_run_runner_options + labels_run_wrapped_double_quotes)}"
+    )
+    click.echo(
+        f"ATS_TESTS_TO_SKIP={' '.join(runner.dry_run_runner_options + labels_skip_wrapped_double_quotes)}"
+    )
 
 
 def _fallback_to_collected_labels(

tests/commands/test_invoke_labelanalysis.py

@@ -123,13 +123,19 @@ 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                     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)",
+            "  --dry-run                     Print list of tests to run AND tests skipped",
+            "                                (and options that need to be added to the test",
+            "                                runner) to stdout. Also prints the same",
+            "                                information in JSON format. JSON will have keys",
+            "                                'ats_tests_to_run', 'ats_tests_to_skip' and",
+            "                                'runner_options'. List of tests to run is",
+            "                                prefixed with ATS_TESTS_TO_RUN= List of tests to",
+            "                                skip is prefixed with ATS_TESTS_TO_SKIP=",
+            "  --dry-run-output-path PATH    Prints the dry-run list (ATS_TESTS_TO_RUN) into",
+            "                                dry_run_output_path (in addition to stdout) AND",
+            "                                prints ATS_TESTS_TO_SKIP into",
+            "                                dry_run_output_path_skipped AND prints dry-run",
+            "                                JSON output into dry_run_output_path.json",
             "  -h, --help                    Show this message and exit.",
             "",
         ]
@@ -231,7 +237,7 @@ def test_invoke_label_analysis_dry_run(self, get_labelanalysis_deps, mocker):
         fake_runner = get_labelanalysis_deps["fake_runner"]
 
         label_analysis_result = {
-            "present_report_labels": ["test_present"],
+            "present_report_labels": ["test_present", "test_in_diff", "test_global"],
             "absent_labels": ["test_absent"],
             "present_diff_labels": ["test_in_diff"],
             "global_level_labels": ["test_global"],
@@ -278,9 +284,14 @@ def test_invoke_label_analysis_dry_run(self, get_labelanalysis_deps, mocker):
         print(result.output)
         assert result.exit_code == 0
         assert (
-            'ATS_TESTS_TO_RUN="--labels test_absent test_global test_in_diff'
+            '{"runner_options": ["--labels"], "ats_tests_to_run": ["\\"test_absent\\"", "\\"test_global\\"", "\\"test_in_diff\\""], "ats_tests_to_skip": ["\\"test_present\\""]}'
             in result.output
         )
+        assert (
+            'ATS_TESTS_TO_RUN=--labels "test_absent" "test_global" "test_in_diff"'
+            in result.output
+        )
+        assert 'ATS_TESTS_TO_SKIP=--labels "test_present"' in result.output
 
     def test_invoke_label_analysis_dry_run_with_output_path(
         self, get_labelanalysis_deps, mocker
@@ -289,7 +300,7 @@ def test_invoke_label_analysis_dry_run_with_output_path(
         fake_runner = get_labelanalysis_deps["fake_runner"]
 
         label_analysis_result = {
-            "present_report_labels": ["test_present"],
+            "present_report_labels": ["test_present", "test_in_diff", "test_global"],
             "absent_labels": ["test_absent"],
             "present_diff_labels": ["test_in_diff"],
             "global_level_labels": ["test_global"],
@@ -332,20 +343,38 @@ def test_invoke_label_analysis_dry_run_with_output_path(
                     ],
                     obj={},
                 )
-                labels_file = Path("ats_output_path")
+                print(result)
+                print(result.output)
+                ats_output_path = "ats_output_path"
+                labels_file = Path(ats_output_path)
+                skip_labels_file = Path(ats_output_path + "_skipped")
+                json_output = Path(ats_output_path + ".json")
                 assert labels_file.exists() and labels_file.is_file()
+                assert skip_labels_file.exists() and skip_labels_file.is_file()
+                assert json_output.exists() and json_output.is_file()
                 with open(labels_file, "r") as fd:
                     assert fd.readlines() == [
-                        "--labels test_absent test_global test_in_diff\n"
+                        '--labels "test_absent" "test_global" "test_in_diff"\n'
+                    ]
+                with open(skip_labels_file, "r") as fd:
+                    assert fd.readlines() == ['--labels "test_present"\n']
+                with open(json_output, "r") as fd:
+                    assert fd.readlines() == [
+                        '{"runner_options": ["--labels"], "ats_tests_to_run": ["\\"test_absent\\"", "\\"test_global\\"", "\\"test_in_diff\\""], "ats_tests_to_skip": ["\\"test_present\\""]}\n'
                     ]
                 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'
+            '{"runner_options": ["--labels"], "ats_tests_to_run": ["\\"test_absent\\"", "\\"test_global\\"", "\\"test_in_diff\\""], "ats_tests_to_skip": ["\\"test_present\\""]}'
+            in result.output
+        )
+        assert (
+            'ATS_TESTS_TO_RUN=--labels "test_absent" "test_global" "test_in_diff"'
             in result.output
         )
+        assert 'ATS_TESTS_TO_SKIP=--labels "test_present"' in result.output
 
     def test_fallback_to_collected_labels(self, mocker):
         mock_runner = mocker.MagicMock()