Skip to content

Commit 593aaad

Browse files
mohamedzeidan2021Mohamed Zeidan
andauthored
fix: stop test_sourcecode_args from depending on an ambient HOME (#6383)
test_environment_variables_in_command_still_expand compared the shell-expanded command against os.environ["HOME"], which fails with KeyError: 'HOME' on every Python version in CI. The unit tests run under tox, whose filtered environment does not pass HOME through, so the variable the test reads is simply absent. Export a variable the module owns instead, via an autouse fixture, and assert against its known value. The parametrized literal-argument cases that used $HOME/${HOME} now use the same variable, so they compare a literal against a non-empty expansion rather than against whatever the ambient environment happens to hold. Test-only change: the SourceCode.args behavior added for #5226 is untouched, and reintroducing the original double-quoted CMD= assignment still fails 23 of the 28 tests in this file, including both reworked cases. Co-authored-by: Mohamed Zeidan <zeidmo@amazon.com>
1 parent 193df0f commit 593aaad

1 file changed

Lines changed: 17 additions & 4 deletions

File tree

‎sagemaker-train/tests/unit/train/test_sourcecode_args.py‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@
5555
max_wait_time_in_seconds=None,
5656
)
5757

58+
# The shell-expansion assertions below need a variable that is guaranteed to be exported with a
59+
# known, non-empty value, so this module sets one itself rather than reading the ambient
60+
# environment. CI runs these tests under tox, whose filtered environment does not pass HOME
61+
# through, so os.environ["HOME"] is not available here.
62+
EXPANDABLE_VAR = "SM_TEST_SOURCECODE_ARGS_VALUE"
63+
EXPANDABLE_VAR_VALUE = "expanded-by-the-shell"
64+
65+
66+
@pytest.fixture(autouse=True)
67+
def expandable_env_var(monkeypatch):
68+
"""Export a known variable for the tests that run the generated command block."""
69+
monkeypatch.setenv(EXPANDABLE_VAR, EXPANDABLE_VAR_VALUE)
70+
5871

5972
@pytest.fixture(autouse=True)
6073
def modules_session():
@@ -237,8 +250,8 @@ def test_prepare_train_script_command_only_no_regression():
237250
"value",
238251
[
239252
"hello world",
240-
"$HOME",
241-
"${HOME}",
253+
"$" + EXPANDABLE_VAR,
254+
"${" + EXPANDABLE_VAR + "}",
242255
"$(id -u)",
243256
"`id -u`",
244257
'a"b',
@@ -301,12 +314,12 @@ def test_environment_variables_in_command_still_expand():
301314
The heredoc stops expansion at *assignment* time only; ``eval`` still parses the command
302315
once, which is the behavior users of ``command`` rely on.
303316
"""
304-
source_code = SourceCode(source_dir="scripts", command="capture_args $HOME")
317+
source_code = SourceCode(source_dir="scripts", command=f"capture_args ${EXPANDABLE_VAR}")
305318
trainer = _make_trainer(source_code)
306319
script = _written_train_script(trainer, source_code)
307320

308321
argv = _argv_from_running_base_command(script)
309-
assert argv == [os.environ["HOME"]]
322+
assert argv == [EXPANDABLE_VAR_VALUE]
310323

311324

312325
@requires_bash

0 commit comments

Comments
 (0)