Skip to content

Commit 5ec0b43

Browse files
committed
fix: address review comments (iteration #3)
1 parent e43c661 commit 5ec0b43

File tree

1 file changed

+21
-28
lines changed

1 file changed

+21
-28
lines changed

sagemaker-train/tests/unit/train/local/test_local_container.py

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,12 @@ def _basic_channel():
9696
return Channel(channel_name="training", data_source=data_source)
9797

9898

99-
@pytest.fixture
100-
def _mock_session():
101-
"""Create a mock SageMaker session."""
102-
session = Mock()
103-
session.boto_region_name = "us-west-2"
104-
session.boto_session = Mock()
105-
session.s3_resource = Mock()
106-
session.s3_resource.meta.client._endpoint.host = "https://s3.us-west-2.amazonaws.com"
107-
return session
108-
109-
110-
def _make_container(_mock_session, _basic_channel):
111-
"""Helper to create a _LocalContainer for compose prefix tests."""
99+
def _make_container(_basic_channel):
100+
"""Helper to create a _LocalContainer for compose prefix tests.
101+
102+
sagemaker_session is set to None because _get_compose_cmd_prefix does not
103+
use it, and Pydantic validation rejects Mock objects for the Session type.
104+
"""
112105
return _LocalContainer(
113106
training_job_name="test-job",
114107
instance_type="local",
@@ -120,44 +113,44 @@ def _make_container(_mock_session, _basic_channel):
120113
hyper_parameters={},
121114
container_entrypoint=[],
122115
container_arguments=[],
123-
sagemaker_session=_mock_session,
116+
sagemaker_session=None,
124117
)
125118

126119

127120
class TestGetComposeCmdPrefix:
128121
"""Test cases for _get_compose_cmd_prefix version detection."""
129122

130123
@patch("sagemaker.train.local.local_container.subprocess.check_output")
131-
def test_get_compose_cmd_prefix_with_docker_compose_v2(self, mock_check_output, _mock_session, _basic_channel):
124+
def test_get_compose_cmd_prefix_with_docker_compose_v2(self, mock_check_output, _basic_channel):
132125
"""Docker Compose v2 should be accepted."""
133-
container = _make_container(_mock_session, _basic_channel)
126+
container = _make_container(_basic_channel)
134127
mock_check_output.return_value = "Docker Compose version v2.20.0"
135128
result = container._get_compose_cmd_prefix()
136129
assert result == ["docker", "compose"]
137130

138131
@patch("sagemaker.train.local.local_container.subprocess.check_output")
139-
def test_get_compose_cmd_prefix_with_docker_compose_v5(self, mock_check_output, _mock_session, _basic_channel):
132+
def test_get_compose_cmd_prefix_with_docker_compose_v5(self, mock_check_output, _basic_channel):
140133
"""Docker Compose v5 should be accepted."""
141-
container = _make_container(_mock_session, _basic_channel)
134+
container = _make_container(_basic_channel)
142135
mock_check_output.return_value = "Docker Compose version v5.1.1"
143136
result = container._get_compose_cmd_prefix()
144137
assert result == ["docker", "compose"]
145138

146139
@patch("sagemaker.train.local.local_container.subprocess.check_output")
147-
def test_get_compose_cmd_prefix_with_docker_compose_v3(self, mock_check_output, _mock_session, _basic_channel):
140+
def test_get_compose_cmd_prefix_with_docker_compose_v3(self, mock_check_output, _basic_channel):
148141
"""Docker Compose v3 should be accepted."""
149-
container = _make_container(_mock_session, _basic_channel)
142+
container = _make_container(_basic_channel)
150143
mock_check_output.return_value = "Docker Compose version v3.0.0"
151144
result = container._get_compose_cmd_prefix()
152145
assert result == ["docker", "compose"]
153146

154147
@patch("sagemaker.train.local.local_container.shutil.which")
155148
@patch("sagemaker.train.local.local_container.subprocess.check_output")
156149
def test_get_compose_cmd_prefix_with_docker_compose_v1_falls_through(
157-
self, mock_check_output, mock_which, _mock_session, _basic_channel
150+
self, mock_check_output, mock_which, _basic_channel
158151
):
159152
"""Docker Compose v1 should not be accepted; falls through to docker-compose standalone."""
160-
container = _make_container(_mock_session, _basic_channel)
153+
container = _make_container(_basic_channel)
161154
mock_check_output.return_value = "docker-compose version 1.29.2"
162155
mock_which.return_value = "/usr/bin/docker-compose"
163156
result = container._get_compose_cmd_prefix()
@@ -166,10 +159,10 @@ def test_get_compose_cmd_prefix_with_docker_compose_v1_falls_through(
166159
@patch("sagemaker.train.local.local_container.shutil.which")
167160
@patch("sagemaker.train.local.local_container.subprocess.check_output")
168161
def test_get_compose_cmd_prefix_with_docker_compose_v1_no_standalone_raises(
169-
self, mock_check_output, mock_which, _mock_session, _basic_channel
162+
self, mock_check_output, mock_which, _basic_channel
170163
):
171164
"""Docker Compose v1 with no standalone fallback should raise ImportError."""
172-
container = _make_container(_mock_session, _basic_channel)
165+
container = _make_container(_basic_channel)
173166
mock_check_output.return_value = "docker-compose version v1.29.2"
174167
mock_which.return_value = None
175168
with pytest.raises(ImportError, match="Docker Compose is not installed"):
@@ -178,10 +171,10 @@ def test_get_compose_cmd_prefix_with_docker_compose_v1_no_standalone_raises(
178171
@patch("sagemaker.train.local.local_container.shutil.which")
179172
@patch("sagemaker.train.local.local_container.subprocess.check_output")
180173
def test_get_compose_cmd_prefix_not_installed_raises(
181-
self, mock_check_output, mock_which, _mock_session, _basic_channel
174+
self, mock_check_output, mock_which, _basic_channel
182175
):
183176
"""When docker compose is not installed at all, should raise ImportError."""
184-
container = _make_container(_mock_session, _basic_channel)
177+
container = _make_container(_basic_channel)
185178
mock_check_output.side_effect = subprocess.CalledProcessError(1, "cmd")
186179
mock_which.return_value = None
187180
with pytest.raises(ImportError, match="Docker Compose is not installed"):
@@ -190,10 +183,10 @@ def test_get_compose_cmd_prefix_not_installed_raises(
190183
@patch("sagemaker.train.local.local_container.shutil.which")
191184
@patch("sagemaker.train.local.local_container.subprocess.check_output")
192185
def test_get_compose_cmd_prefix_standalone_fallback(
193-
self, mock_check_output, mock_which, _mock_session, _basic_channel
186+
self, mock_check_output, mock_which, _basic_channel
194187
):
195188
"""When docker compose plugin fails, falls back to docker-compose standalone."""
196-
container = _make_container(_mock_session, _basic_channel)
189+
container = _make_container(_basic_channel)
197190
mock_check_output.side_effect = subprocess.CalledProcessError(1, "cmd")
198191
mock_which.return_value = "/usr/local/bin/docker-compose"
199192
result = container._get_compose_cmd_prefix()

0 commit comments

Comments
 (0)