test: reduce flakiness of test-fs-read-position-validation.mjs

PR-URL: nodejs#42999 Reviewed-By: Antoine du Hamel <[email protected]> Backport-PR-URL: nodejs#43588
LiviaMedeiros · Jun 27, 2022 · ceee7be · ceee7be
1 parent 1656e1b
commit ceee7be
Show file tree

Hide file tree

Showing 2 changed files with 102 additions and 66 deletions.
diff --git a/test/parallel/test-fs-read-position-validation.mjs b/test/parallel/test-fs-read-position-validation.mjs
@@ -14,73 +14,26 @@ const length = buffer.byteLength;
 // allowedErrors is an array of acceptable internal errors
 // For example, on some platforms read syscall might return -EFBIG
 async function testValid(position, allowedErrors = []) {
-  let fdSync;
-  try {
-    fdSync = fs.openSync(filepath, 'r');
-    fs.readSync(fdSync, buffer, offset, length, position);
-    fs.readSync(fdSync, buffer, { offset, length, position });
-  } catch (err) {
-    if (!allowedErrors.includes(err.code)) {
-      assert.fail(err);
-    }
-  } finally {
-    if (fdSync) fs.closeSync(fdSync);
-  }
-
-  fs.open(filepath, 'r', common.mustSucceed((fd) => {
-    try {
-      if (allowedErrors.length) {
-        fs.read(fd, buffer, offset, length, position, common.mustCall((err, ...results) => {
-          if (err && !allowedErrors.includes(err.code)) {
-            assert.fail(err);
-          }
-        }));
-        fs.read(fd, { buffer, offset, length, position }, common.mustCall((err, ...results) => {
-          if (err && !allowedErrors.includes(err.code)) {
-            assert.fail(err);
-          }
-        }));
-      } else {
-        fs.read(fd, buffer, offset, length, position, common.mustSucceed());
-        fs.read(fd, { buffer, offset, length, position }, common.mustSucceed());
-      }
-    } finally {
-      fs.close(fd, common.mustSucceed());
-    }
-  }));
-}
-
-async function testInvalid(code, position, internalCatch = false) {
-  let fdSync;
-  try {
-    fdSync = fs.openSync(filepath, 'r');
-    assert.throws(
-      () => fs.readSync(fdSync, buffer, offset, length, position),
-      { code }
-    );
-    assert.throws(
-      () => fs.readSync(fdSync, buffer, { offset, length, position }),
-      { code }
-    );
-  } finally {
-    if (fdSync) fs.closeSync(fdSync);
-  }
-
-  // Set this flag for catching errors via first argument of callback function
-  if (internalCatch) {
+  return new Promise((resolve, reject) => {
     fs.open(filepath, 'r', common.mustSucceed((fd) => {
-      try {
-        fs.read(fd, buffer, offset, length, position, (err, ...results) => {
-          assert.strictEqual(err.code, code);
-        });
-        fs.read(fd, { buffer, offset, length, position }, (err, ...results) => {
-          assert.strictEqual(err.code, code);
-        });
-      } finally {
-        fs.close(fd, common.mustSucceed());
-      }
+      let callCount = 2;
+      const handler = common.mustCall((err) => {
+        callCount--;
+        if (err && !allowedErrors.includes(err.code)) {
+          fs.close(fd, common.mustSucceed());
+          reject(err);
+        } else if (callCount === 0) {
+          fs.close(fd, common.mustSucceed(resolve));
+        }
+      }, callCount);
+      fs.read(fd, buffer, offset, length, position, handler);
+      fs.read(fd, { buffer, offset, length, position }, handler);
     }));
-  } else {
+  });
+}
+
+async function testInvalid(code, position) {
+  return new Promise((resolve, reject) => {
     fs.open(filepath, 'r', common.mustSucceed((fd) => {
       try {
         assert.throws(
@@ -91,11 +44,14 @@ async function testInvalid(code, position, internalCatch = false) {
           () => fs.read(fd, { buffer, offset, length, position }, common.mustNotCall()),
           { code }
         );
+        resolve();
+      } catch (err) {
+        reject(err);
       } finally {
         fs.close(fd, common.mustSucceed());
       }
     }));
-  }
+  });
 }
 
 {

diff --git a/test/parallel/test-fs-readSync-position-validation.mjs b/test/parallel/test-fs-readSync-position-validation.mjs
@@ -0,0 +1,80 @@
+import '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import fs from 'fs';
+import assert from 'assert';
+
+// This test ensures that "position" argument is correctly validated
+
+const filepath = fixtures.path('x.txt');
+
+const buffer = Buffer.from('xyz\n');
+const offset = 0;
+const length = buffer.byteLength;
+
+// allowedErrors is an array of acceptable internal errors
+// For example, on some platforms read syscall might return -EFBIG
+function testValid(position, allowedErrors = []) {
+  let fdSync;
+  try {
+    fdSync = fs.openSync(filepath, 'r');
+    fs.readSync(fdSync, buffer, offset, length, position);
+    fs.readSync(fdSync, buffer, { offset, length, position });
+  } catch (err) {
+    if (!allowedErrors.includes(err.code)) {
+      assert.fail(err);
+    }
+  } finally {
+    if (fdSync) fs.closeSync(fdSync);
+  }
+}
+
+function testInvalid(code, position, internalCatch = false) {
+  let fdSync;
+  try {
+    fdSync = fs.openSync(filepath, 'r');
+    assert.throws(
+      () => fs.readSync(fdSync, buffer, offset, length, position),
+      { code }
+    );
+    assert.throws(
+      () => fs.readSync(fdSync, buffer, { offset, length, position }),
+      { code }
+    );
+  } finally {
+    if (fdSync) fs.closeSync(fdSync);
+  }
+}
+
+{
+  testValid(undefined);
+  testValid(null);
+  testValid(-1);
+  testValid(-1n);
+
+  testValid(0);
+  testValid(0n);
+  testValid(1);
+  testValid(1n);
+  testValid(9);
+  testValid(9n);
+  testValid(Number.MAX_SAFE_INTEGER, [ 'EFBIG' ]);
+
+  testValid(2n ** 63n - 1n - BigInt(length), [ 'EFBIG' ]);
+  testInvalid('ERR_OUT_OF_RANGE', 2n ** 63n);
+
+  // TODO(LiviaMedeiros): test `2n ** 63n - BigInt(length)`
+
+  testInvalid('ERR_OUT_OF_RANGE', NaN);
+  testInvalid('ERR_OUT_OF_RANGE', -Infinity);
+  testInvalid('ERR_OUT_OF_RANGE', Infinity);
+  testInvalid('ERR_OUT_OF_RANGE', -0.999);
+  testInvalid('ERR_OUT_OF_RANGE', -(2n ** 64n));
+  testInvalid('ERR_OUT_OF_RANGE', Number.MAX_SAFE_INTEGER + 1);
+  testInvalid('ERR_OUT_OF_RANGE', Number.MAX_VALUE);
+
+  for (const badTypeValue of [
+    false, true, '1', Symbol(1), {}, [], () => {}, Promise.resolve(1),
+  ]) {
+    testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
+  }
+}