Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(node/fs): add test-fs-read-stream.js #2694

Merged
merged 1 commit into from
Sep 24, 2022

Conversation

PolarETech
Copy link
Contributor

This PR adds parallel/test-fs-read-stream.js, which was waiting for child_process.spawnSync() and child_process.exec() implementation in #2653.

Comment on lines +200 to +209
if (!common.isWindows) {
// Verify that end works when start is not specified, and we do not try to
// use positioned reads. This makes sure that this keeps working for
// non-seekable file descriptors.
tmpdir.refresh();
const filename = `${tmpdir.path}/foo.pipe`;
const mkfifoResult = child_process.spawnSync('mkfifo', [filename]);
if (!mkfifoResult.error) {
child_process.exec(`echo "xyz foobar" > '${filename}'`);
const stream = new fs.createReadStream(filename, common.mustNotMutateObjectDeep({ end: 1 }));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test item which uses child_process will not be executed on Windows. Therefore, there is no need to add this test to "windowsIgnore" in config.json, as the test will not fail due to escaping issue on Windows.

@PolarETech PolarETech marked this pull request as ready for review September 24, 2022 10:57
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice!

@kt3k kt3k merged commit 5fb2569 into denoland:main Sep 24, 2022
@PolarETech PolarETech deleted the test-fs-read-stream branch September 25, 2022 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants