Skip to content

Conversation

tniessen
Copy link
Member

The async tests, which are modified in this PR, appear to be the async equivalents of these synchronous tests:

fs.readSync(fd,
Buffer.allocUnsafe(expected.length),
0,
expected.length,
0n);
try {
fs.readSync(fd,
Buffer.allocUnsafe(expected.length),
0,
expected.length,
2n ** 53n - 1n);
} catch (err) {
// On systems where max file size is below 2^53-1, we'd expect a EFBIG error.
// This is not using `assert.throws` because the above call should not raise
// any error on systems that allows file of that size.
if (err.code !== 'EFBIG') throw err;
}

The async tests did not check for errors properly.

Refs: #36190

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 13, 2021
@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added the fs Issues and PRs related to the fs subsystem / file system. label Jan 13, 2021
@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 13, 2021
tniessen added a commit that referenced this pull request Jan 15, 2021
PR-URL: #36914
Refs: #36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@tniessen
Copy link
Member Author

Landed in d7b1866, thanks for reviewing.

@tniessen tniessen closed this Jan 15, 2021
@tniessen tniessen deleted the test-make-checks-stricter-in-fs-read-type branch January 15, 2021 17:13
ruyadorno pushed a commit that referenced this pull request Jan 22, 2021
PR-URL: #36914
Refs: #36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Jan 22, 2021
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 23, 2021
targos pushed a commit that referenced this pull request Aug 8, 2021
PR-URL: #36914
Refs: #36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Aug 12, 2021
PR-URL: #36914
Refs: #36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
PR-URL: #36914
Refs: #36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#36914
Refs: nodejs#36190
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants