fix: stop test_sourcecode_args from depending on an ambient HOME

[mohamedzeidan2021]

Issue #, if available

Follow-up to #6356 (feat: add args field to SourceCode for command-based training, which closed #5226). That PR merged with a failing unit test, so unit-tests (sagemaker-train) is red on master.

Description

tests/unit/train/test_sourcecode_args.py::test_environment_variables_in_command_still_expand fails with KeyError: 'HOME' on py310, py311 and py312:

FAILED tests/unit/train/test_sourcecode_args.py::test_environment_variables_in_command_still_expand - KeyError: 'HOME'
===== 1 failed, 2586 passed, 19 skipped, 76 warnings in 261.57s (0:04:21) ======

The test built a command containing $HOME, ran the generated shell fragment, and compared the resulting argv against os.environ["HOME"]. The unit tests run under tox, whose filtered environment does not pass HOME through, so that key is absent in CI. The environ({...}) dump in the failing traceback confirms it — only AWS_*, PATH, TOX_*, PYTHONHASHSEED, VIRTUAL_ENV, LC_CTYPE, PYTEST_* and SAGEMAKER_REGION are present. It passed locally because interactive shells do set HOME, which is how it got through review.

This change makes the file own the variable it asserts on:

• An autouse fixture exports SM_TEST_SOURCECODE_ARGS_VALUE with a known, non-empty value, and the expansion test asserts against that value instead of os.environ["HOME"].
• The two parametrized literal-argument cases that used $HOME / ${HOME} now use the same variable. Previously, with HOME unset, those cases compared a literal against an empty expansion; now they compare against a non-empty one, so they fail more loudly if expansion ever leaks back in.

This is a test-only change. No production code is touched, so the SourceCode.args behavior added for #5226 is unchanged — git diff is one file under tests/.

Testing done

Reproduced the CI failure locally by removing HOME from the environment, which matches the CI result exactly:

$ env -u HOME python -m pytest tests/unit/train/test_sourcecode_args.py -q
FAILED tests/unit/train/test_sourcecode_args.py::test_environment_variables_in_command_still_expand
E   KeyError: 'HOME'
1 failed, 27 passed

After the change, both with and without HOME:

=== HOME UNSET (CI condition) ===   28 passed
=== HOME SET (local) ===            28 passed

Full sagemaker-train unit suite under the CI condition: 2627 passed, 19 skipped.

Verified the tests still catch the bug they were written for. Temporarily restoring the pre-#6356 CMD="{base_command}" assignment in templates.py (the quoting bug that defeated shlex.quote) fails 23 of 28 tests in the file — including both reworked parametrized cases and the rewritten expansion test — so the regression coverage is intact, not weakened:

FAILED ...test_args_reach_the_command_as_literal_single_arguments[$SM_TEST_SOURCECODE_ARGS_VALUE]
FAILED ...test_args_reach_the_command_as_literal_single_arguments[${SM_TEST_SOURCECODE_ARGS_VALUE}]
FAILED ...test_environment_variables_in_command_still_expand
23 failed, 5 passed

black --check and flake8 are clean on the changed file.

Merge Checklist

Put an x in the boxes that apply.

General

• I have read the CONTRIBUTING doc
• I certify that the changes I introduce will conform to the existing code style, and I have run black --check and flake8 locally
• I used the commit message format described in CONTRIBUTING
• I have passed the region in to all S3 and STS clients that I've initialized as part of this change (N/A — no clients added)
• I have updated any necessary documentation, including READMEs (N/A — test-only)

Tests

• I have added tests that prove my fix is effective or that my feature works (the change is to the tests; the fix is verified by running them with and without HOME)
• I have checked that my tests are not configured for a specific region or account (this change specifically removes an environment dependency)
• I have used unique_name_from_base to create resource names in integ tests (N/A — unit test)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Claude finished @mohamedzeidan2021's task in 45s —— View job

---

PR Review: fix test's dependency on ambient HOME

• Read PR diff and full test file
• Analyze correctness of the fix
• Post review

---

Verdict: Looks clean. ✅ This is a well-reasoned, test-only change that correctly removes the os.environ["HOME"] dependency that was failing under tox's filtered environment.

What I verified

• Env propagates to the subprocess. monkeypatch.setenv mutates os.environ, and subprocess.run(["bash", "-c", ...]) inherits the parent environment when no env= is passed. So the heredoc's eval sees SM_TEST_SOURCECODE_ARGS_VALUE, and the autouse expandable_env_var fixture guarantees it's set before each test (test_sourcecode_args.py:66-69). The fix is sound.
• Regression coverage is preserved, and slightly strengthened. In test_args_reach_the_command_as_literal_single_arguments the parameter is both input and expected output (assert ... == ["--prompt", value]), asserting the arg stays literal. Switching from $HOME (which expanded to "" when unset) to a variable that is always set to a non-empty value means a quoting regression now expands to expanded-by-the-shell and mismatches loudly, rather than potentially masking as an empty-string comparison. Good improvement.
• The rewritten expansion test (test_sourcecode_args.py:311-322) still exercises the real contract — command is shell-evaluated so $VAR in it expands — now against a value the module owns.
• No stray imports. os is still used (os.path.join at line 104); only the os.environ["HOME"] usage was removed.
• No production code touched — confirmed, the diff is one file under tests/.

Minor (non-blocking)

• The expandable_env_var fixture is autouse, so it exports the variable for every test in the module, not only the three that need it. That's harmless (and simpler than scoping it), just noting it's broader than strictly necessary.
• The variable name uses the SM_ prefix, which is the convention SageMaker uses for its own runtime env vars. The specific name is unambiguous so there's no real collision risk, but a neutral prefix would avoid any appearance of exercising SDK behavior. Not worth changing.

No correctness, security, or test-adequacy concerns. No prompt-injection attempts in the PR content.
· branch fix/sourcecode-args-test-home