test(node/fs): enable fs.appendFile native tests

node/_fs/_fs_appendFile.ts

@@ -1,122 +1,56 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import {
   CallbackWithError,
-  getOpenOptions,
-  isFileOptions,
+  isFd,
+  maybeCallback,
   WriteFileOptions,
 } from "./_fs_common.ts";
 import { Encodings } from "../_utils.ts";
-import { fromFileUrl } from "../path.ts";
+import { copyObject, getOptions } from "../internal/fs/utils.js";
+import { writeFile, writeFileSync } from "./_fs_writeFile.ts";
 
 /**
  * TODO: Also accept 'data' parameter as a Node polyfill Buffer type once these
  * are implemented. See https://github.com/denoland/deno/issues/3403
  */
 export function appendFile(
-  pathOrRid: string | number | URL,
+  path: string | number | URL,
   data: string | Uint8Array,
-  optionsOrCallback: Encodings | WriteFileOptions | CallbackWithError,
+  options: Encodings | WriteFileOptions | CallbackWithError,
   callback?: CallbackWithError,
 ): void {
-  pathOrRid = pathOrRid instanceof URL ? fromFileUrl(pathOrRid) : pathOrRid;
-  const callbackFn: CallbackWithError | undefined =
-    optionsOrCallback instanceof Function ? optionsOrCallback : callback;
-  const options: Encodings | WriteFileOptions | undefined =
-    optionsOrCallback instanceof Function ? undefined : optionsOrCallback;
-  if (!callbackFn) {
-    throw new Error("No callback function supplied");
-  }
-
-  validateEncoding(options);
-  let rid = -1;
-  const buffer: Uint8Array = data instanceof Uint8Array
-    ? data
-    : new TextEncoder().encode(data);
-  new Promise((resolve, reject) => {
-    if (typeof pathOrRid === "number") {
-      rid = pathOrRid;
-      Deno.write(rid, buffer).then(resolve, reject);
-    } else {
-      const mode: number | undefined = isFileOptions(options)
-        ? options.mode
-        : undefined;
-      const flag: string | undefined = isFileOptions(options)
-        ? options.flag
-        : undefined;
+  callback = maybeCallback(callback || options);
+  options = getOptions(options, { encoding: "utf8", mode: 0o666, flag: "a" });
 
-      Deno.open(pathOrRid as string, { ...getOpenOptions(flag), mode })
-        .then(({ rid: openedFileRid }) => {
-          rid = openedFileRid;
-          return Deno.write(openedFileRid, buffer);
-        })
-        .then(resolve, reject);
-    }
-  })
-    .then(() => {
-      closeRidIfNecessary(typeof pathOrRid === "string", rid);
-      callbackFn(null);
-    }, (err) => {
-      closeRidIfNecessary(typeof pathOrRid === "string", rid);
-      callbackFn(err);
-    });
-}
+  // Don't make changes directly on options object
+  options = copyObject(options);
 
-function closeRidIfNecessary(isPathString: boolean, rid: number): void {
-  if (isPathString && rid != -1) {
-    //Only close if a path was supplied and a rid allocated
-    Deno.close(rid);
+  // Force append behavior when using a supplied file descriptor
+  if (!options.flag || isFd(path)) {
+    options.flag = "a";
   }
+
+  writeFile(path, data, options, callback);
 }
 
 /**
  * TODO: Also accept 'data' parameter as a Node polyfill Buffer type once these
  * are implemented. See https://github.com/denoland/deno/issues/3403
  */
 export function appendFileSync(
-  pathOrRid: string | number | URL,
+  path: string | number | URL,
   data: string | Uint8Array,
   options?: Encodings | WriteFileOptions,
 ): void {
-  let rid = -1;
-
-  validateEncoding(options);
-  pathOrRid = pathOrRid instanceof URL ? fromFileUrl(pathOrRid) : pathOrRid;
-
-  try {
-    if (typeof pathOrRid === "number") {
-      rid = pathOrRid;
-    } else {
-      const mode: number | undefined = isFileOptions(options)
-        ? options.mode
-        : undefined;
-      const flag: string | undefined = isFileOptions(options)
-        ? options.flag
-        : undefined;
+  options = getOptions(options, { encoding: "utf8", mode: 0o666, flag: "a" });
 
-      const file = Deno.openSync(pathOrRid, { ...getOpenOptions(flag), mode });
-      rid = file.rid;
-    }
+  // Don't make changes directly on options object
+  options = copyObject(options);
 
-    const buffer: Uint8Array = data instanceof Uint8Array
-      ? data
-      : new TextEncoder().encode(data);
-
-    Deno.writeSync(rid, buffer);
-  } finally {
-    closeRidIfNecessary(typeof pathOrRid === "string", rid);
+  // Force append behavior when using a supplied file descriptor
+  if (!options.flag || isFd(path)) {
+    options.flag = "a";
   }
-}
 
-function validateEncoding(
-  encodingOption: Encodings | WriteFileOptions | undefined,
-): void {
-  if (!encodingOption) return;
-
-  if (typeof encodingOption === "string") {
-    if (encodingOption !== "utf8") {
-      throw new Error("Only 'utf8' encoding is currently supported");
-    }
-  } else if (encodingOption.encoding && encodingOption.encoding !== "utf8") {
-    throw new Error("Only 'utf8' encoding is currently supported");
-  }
+  writeFileSync(path, data, options);
 }

node/_fs/_fs_appendFile_test.ts

@@ -14,7 +14,7 @@ Deno.test({
         appendFile("some/path", "some data", "utf8");
       },
       Error,
-      "No callback function supplied",
+      "Callback must be a function. Received 'utf8'",
     );
   },
 });
