Ensure ddev env test skips environments when disabled by e2e-env (#21119)

* Modify ddev env test to ensure that when an environment does not have e2e enabled the command does not run

* Add chnagelog and remove previous fix based on hardcoded paths

* Improve test coverage for ddev env test

Author: AAraKKe

ddev/changelog.d/21119.fixed

@@ -0,0 +1 @@
+Fix ddev env test to respect e2e-env config flag even when an environment is specified

ddev/pyproject.toml

@@ -86,6 +86,7 @@ target-version = ["py312"]
 extend-exclude = "src/ddev/_version.py"
 
 [tool.ruff]
+extend = "../pyproject.toml"
 exclude = []
 target-version = "py312"
 line-length = 120

ddev/src/ddev/cli/env/test.py

@@ -78,24 +78,21 @@ def test_command(
     from ddev.config.constants import AppEnvVars
     from ddev.e2e.config import EnvDataStorage
     from ddev.e2e.constants import E2EMetadata
-    from ddev.repo.constants import NOT_E2E_TESTABLE
     from ddev.utils.ci import running_in_ci
     from ddev.utils.structures import EnvVars
 
     app: Application = ctx.obj
     integration = app.repo.integrations.get(intg_name)
 
-    if integration.name in NOT_E2E_TESTABLE:
-        app.display_info(f"Selected target {integration.name!r} does not have E2E tests to run. Skipping.")
-        return
-
     storage = EnvDataStorage(app.data_dir)
     active_envs = storage.get_environments(integration.name)
 
     if environment is None:
         environment = 'all' if (not active_envs or running_in_ci()) else 'active'
 
-    if environment == 'all':
+    if environment == 'active':
+        env_names = active_envs
+    else:
         import json
         import sys
 
@@ -108,20 +105,20 @@ def test_command(
             except json.JSONDecodeError:
                 app.abort(f'Failed to parse environments for `{integration.name}`:\n{repr(env_data_output)}')
 
+        no_python_filter = python_filter is None
+        all_environments = environment == 'all'
+
         env_names = [
             name
             for name, data in environments.items()
-            if data.get('e2e-env')
+            if data.get('e2e-env', False)
             and (not data.get('platforms') or app.platform.name in data['platforms'])
-            and (python_filter is None or data.get('python') == python_filter)
+            and (no_python_filter or data.get('python') == python_filter)
+            and (name == environment or all_environments)
         ]
 
-    elif environment == 'active':
-        env_names = active_envs
-    else:
-        env_names = [environment]
-
     if not env_names:
+        app.display_info(f"Selected target {integration.name!r} disabled by e2e-env option.")
         return
 
     app.display_header(integration.display_name)

ddev/src/ddev/repo/constants.py

@@ -3,7 +3,6 @@
 # Licensed under a 3-clause BSD style license (see LICENSE)
 CONFIG_DIRECTORY = '.ddev'
 NOT_SHIPPABLE = frozenset(['datadog_checks_dev', 'datadog_checks_tests_helper', 'ddev'])
-NOT_E2E_TESTABLE = frozenset(['datadog_checks_dev', 'datadog_checks_base', 'datadog_checks_tests_helper', 'ddev'])
 FULL_NAMES = {
     'core': 'integrations-core',
     'extras': 'integrations-extras',

ddev/tests/cli/env/test_test.py

@@ -1,49 +1,173 @@
 # (C) Datadog, Inc. 2024-present
 # All rights reserved
 # Licensed under a 3-clause BSD style license (see LICENSE)
-from contextlib import nullcontext
 
-import mock
+import json
+from collections.abc import Callable, Generator, Mapping
+from typing import Any
+from unittest.mock import MagicMock
+
 import pytest
+from click.testing import Result
+from pytest_mock import MockerFixture, MockType
 
+from ddev.e2e.config import EnvData, EnvDataStorage
+from ddev.utils.fs import Path
 from tests.helpers.mocks import MockPopen
+from tests.helpers.runner import CliRunner
 
 
-class MockEnvVars:
-    def __init__(self, env_vars=None):
-        assert env_vars['DDEV_REPO'] == 'core'
+def setup(
+    mocker: MockerFixture,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    hatch_json_output: Mapping[str, Any] | str | None = None,
+):
+    mocker.patch('subprocess.run', side_effect=write_result_file({'metadata': {}, 'config': {}}))
 
-    def __enter__(*_args, **_kwargs):
-        pass
+    if hatch_json_output is not None:
+        if isinstance(hatch_json_output, str):
+            hatch_output = hatch_json_output.encode()
+        elif isinstance(hatch_json_output, dict):
+            hatch_output = json.dumps(hatch_json_output).encode()
+        else:
+            pytest.fail('Invalid hatch_json_output type')
 
-    def __exit__(*_args, **_kwargs):
-        pass
+        mocker.patch('subprocess.Popen', return_value=MockPopen(returncode=0, stdout=hatch_output))
 
 
-def test_env_vars_repo(ddev, helpers, data_dir, write_result_file, mocker):
-    mocker.patch('subprocess.run', side_effect=write_result_file({'metadata': {}, 'config': {}}))
-    mocker.patch('subprocess.Popen', return_value=MockPopen(returncode=0))
-    with mock.patch('ddev.utils.structures.EnvVars', side_effect=MockEnvVars):
-        result = ddev('env', 'test', 'postgres', 'py3.12')
-        assert result.exit_code == 0, result.output
-        # Ensure test was not skipped
-        assert "does not have E2E tests to run" not in result.output
+@pytest.fixture()
+def mock_commands(mocker: MockerFixture) -> Generator[tuple[MockType, MockType, MockType]]:
+    start_mock = mocker.patch(
+        'ddev.cli.env.start.start',
+        return_value=Result(
+            return_value=0,
+            runner=MagicMock(),
+            stdout_bytes=b'',
+            stderr_bytes=b'',
+            exit_code=0,
+            exception=None,
+        ),
+    )
+    stop_mock = mocker.patch(
+        'ddev.cli.env.stop.stop',
+        return_value=Result(
+            return_value=0,
+            runner=MagicMock(),
+            stdout_bytes=b'',
+            stderr_bytes=b'',
+            exit_code=0,
+            exception=None,
+        ),
+    )
+    test_mock = mocker.patch(
+        'ddev.cli.test.test',
+        return_value=Result(
+            return_value=0,
+            runner=MagicMock(),
+            stdout_bytes=b'',
+            stderr_bytes=b'',
+            exit_code=0,
+            exception=None,
+        ),
+    )
+    yield start_mock, stop_mock, test_mock
+
+
+def assert_commands_run(mock_commands: tuple[MockType, MockType, MockType], call_count: int = 1):
+    assert mock_commands[0].call_count == call_count
+    assert mock_commands[1].call_count == call_count
+    assert mock_commands[2].call_count == call_count
 
 
 @pytest.mark.parametrize(
-    'target, expectation',
+    'e2e_env, predicate',
     [
-        ('datadog_checks_dev', nullcontext()),
-        ('datadog_checks_base', nullcontext()),
-        # This will raise an OSError because the package is not a valid integration
-        ('datadog_checks_tests_helper', pytest.raises(OSError)),
-        ('ddev', nullcontext()),
+        (False, lambda result: "disabled by e2e-env option" in result.output),
+        (True, lambda result: "disabled by e2e-env option" not in result.output),
     ],
-    ids=['datadog_checks_dev', 'datadog_checks_base', 'datadog_checks_tests_helper', 'ddev'],
+    ids=['e2e-env-false', 'e2e-env-true'],
 )
-@pytest.mark.parametrize('env', ['py3.12', 'all', ''], ids=['py3.12', 'all', 'no-env'])
-def test_env_test_not_e2e_testable(ddev, target: str, env: str, expectation):
-    with expectation:
-        result = ddev('env', 'test', target, env)
+def test_env_vars_repo(
+    ddev: CliRunner,
+    data_dir: Path,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    mocker: MockerFixture,
+    e2e_env: bool,
+    predicate: Callable[[Result], bool],
+    mock_commands: tuple[MockType, MockType, MockType],
+):
+    setup(mocker, write_result_file, hatch_json_output={'py3.12': {'e2e-env': e2e_env}})
+    mocker.patch.object(EnvData, 'read_metadata', return_value={})
+
+    result = ddev('env', 'test', 'postgres', 'py3.12')
+    assert result.exit_code == 0, result.output
+    # Ensure test was not skipped
+    assert predicate(result)
+    assert_commands_run(mock_commands, 1 if e2e_env else 0)
+
+
+@pytest.mark.parametrize('environment, command_call_count', [('active', 0), ('all', 2), ('py3.12', 1)])
+def test_environment_runs_for_enabled_environments(
+    ddev: CliRunner,
+    data_dir: Path,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    mocker: MockerFixture,
+    environment: str,
+    mock_commands: tuple[MockType, MockType, MockType],
+    command_call_count: int,
+):
+    setup(
+        mocker,
+        write_result_file,
+        hatch_json_output={'py3.12': {'e2e-env': True}, 'py3.13': {'e2e-env': False}, 'py3.13-v1': {'e2e-env': True}},
+    )
+    with mocker.patch.object(EnvData, 'read_metadata', return_value={}):
+        result = ddev('env', 'test', 'postgres', environment)
+        assert result.exit_code == 0, result.output
+        assert_commands_run(mock_commands, command_call_count)
+
+
+def test_command_errors_out_when_cannot_parse_json_output_from_hatch(
+    ddev: CliRunner,
+    data_dir: Path,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    mocker: MockerFixture,
+):
+    setup(mocker, write_result_file, hatch_json_output='invalid json')
+    result = ddev('env', 'test', 'postgres', 'py3.12')
+    assert result.exit_code == 1, result.output
+
+
+def test_runningin_ci_triggers_all_environments_when_not_supplied(
+    ddev: CliRunner,
+    data_dir: Path,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    mocker: MockerFixture,
+    mock_commands: tuple[MockType, MockType, MockType],
+):
+    setup(mocker, write_result_file, hatch_json_output={'py3.12': {'e2e-env': True}, 'py3.13': {'e2e-env': True}})
+    mocker.patch('ddev.utils.ci.running_in_ci', return_value=True)
+
+    with mocker.patch.object(EnvData, 'read_metadata', return_value={}):
+        result = ddev('env', 'test', 'postgres')
+        assert result.exit_code == 0, result.output
+        assert_commands_run(mock_commands, 2)
+
+
+def test_run_only_active_environments_when_not_running_in_ci_and_active_environments_exist(
+    ddev: CliRunner,
+    data_dir: Path,
+    write_result_file: Callable[[Mapping[str, Any]], None],
+    mocker: MockerFixture,
+    mock_commands: tuple[MockType, MockType, MockType],
+):
+    setup(mocker, write_result_file, hatch_json_output={'py3.12': {'e2e-env': True}, 'py3.13': {'e2e-env': True}})
+    mocker.patch('ddev.utils.ci.running_in_ci', return_value=False)
+
+    with (
+        mocker.patch.object(EnvData, 'read_metadata', return_value={}),
+        mocker.patch.object(EnvDataStorage, 'get_environments', return_value=['py3.12']),
+    ):
+        result = ddev('env', 'test', 'postgres')
         assert result.exit_code == 0, result.output
-        assert "does not have E2E tests to run" in result.output
+        assert_commands_run(mock_commands, 1)