Merge pull request #1143 from log4js-node/update-test

chore(test): update teardown() for tests to remove tmp files

package.json

@@ -29,7 +29,7 @@
   },
   "scripts": {
     "pretest": "eslint \"lib/**/*.js\" \"test/**/*.js\"",
-    "test": "tap \"test/tap/**/*.js\" --cov",
+    "test": "tap \"test/tap/**/*.js\" --cov --timeout=45",
     "typings": "tsc -p types/tsconfig.json",
     "codecov": "tap \"test/tap/**/*.js\" --cov --coverage-report=lcov && codecov"
   },

test/tap/configuration-validation-test.js

@@ -1,13 +1,20 @@
 const { test } = require("tap");
-const { unlinkSync } = require("fs");
 const util = require("util");
 const path = require("path");
 const sandbox = require("@log4js-node/sandboxed-module");
 const debug = require("debug")("log4js:test.configuration-validation");
 const deepFreeze = require("deep-freeze");
+const fs = require("fs");
 const log4js = require("../../lib/log4js");
 const configuration = require("../../lib/configuration");
 
+const removeFiles = async filenames => {
+  if (!Array.isArray(filenames))
+    filenames = [filenames];
+  const promises = filenames.map(filename => fs.promises.unlink(filename));
+  await Promise.allSettled(promises);
+};
+
 const testAppender = (label, result) => ({
   configure(config, layouts, findAppender) {
     debug(
@@ -248,21 +255,17 @@ test("log4js configuration validation", batch => {
   );
 
   batch.test("should not throw error if configure object is freezed", t => {
-    const filename = "test/tap/freeze-date-file-test"
+    const testFile = "test/tap/freeze-date-file-test";
     t.tearDown(() => {
-      try {
-        unlinkSync(filename);
-      } catch (_) {
-        // doesn't really matter if it failed
-      }
+      removeFiles(testFile);
     });
     t.doesNotThrow(() =>
       log4js.configure(
         deepFreeze({
           appenders: {
             dateFile: {
-              filename,
               type: "dateFile",
+              filename: testFile,
               alwaysIncludePattern: false
             }
           },

test/tap/dateFileAppender-test.js

@@ -25,7 +25,7 @@ test("../../lib/appenders/dateFile", batch => {
     const logger = log4js.getLogger("default-settings");
 
     logger.info("This should be in the file.");
-    t.teardown(() => {
+    t.tearDown(() => {
       removeFile("date-appender-default.log");
     });
 
@@ -72,7 +72,7 @@ test("../../lib/appenders/dateFile", batch => {
       );
     });
 
-    t.teardown(() => {
+    t.tearDown(() => {
       removeFile("date-file-test.log");
     });
   });
@@ -98,18 +98,18 @@ test("../../lib/appenders/dateFile", batch => {
       options.appenders.date.pattern,
       new Date()
     );
+    const testFile = `date-file-test.${thisTime}`;
     const existingFile = path.join(
-      process.cwd(),
-      "test/tap/",
-      `date-file-test.${thisTime}`
+      __dirname,
+      testFile
     );
     fs.writeFileSync(existingFile, `this is existing data${EOL}`, "utf8");
     log4js.configure(options);
     const logger = log4js.getLogger("tests");
     logger.warn("this should be written to the file with the appended date");
 
-    t.teardown(() => {
-      removeFile(existingFile);
+    t.tearDown(() => {
+      removeFile(testFile);
     });
 
     // wait for filesystem to catch up
@@ -140,7 +140,7 @@ test("../../lib/appenders/dateFile", batch => {
     logger.info("1");
     logger.info("2");
     logger.info("3");
-    t.teardown(() => {
+    t.tearDown(() => {
       removeFile("date-appender-flush.log");
     });
 

test/tap/file-descriptor-leak-test.js

@@ -67,7 +67,7 @@ if (process.platform !== "win32") {
       }, 250);
     });
 
-    batch.teardown(async () => {
+    batch.tearDown(async () => {
       log4js.shutdown();
 
       const filenames = Object.values(config.appenders).map(appender => appender.filename);

test/tap/file-sighup-test.js

@@ -38,7 +38,7 @@ test("file appender single SIGHUP handler", t => {
   const log4js = require("../../lib/log4js");
   log4js.configure(config);
 
-  t.teardown(async () => {
+  t.tearDown(async () => {
     log4js.shutdown();
 
     const filenames = Object.values(config.appenders).map(appender => appender.filename);
@@ -120,6 +120,9 @@ test("file appender SIGHUP handler leak", t => {
     },
     categories: { default: { appenders: ["file"], level: "info" } }
   });
+  t.tearDown(async () => {
+    await removeFiles("test.log");
+  });
   t.plan(2);
   t.equal(process.listenerCount("SIGHUP"), initialListeners + 1);
   log4js.shutdown(() => {

test/tap/fileAppender-test.js

@@ -50,6 +50,11 @@ test("log4js fileAppender", batch => {
     const testFile = path.join(__dirname, "fa-default-test.log");
     await removeFile(testFile);
 
+    t.tearDown(async () => {
+      await new Promise(resolve => log4js.shutdown(resolve));
+      await removeFile(testFile);
+    });
+
     log4js.configure({
       appenders: { test: { type: "file", filename: testFile } },
       categories: { default: { appenders: ["test"], level: "trace" } }
@@ -76,6 +81,7 @@ test("log4js fileAppender", batch => {
     const logger = log4js.getLogger("max-file-size");
 
     t.tearDown(async () => {
+      await new Promise(resolve => log4js.shutdown(resolve));
       await Promise.all([removeFile(testFile), removeFile(`${testFile}.1`)]);
     });
     await Promise.all([removeFile(testFile), removeFile(`${testFile}.1`)]);
@@ -116,6 +122,7 @@ test("log4js fileAppender", batch => {
     const logger = log4js.getLogger("max-file-size-unit");
 
     t.tearDown(async () => {
+      await new Promise(resolve => log4js.shutdown(resolve));
       await Promise.all([removeFile(testFile), removeFile(`${testFile}.1`)]);
     });
     await Promise.all([removeFile(testFile), removeFile(`${testFile}.1`)]);
@@ -168,6 +175,7 @@ test("log4js fileAppender", batch => {
     ]);
 
     t.tearDown(async () => {
+      await new Promise(resolve => log4js.shutdown(resolve));
       await Promise.all([
         removeFile(testFile),
         removeFile(`${testFile}.1`),
@@ -227,6 +235,7 @@ test("log4js fileAppender", batch => {
     ]);
 
     t.tearDown(async () => {
+      await new Promise(resolve => log4js.shutdown(resolve));
       await Promise.all([
         removeFile(testFile),
         removeFile(`${testFile}.1.gz`),

test/tap/fileSyncAppender-test.js

@@ -42,7 +42,6 @@ test("log4js fileSyncAppender", batch => {
   batch.test("with a max file size and no backups", t => {
     const testFile = path.join(__dirname, "/fa-maxFileSize-sync-test.log");
     const logger = log4js.getLogger("max-file-size");
-
     remove(testFile);
     remove(`${testFile}.1`);
 
@@ -93,7 +92,6 @@ test("log4js fileSyncAppender", batch => {
   batch.test("with a max file size in unit mode and no backups", t => {
     const testFile = path.join(__dirname, "/fa-maxFileSize-unit-sync-test.log");
     const logger = log4js.getLogger("max-file-size-unit");
-
     remove(testFile);
     remove(`${testFile}.1`);
 
@@ -219,13 +217,20 @@ test("log4js fileSyncAppender", batch => {
   });
 
   batch.test("configure with fileSyncAppender", t => {
+    const testFile = "tmp-sync-tests.log";
+    remove(testFile);
+
+    t.tearDown(() => {
+      remove(testFile);
+    });
+
     // this config defines one file appender (to ./tmp-sync-tests.log)
     // and sets the log level for "tests" to WARN
     log4js.configure({
       appenders: {
         sync: {
           type: "fileSync",
-          filename: "tmp-sync-tests.log",
+          filename: testFile,
           layout: { type: "messagePassThrough" }
         }
       },
@@ -238,20 +243,27 @@ test("log4js fileSyncAppender", batch => {
     logger.info("this should not be written to the file");
     logger.warn("this should be written to the file");
 
-    fs.readFile("tmp-sync-tests.log", "utf8", (err, contents) => {
+    fs.readFile(testFile, "utf8", (err, contents) => {
       t.include(contents, `this should be written to the file${EOL}`);
       t.equal(contents.indexOf("this should not be written to the file"), -1);
       t.end();
     });
   });
 
   batch.test("test options", t => {
+    const testFile = "tmp-options-tests.log";
+    remove(testFile);
+
+    t.tearDown(() => {
+      remove(testFile);
+    });
+
     // using non-standard options
     log4js.configure({
       appenders: {
         sync: {
           type: "fileSync",
-          filename: "tmp-options-tests.log",
+          filename: testFile,
           layout: { type: "messagePassThrough" },
           flags: "w",
           encoding: "ascii",
@@ -265,7 +277,7 @@ test("log4js fileSyncAppender", batch => {
     const logger = log4js.getLogger();
     logger.warn("log message");
 
-    fs.readFile("tmp-options-tests.log", "ascii", (err, contents) => {
+    fs.readFile(testFile, "ascii", (err, contents) => {
       t.include(contents, `log message${EOL}`);
       t.end();
     });

test/tap/multi-file-appender-test.js

@@ -4,10 +4,20 @@ const debug = require("debug");
 const fs = require("fs");
 const log4js = require("../../lib/log4js");
 
+const removeFiles = async filenames => {
+  if (!Array.isArray(filenames))
+    filenames = [filenames];
+  const promises = filenames.map(filename => fs.promises.unlink(filename));
+  await Promise.allSettled(promises);
+};
+
 test("multiFile appender", batch => {
   batch.test(
     "should write to multiple files based on the loggingEvent property",
     t => {
+      t.tearDown(async () => {
+        await removeFiles(["logs/A.log", "logs/B.log"]);
+      });
       log4js.configure({
         appenders: {
           multi: {
@@ -34,6 +44,9 @@ test("multiFile appender", batch => {
   batch.test(
     "should write to multiple files based on loggingEvent.context properties",
     t => {
+      t.tearDown(async () => {
+        await removeFiles(["logs/C.log", "logs/D.log"]);
+      });
       log4js.configure({
         appenders: {
           multi: {
@@ -60,6 +73,9 @@ test("multiFile appender", batch => {
   );
 
   batch.test("should close file after timeout", t => {
+    t.tearDown(async () => {
+      await removeFiles("logs/C.log");
+    });
     /* checking that the file is closed after a timeout is done by looking at the debug logs
       since detecting file locks with node.js is platform specific.
      */
@@ -104,6 +120,9 @@ test("multiFile appender", batch => {
   batch.test(
     "should fail silently if loggingEvent property has no value",
     t => {
+      t.tearDown(async () => {
+        await removeFiles("logs/E.log");
+      });
       log4js.configure({
         appenders: {
           multi: {
@@ -133,6 +152,9 @@ test("multiFile appender", batch => {
   );
 
   batch.test("should pass options to rolling file stream", t => {
+    t.tearDown(async () => {
+      await removeFiles(["logs/F.log", "logs/F.log.1", "logs/F.log.2"]);
+    });
     log4js.configure({
       appenders: {
         multi: {
@@ -164,6 +186,9 @@ test("multiFile appender", batch => {
   });
 
   batch.test("should inherit config from category hierarchy", t => {
+    t.tearDown(async () => {
+      await removeFiles("logs/test.someTest.log");
+    });
     log4js.configure({
       appenders: {
         out: { type: "stdout" },
@@ -211,5 +236,15 @@ test("multiFile appender", batch => {
     });
   });
 
+  batch.tearDown(async () => {
+    try {
+      const files = fs.readdirSync("logs");
+      await removeFiles(files.map(filename => `logs/${filename}`));
+      fs.rmdirSync("logs");
+    } catch (e) {
+      // doesn't matter
+    }
+  });
+
   batch.end();
 });

test/tap/pause-test.js

@@ -1,9 +1,20 @@
 const tap = require("tap");
+const fs = require("fs");
 const log4js = require("../../lib/log4js");
 
+const removeFiles = async filenames => {
+  if (!Array.isArray(filenames))
+    filenames = [filenames];
+  const promises = filenames.map(filename => fs.promises.unlink(filename));
+  await Promise.allSettled(promises);
+};
+
 tap.test("Drain event test", batch => {
 
   batch.test("Should emit pause event and resume when logging in a file with high frequency", t => {
+    t.tearDown(async () => {
+      await removeFiles("logs/drain.log");
+    });
     // Generate logger with 5k of highWaterMark config
     log4js.configure({
       appenders: {
@@ -36,6 +47,9 @@ tap.test("Drain event test", batch => {
 
 
   batch.test("Should emit pause event and resume when logging in a date file with high frequency", (t) => {
+    t.tearDown(async () => {
+      await removeFiles("logs/date-file-drain.log");
+    });
     // Generate date file logger with 5kb of highWaterMark config
     log4js.configure({
       appenders: {
@@ -63,7 +77,16 @@ tap.test("Drain event test", batch => {
         logger.info("This is a test for emitting drain event in date file logger");
     }
     t.end();
+  });
 
+  batch.tearDown(async () => {
+    try {
+      const files = fs.readdirSync("logs");
+      await removeFiles(files.map(filename => `logs/${filename}`));
+      fs.rmdirSync("logs");
+    } catch (e) {
+      // doesn't matter
+    }
   });
 
   batch.end();