From c5e2e812a4bcf988f40dc3dc2b1dbbf05719632e Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Fri, 2 May 2025 14:36:44 -0700 Subject: [PATCH] fix(up): Cleaning up 'up' local runtime logic --- devservices/commands/up.py | 32 +++++++++++----------- tests/commands/test_up.py | 56 ++++++++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index 127425ef..84aad79d 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -101,14 +101,14 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: services_with_local_runtime = state.get_services_by_runtime( ServiceRuntime.LOCAL ) - skipped_services = set() + local_runtime_dependency_names = set() for service_with_local_runtime in services_with_local_runtime: if ( mode in modes and service_with_local_runtime != service.name and service_with_local_runtime in modes[mode] ): - skipped_services.add(service_with_local_runtime) + local_runtime_dependency_names.add(service_with_local_runtime) if args.exclude_local: status.warning( f"Skipping '{service_with_local_runtime}' as it is set to run locally" @@ -139,9 +139,9 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: if ( service_with_local_runtime in [dep.service_name for dep in remote_dependencies] - and service_with_local_runtime not in skipped_services + and service_with_local_runtime not in local_runtime_dependency_names ): - skipped_services.add(service_with_local_runtime) + local_runtime_dependency_names.add(service_with_local_runtime) if args.exclude_local: status.warning( f"Skipping '{service_with_local_runtime}' as it is set to run locally" @@ -155,19 +155,19 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: for dep in remote_dependencies if dep.service_name not in services_with_local_runtime } + if not args.exclude_local and len(local_runtime_dependency_names) > 0: + status.warning("Starting dependencies with local runtimes...") + for local_runtime_dependency_name in local_runtime_dependency_names: + up( + Namespace( + service_name=local_runtime_dependency_name, + mode=mode, + exclude_local=True, + ), + status, + ) + status.warning(f"Continuing with service '{service.name}'") try: - if not args.exclude_local: - status.warning("Starting dependencies with local runtimes...") - for skipped_service in skipped_services: - up( - Namespace( - service_name=skipped_service, - mode=mode, - exclude_local=True, - ), - status, - ) - status.warning(f"Continuing with service '{service.name}'") _up(service, [mode], remote_dependencies, mode_dependencies, status) except DockerComposeError as dce: capture_exception(dce, level="info") diff --git a/tests/commands/test_up.py b/tests/commands/test_up.py index ea0126d1..1274671b 100644 --- a/tests/commands/test_up.py +++ b/tests/commands/test_up.py @@ -768,6 +768,9 @@ def test_up_mode_simple( mock_check_all_containers_healthy.assert_called_once() captured = capsys.readouterr() assert "Retrieving dependencies" in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert "Starting 'example-service' in mode: 'test'" in captured.out.strip() assert "Starting redis" in captured.out.strip() @@ -851,6 +854,9 @@ def test_up_mode_does_not_exist( captured = capsys.readouterr() assert "Retrieving dependencies" not in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert "Starting 'example-service' in mode: 'test'" not in captured.out.strip() assert "Starting clickhouse" not in captured.out.strip() assert "Starting redis" not in captured.out.strip() @@ -957,6 +963,9 @@ def test_up_multiple_modes( captured = capsys.readouterr() assert "Starting 'example-service' in mode: 'test'" in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert "Retrieving dependencies" in captured.out.strip() assert "Starting redis" in captured.out.strip() @@ -1122,6 +1131,9 @@ def test_up_multiple_modes_overlapping_running_service( captured = capsys.readouterr() assert "Starting 'example-service' in mode: 'test'" in captured.out.strip() assert "Retrieving dependencies" in captured.out.strip() + assert ( + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert "Starting clickhouse" in captured.out.strip() @@ -1494,6 +1506,10 @@ def test_up_dependency_set_to_local( captured = capsys.readouterr() if exclude_local: + assert ( + "Starting dependencies with local runtimes..." + not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert ( captured.out.strip().count( "Skipping 'local-runtime-service' as it is set to run locally" @@ -1501,6 +1517,12 @@ def test_up_dependency_set_to_local( == 1 ) else: + assert ( + captured.out.strip().count( + "Starting dependencies with local runtimes..." + ) + == 1 + ), "This should be printed since we have dependencies with local runtimes" assert ( captured.out.strip().count( "Skipping 'local-runtime-service' as it is set to run locally" @@ -1797,6 +1819,10 @@ def test_up_nested_dependency_set_to_local( captured = capsys.readouterr() if exclude_local: + assert ( + "Starting dependencies with local runtimes..." + not in captured.out.strip() + ), "This shouldn't be printed since we don't have any dependencies with local runtimes" assert ( captured.out.strip().count( "Skipping 'local-runtime-service' as it is set to run locally" @@ -1805,11 +1831,12 @@ def test_up_nested_dependency_set_to_local( ) else: assert ( - captured.out.strip().count( - "Skipping 'local-runtime-service' as it is set to run locally" - ) - == 0 - ) + "Starting dependencies with local runtimes..." in captured.out.strip() + ), "This should be printed since we have dependencies with local runtimes" + assert ( + "Skipping 'local-runtime-service' as it is set to run locally" + not in captured.out.strip() + ), "This shouldn't be printed since local-runtime-service is being brought up" @mock.patch("devservices.utils.state.State.update_service_entry") @@ -1991,11 +2018,9 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no captured = capsys.readouterr() assert ( - captured.out.strip().count( - "Skipping 'local-runtime-service' as it is set to run locally" - ) - == 0 - ) + "Skipping 'local-runtime-service' as it is set to run locally" + not in captured.out.strip() + ), "This shouldn't be printed since local-runtime-service is not being brought up" @mock.patch("devservices.utils.state.State.update_service_entry") @@ -2180,8 +2205,9 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta captured = capsys.readouterr() assert ( - captured.out.strip().count( - "Skipping 'local-runtime-service' as it is set to run locally" - ) - == 0 - ) + "Starting dependencies with local runtimes..." not in captured.out.strip() + ), "This shouldn't be printed since other-service isn't being brought up in a mode that includes local-runtime-service" + assert ( + "Skipping 'local-runtime-service' as it is set to run locally" + not in captured.out.strip() + ), "This shouldn't be printed since other-service isn't being brought up in a mode that includes local-runtime-service"