Merge pytest addopts (#16614)

Closes #16601

docs/markdown/Python/python-goals/python-test-goal.md

@@ -499,3 +499,21 @@ report = true
 ```
 
 This will default to writing test reports to `dist/test/reports`. You may also want to set the option `[pytest].junit_family` to change the format. Run `./pants help-advanced pytest` for more information.
+
+Customizing Pytest command line options per target
+--------------------------------------------------
+
+You can set `PYTEST_ADDOPTS` environment variable to add your own command line options, like this:
+
+```python BUILD
+python_tests(
+    name="tests",
+    ...
+    extra_env_vars=[
+        "PYTEST_ADDOPTS=-p myplugin --reuse-db",
+    ],
+    ...
+)
+```
+
+Take note that Pants uses some CLI args for its internal mechanism of controlling Pytest (`--color`, `--junit-xml`, `junit_family`, `--cov`, `--cov-report` and `--cov-config`). If these options are overridden, Pants Pytest handling may not work correctly. Set these at your own peril!

src/python/pants/backend/python/goals/pytest_runner.py

@@ -322,7 +322,10 @@ async def setup_pytest_for_target(
         ),
     )
 
-    add_opts = [f"--color={'yes' if global_options.colors else 'no'}"]
+    # Don't forget to keep "Customize Pytest command line options per target" section in
+    # docs/markdown/Python/python-goals/python-test-goal.md up to date when changing
+    # which flags are added to `pytest_args`.
+    pytest_args = [f"--color={'yes' if global_options.colors else 'no'}"]
     output_files = []
 
     results_file_name = None
@@ -333,12 +336,11 @@ async def setup_pytest_for_target(
                 f"batch-of-{results_file_prefix}+{len(request.field_sets)-1}-files"
             )
         results_file_name = f"{results_file_prefix}.xml"
-        add_opts.extend(
-            (f"--junitxml={results_file_name}", "-o", f"junit_family={pytest.junit_family}")
+        pytest_args.extend(
+            (f"--junit-xml={results_file_name}", "-o", f"junit_family={pytest.junit_family}")
         )
         output_files.append(results_file_name)
 
-    coverage_args = []
     if test_subsystem.use_coverage and not request.is_debug:
         pytest.validate_pytest_cov_included()
         output_files.append(".coverage")
@@ -353,14 +355,15 @@ async def setup_pytest_for_target(
             # materialized to the Process chroot.
             cov_args = [f"--cov={source_root}" for source_root in prepared_sources.source_roots]
 
-        coverage_args = [
-            "--cov-report=",  # Turn off output.
-            f"--cov-config={coverage_config.path}",
-            *cov_args,
-        ]
+        pytest_args.extend(
+            (
+                "--cov-report=",  # Turn off output.
+                f"--cov-config={coverage_config.path}",
+                *cov_args,
+            )
+        )
 
     extra_env = {
-        "PYTEST_ADDOPTS": " ".join(add_opts),
         "PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots),
         **test_extra_env.env,
         # NOTE: field_set_extra_env intentionally after `test_extra_env` to allow overriding within
@@ -402,7 +405,9 @@ async def setup_pytest_for_target(
                 *(("-n", "{pants_concurrency}") if xdist_concurrency else ()),
                 *request.prepend_argv,
                 *pytest.args,
-                *coverage_args,
+                # N.B.: Now that we're using command-line options instead of the PYTEST_ADDOPTS
+                # environment variable, it's critical that `pytest_args` comes after `pytest.args`.
+                *pytest_args,
                 *field_set_source_files.files,
             ),
             extra_env=extra_env,

src/python/pants/backend/python/goals/pytest_runner_integration_test.py

@@ -539,14 +539,14 @@ def test_args():
             ),
             f"{PACKAGE}/BUILD": dedent(
                 """\
-            python_tests(
-                extra_env_vars=(
-                    "PYTHON_TESTS_VAR_WITHOUT_VALUE",
-                    "PYTHON_TESTS_VAR_WITH_VALUE=python_tests_var_with_value",
-                    "PYTHON_TESTS_OVERRIDE_WITH_VALUE_VAR=python_tests_override_with_value_var_override",
+                python_tests(
+                    extra_env_vars=(
+                        "PYTHON_TESTS_VAR_WITHOUT_VALUE",
+                        "PYTHON_TESTS_VAR_WITH_VALUE=python_tests_var_with_value",
+                        "PYTHON_TESTS_OVERRIDE_WITH_VALUE_VAR=python_tests_override_with_value_var_override",
+                    )
                 )
-            )
-            """
+                """
             ),
         }
     )
@@ -555,7 +555,7 @@ def test_args():
         rule_runner,
         [tgt],
         extra_args=[
-            '--test-extra-env-vars=["ARG_WITH_VALUE_VAR=arg_with_value_var", "ARG_WITHOUT_VALUE_VAR", "PYTHON_TESTS_OVERRIDE_ARG_WITH_VALUE_VAR"]'
+            "--test-extra-env-vars=['ARG_WITH_VALUE_VAR=arg_with_value_var', 'ARG_WITHOUT_VALUE_VAR', 'PYTHON_TESTS_OVERRIDE_ARG_WITH_VALUE_VAR']"
         ],
         env={
             "ARG_WITHOUT_VALUE_VAR": "arg_without_value_value",
@@ -566,6 +566,76 @@ def test_args():
     assert result.exit_code == 0
 
 
+def test_pytest_addopts_test_extra_env(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            f"{PACKAGE}/test_pytest_addopts_test_extra_env.py": dedent(
+                """\
+                import os
+
+                def test_addopts():
+                    assert "-vv" in os.getenv("PYTEST_ADDOPTS")
+                    assert "--maxfail=2" in os.getenv("PYTEST_ADDOPTS")
+                """
+            ),
+            f"{PACKAGE}/BUILD": dedent(
+                """\
+                python_tests()
+                """
+            ),
+        }
+    )
+    tgt = rule_runner.get_target(
+        Address(PACKAGE, relative_file_path="test_pytest_addopts_test_extra_env.py")
+    )
+    result = run_pytest(
+        rule_runner,
+        [tgt],
+        extra_args=[
+            "--test-extra-env-vars=['PYTEST_ADDOPTS=-vv --maxfail=2']",
+        ],
+    )
+    assert result.exit_code == 0
+
+
+def test_pytest_addopts_field_set_extra_env(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            f"{PACKAGE}/test_pytest_addopts_field_set_extra_env.py": dedent(
+                """\
+                import os
+
+                def test_addopts():
+                    assert "-vv" not in os.getenv("PYTEST_ADDOPTS")
+                    assert "--maxfail=2" not in os.getenv("PYTEST_ADDOPTS")
+                    assert "-ra" in os.getenv("PYTEST_ADDOPTS")
+                    assert "-q" in os.getenv("PYTEST_ADDOPTS")
+                """
+            ),
+            f"{PACKAGE}/BUILD": dedent(
+                """\
+                python_tests(
+                    extra_env_vars=(
+                        "PYTEST_ADDOPTS=-ra -q",
+                    )
+                )
+                """
+            ),
+        }
+    )
+    tgt = rule_runner.get_target(
+        Address(PACKAGE, relative_file_path="test_pytest_addopts_field_set_extra_env.py")
+    )
+    result = run_pytest(
+        rule_runner,
+        [tgt],
+        extra_args=[
+            "--test-extra-env-vars=['PYTEST_ADDOPTS=-vv --maxfail=2']",  # should be overridden by `python_tests`
+        ],
+    )
+    assert result.exit_code == 0
+
+
 class UsedPlugin(PytestPluginSetupRequest):
     @classmethod
     def is_applicable(cls, target: Target) -> bool:
@@ -735,6 +805,7 @@ def test_debug_adaptor_request_argv(rule_runner: RuleRunner) -> None:
         "--wait-for-client",
         "-c",
         unittest.mock.ANY,
+        "--color=no",
         "tests/python/pants_test/test_foo.py",
     )