Re-enable pathlib home expansion test - #897
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores Nanvix coverage for pathlib home-directory expansion by ensuring the Nanvix test harness provides a writable HOME environment variable, allowing the previously-skipped test_expanduser_common to run. It also updates the Nanvix documentation examples so the documented guest environment matches the harness behavior.
Changes:
- Re-enabled
Lib/test/test_pathlib.py::test_expanduser_commonon Nanvix by removing the Nanvix skip (NSKIP025). - Set
HOME=/tmpin the Nanvix test harness environment alongsideTMPDIR=/tmp. - Updated
.nanvix/NANVIX.mdguest invocation examples to includeHOME=/tmp.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Lib/test/test_pathlib.py | Removes the Nanvix-specific skip so the expanduser test runs when HOME is provided by the harness. |
| .nanvix/src/run-tests.py | Injects HOME=/tmp into the guest environment so os.path.expanduser('~') avoids pwd dependency and behaves predictably. |
| .nanvix/NANVIX.md | Keeps the documented guest invocation environment consistent by adding HOME=/tmp to examples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
53ea184 to
d5647fb
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The Nanvix docs’ example env strings add HOME but still omit TMPDIR=/tmp, so they don’t fully match the harness environment being described.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.nanvix/NANVIX.md:220
- This one-shot invocation example sets HOME but not TMPDIR, whereas the test harness uses TMPDIR=/tmp too. Add TMPDIR here for consistency (and to match the documented
<ENV1=val ENV2=val ...>format).
"-B ./test_hello.py;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_"
.nanvix/NANVIX.md:231
- The single-module test invocation example now sets HOME but still omits TMPDIR, which the harness sets to /tmp. Consider adding TMPDIR=/tmp so the documented invocation matches what
run-tests.pyruns.
"-B -m test --verbose test_int;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp NANVIX_STANDALONE=1 _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_"
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| ./bin/nanvixd.elf -bin-dir ./bin -ramfs ./cpython-rootfs.img \ | ||
| -- ./bin/python3.12 \ | ||
| "-i;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_" | ||
| "-i;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_" |
d5647fb to
fcdf016
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The updated Nanvix documentation examples still don’t fully match the harness environment (notably TMPDIR=/tmp), which conflicts with the PR’s stated goal of keeping invocations consistent.
Review details
Suppressed comments (3)
.nanvix/NANVIX.md:231
- For consistency with the harness environment, include TMPDIR=/tmp alongside HOME=/tmp in this invocation too (the harness passes both).
"-B -m test --verbose test_int;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp NANVIX_STANDALONE=1 _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_"
.nanvix/NANVIX.md:196
- The docs say guest invocations are kept consistent with the test harness, but the harness also sets TMPDIR=/tmp (see .nanvix/src/run-tests.py app_env). Consider adding TMPDIR here as well so the example matches the harness and tempfile-related behavior.
"-i;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_"
.nanvix/NANVIX.md:220
- This example still omits TMPDIR=/tmp even though the harness passes it; adding it would keep the documented env list aligned with how tests are executed and make tempfile behavior reproducible.
"-B ./test_hello.py;PYTHONHOME=/ PYTHONDONTWRITEBYTECODE=1 HOME=/tmp _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__nanvix_"
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Set a writable
HOMEin the Nanvix test harness and re-enable the NSKIP025 pathlib test. Keep the documented guest invocations consistent with the harness.Closes #505
Testing
Verified against the full lifecycle.