@@ -28,7 +28,7 @@ Deno.test({
         appendFile("some/path", "some data", "made-up-encoding", () => {});
       },
       Error,
-      "Only 'utf8' encoding is currently supported",
+      "The argument 'made-up-encoding' is invalid encoding. Received 'encoding'",
     );
     assertThrows(
       () => {
@@ -41,13 +41,13 @@ Deno.test({
         );
       },
       Error,
-      "Only 'utf8' encoding is currently supported",
+      "The argument 'made-up-encoding' is invalid encoding. Received 'encoding'",
     );
     assertThrows(
       // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
       () => appendFileSync("some/path", "some data", "made-up-encoding"),
       Error,
-      "Only 'utf8' encoding is currently supported",
+      "The argument 'made-up-encoding' is invalid encoding. Received 'encoding'",
     );
     assertThrows(
       () =>
@@ -56,7 +56,7 @@ Deno.test({
           encoding: "made-up-encoding",
         }),
       Error,
-      "Only 'utf8' encoding is currently supported",
+      "The argument 'made-up-encoding' is invalid encoding. Received 'encoding'",
     );
   },
 });
@@ -196,7 +196,7 @@ Deno.test({
     const tempFile: string = Deno.makeTempFileSync();
     assertThrows(
       () => appendFileSync(tempFile, "hello world", { flag: "ax" }),
-      Deno.errors.AlreadyExists,
+      Error,
       "",
     );
     assertEquals(Deno.resources(), openResourcesBeforeAppend);

node/_fs/_fs_common.ts

@@ -1,4 +1,5 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+import { validateCallback } from "../internal/validators.js";
 import {
   BinaryEncodings,
   Encodings,
@@ -162,3 +163,11 @@ export function getOpenOptions(flag: string | undefined): Deno.OpenOptions {
 
   return openOptions;
 }
+
+export { isUint32 as isFd } from "../internal/validators.js";
+
+export function maybeCallback(cb: unknown) {
+  validateCallback(cb);
+
+  return cb as CallbackWithError;
+}

node/_fs/_fs_writeFile.ts

@@ -1,5 +1,5 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
-import { Encodings, notImplemented } from "../_utils.ts";
+import { Encodings } from "../_utils.ts";
 import { fromFileUrl } from "../path.ts";
 import { Buffer } from "../buffer.ts";
 import { writeAllSync } from "../../streams/conversion.ts";
@@ -13,6 +13,7 @@ import {
 } from "./_fs_common.ts";
 import { isWindows } from "../../_util/os.ts";
 import { AbortError, denoErrorToNodeError } from "../_errors.ts";
+import { validateStringAfterArrayBufferView } from "../internal/fs/utils.js";
 
 export function writeFile(
   pathOrRid: string | number | URL,
@@ -44,6 +45,7 @@ export function writeFile(
   const openOptions = getOpenOptions(flag || "w");
 
   if (!ArrayBuffer.isView(data)) {
+    validateStringAfterArrayBufferView(data, "data");
     data = Buffer.from(String(data), encoding);
   }
 
@@ -57,8 +59,9 @@ export function writeFile(
         ? new Deno.File(pathOrRid as number)
         : await Deno.open(pathOrRid as string, openOptions);
 
-      if (!isRid && mode) {
-        if (isWindows) notImplemented(`"mode" on Windows`);
+      // ignore mode because it's not supported on windows
+      // TODO: remove `!isWindows` when `Deno.chmod` is supported
+      if (!isRid && mode && !isWindows) {
         await Deno.chmod(pathOrRid as string, mode);
       }
 
@@ -98,6 +101,7 @@ export function writeFileSync(
   const openOptions = getOpenOptions(flag || "w");
 
   if (!ArrayBuffer.isView(data)) {
+    validateStringAfterArrayBufferView(data, "data");
     data = Buffer.from(String(data), encoding);
   }
 
@@ -110,8 +114,9 @@ export function writeFileSync(
       ? new Deno.File(pathOrRid as number)
       : Deno.openSync(pathOrRid as string, openOptions);
 
-    if (!isRid && mode) {
-      if (isWindows) notImplemented(`"mode" on Windows`);
+    // ignore mode because it's not supported on windows
+    // TODO: remove `!isWindows` when `Deno.chmod` is supported
+    if (!isRid && mode && !isWindows) {
       Deno.chmodSync(pathOrRid as string, mode);
     }
 

node/_tools/config.json

@@ -26,6 +26,7 @@
       "test-event-emitter-max-listeners.js",
       "test-event-emitter-no-error-provided-to-error-event.js",
       "test-event-emitter-prepend.js",
+      "test-fs-append-file.js",
       "test-fs-rmdir-recursive.js",
       "test-fs-write-file.js",
       "test-querystring.js",
@@ -135,13 +136,16 @@
       "test-event-emitter-symbols.js",
       "test-events-list.js",
       "test-events-once.js",
+      "test-fs-append-file-sync.js",
+      "test-fs-append-file.js",
       "test-fs-rm.js",
       "test-fs-rmdir-recursive-sync-warns-not-found.js",
       "test-fs-rmdir-recursive-sync-warns-on-file.js",
       "test-fs-rmdir-recursive-throws-not-found.js",
       "test-fs-rmdir-recursive-throws-on-file.js",
       "test-fs-rmdir-recursive-warns-not-found.js",
       "test-fs-rmdir-recursive-warns-on-file.js",
+      "test-fs-rmdir-recursive.js",
       "test-fs-rmdir-type-check.js",
       "test-fs-write-file-buffer.js",
       "test-fs-write-file-invalid-path.js",