Repository navigation
Conversation
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. Caution AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context. |
sxa
left a comment
There was a problem hiding this comment.
That was quick :-)
Also tagging @christianaurichzm who put the original test in in case he wants to look at this fix.
christianaurichzm
left a comment
There was a problem hiding this comment.
LGTM, thanks. /proc/<pid>/io only exists when the kernel is built with CONFIG_TASK_IO_ACCOUNTING, so checking /proc/self/io covers the child as well.
| const { spawn } = require('child_process'); | ||
| const fs = require('fs'); | ||
|
|
||
| if (!fs.existsSync('/proc/self/io')) |
There was a problem hiding this comment.
Nit: with this check the common.isLinux one above is redundant, since /proc/self/io doesn't exist outside Linux. Both could be folded into one by moving the fs require up:
const common = require('../common');
const fs = require('fs');
if (!fs.existsSync('/proc/self/io'))
common.skip('Paces the writes with /proc/<pid>/io.');There was a problem hiding this comment.
I agree that this would be preferable. if the author of this (who I am inclined to suggest may be an AI bot given that they have sometimes sent hundreds of github events each day) does not respond I will create a new PR with that change.
There was a problem hiding this comment.
Creating a separate PR is probably the cleanest way forward.
The submitter also did not respond to my comments / question and they do appear to be ignoring the rules that ban automated submissions and guidelines that require submissions to be pre-tested.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #67242 +/- ##
==========================================
+ Coverage 90.68% 90.69% +0.01%
==========================================
Files 797 797
Lines 279367 279611 +244
Branches 53737 53784 +47
==========================================
+ Hits 253344 253602 +258
+ Misses 16319 16300 -19
- Partials 9704 9709 +5 🚀 New features to boost your workflow:
|
Since you're a first-time contributor you may have missed the information in the Pull Request template:
Also, Pull requests > Step 6: Test
with further details under BUILDING > Running tests. Do you have a Linux/riscv64 system available that you can test on? |
I've already ran the whole test suite on Linux/riscv64 from this PR and it works as it should :-) |
Assisted-by: Codex Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
|
@sxa - I overlooked that Jenkins needs a new approval before it will run via |
The readFileSync short reads regression test depends on /proc//io, which is not present on some Linux systems. It now skips when /proc/self/io is unavailable.
Verification: git diff --check passed. python tools/test.py --mode=release parallel/test-fs-readfilesync-pipe-short-reads could not run because this checkout has no built Release/node.exe.
Regression test: untested: CONTRIBUTING does not mention tests
Fixes #67241