Repository navigation
Re-enable NSKIP015 - #863
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to re-enable two previously Nanvix-skipped large-resource regression tests by removing the Nanvix-specific NSKIP015 module skips and adding the modules to Nanvix’s passing-test allowlist.
Changes:
- Removed Nanvix-only module-level
SkipTestguards fromtest_largefileandtest_zipfile64. - Added
test_largefileandtest_zipfile64to.nanvix’sNANVIX_TEST_LISTso they are included in the Nanvix test run selection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Lib/test/test_zipfile64.py | Removes Nanvix skip guard; now relies on existing extralargefile resource gating (and needs import cleanup). |
| Lib/test/test_largefile.py | Removes Nanvix skip guard; currently not resource-gated on non-win/darwin, so it can execute by default on Nanvix. |
| .nanvix/src/config.py | Adds the two modules to NANVIX_TEST_LIST (modules “known to pass on Nanvix”). |
Suppressed comments (1)
Lib/test/test_zipfile64.py:17
- After dropping the Nanvix-specific SkipTest,
is_nanvixis now unused andunittestis imported twice (here and again later). This will trip basic linters and makes the import section harder to read/maintain.
from test.support import is_nanvix
import unittest
# Tests of the full ZIP64 functionality of zipfile
# The support.requires call is the only reason for keeping this separate
# from test_zipfile
from test import support
# XXX(nnorwitz): disable this test by looking for extralargefile resource,
# which doesn't exist. This test takes over 30 minutes to run in general
# and requires more disk space than most of the buildbots.
support.requires(
'extralargefile',
'test requires loads of disk-space bytes and a long time to run'
)
import zipfile, unittest
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from test.support import is_nanvix | ||
| import unittest | ||
| if is_nanvix: | ||
| raise unittest.SkipTest("NSKIP015: OOM — test allocation exceeds available heap") # detail: test requires >2 GB disk I/O | ||
|
|
||
| import os |
| "test_shutil", | ||
| "test_largefile", | ||
| "test_zipfile64", | ||
| "test_zipfile", |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.nanvix/src/config.py:196
- Adding
test_largefileto the Nanvix test manifest will now attempt to run/import it in the Nanvix CI environment. Given the module's >2 GiB file creation and the removal of the Nanvix-specific skip in this PR, this is likely to trigger the same NSKIP015 resource failures the issue describes.
If this PR is relying on resource-gating to skip these tests, test_largefile still isn’t reliably gated on non-Windows/macOS platforms, so it should remain out of the Nanvix manifest until there is an explicit skip/resource gate for Nanvix or a larger VM configuration is available.
"test_largefile",
"test_zipfile64",
Lib/test/test_largefile.py:6
- Removing the Nanvix module-level skip re-enables this test suite on Nanvix, but the module still unconditionally creates and operates on a >2 GiB file (e.g.,
size = 2_500_000_000andLargeFileTest.setUp()), and it is not guarded byrequires('largefile')on non-Windows/macOS platforms. This contradicts the PR description that regrtest's largefile resource gate will skip it, and is likely to reintroduce the NSKIP015 OOM/disk exhaustion failures on the current Nanvix VM.
To keep the suite runnable on Nanvix with current resource limits, restore the explicit Nanvix skip guard (or add an equivalent Nanvix-specific resource gate).
from test.support import is_nanvix
import unittest
Lib/test/test_zipfile64.py:2
is_nanvixis no longer referenced after removing the Nanvix-only SkipTest, andunittestis imported twice (here and again inimport zipfile, unittest). This leaves unused/redundant imports at module import time.
from test.support import is_nanvix
import unittest
Redundant: regrtest's largefile resource gate already skips these.
There was a problem hiding this comment.
Review details
Suppressed comments (3)
Lib/test/test_largefile.py:6
- Removing the Nanvix skip causes this module to run under Nanvix’s default regrtest resource settings (no -u largefile), but the test’s setup creates a >2.5GB file on non-win/darwin platforms. This contradicts the PR rationale that regrtest’s largefile resource gate already skips it, and is likely to fail/hang in the constrained Nanvix VM/disk environment.
from test.support import is_nanvix
import unittest
.nanvix/src/config.py:196
- These two modules are being added to the Nanvix default test list, but they are not runnable in the default standalone configuration: test_largefile will attempt to create a multi-gigabyte file (no resource gate on Nanvix), and test_zipfile64 is guarded by the extralargefile resource (disabled by default). Keeping them off the default list avoids predictable failures/timeouts in CI until a larger VM/disk configuration is introduced.
"test_largefile",
"test_zipfile64",
Lib/test/test_zipfile64.py:2
- After removing the Nanvix-specific SkipTest,
is_nanvixand the earlyunittestimport are now unused, andunittestis imported twice. This is dead code in the module header and makes the imports harder to follow.
from test.support import is_nanvix
import unittest
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Closes #614
What
Add test_largefile / test_zipfile64 to the test list; drop the NSKIP015 module skip.
Why
Redundant: regrtest's largefile resource gate already skips these.
Verified the full lifecycle on Linux.