From 3caa706ee259f2f0ade560d99f94ddbef7a74704 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Wed, 19 Jan 2022 00:23:07 +0800 Subject: [PATCH 1/5] chore(test): Changed default TAP test suite timeout from 30s to 45s because Windows takes a long time --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 58eb63f5..1c597aa1 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ }, "scripts": { "pretest": "eslint \"lib/**/*.js\" \"test/**/*.js\"", - "test": "tap \"test/tap/**/*.js\" --cov", + "test": "TAP_TIMEOUT=45 tap \"test/tap/**/*.js\" --cov", "typings": "tsc -p types/tsconfig.json", "codecov": "tap \"test/tap/**/*.js\" --cov --coverage-report=lcov && codecov" }, From c0f95fa53a88f631f1ac8c60856a6e0612ba996f Mon Sep 17 00:00:00 2001 From: peteriman Date: Tue, 18 Jan 2022 14:44:13 +0800 Subject: [PATCH 2/5] chore(test): update teardown() for tests to remove tmp files --- test/tap/configuration-validation-test.js | 13 +++++++++- test/tap/dateFileAppender-test.js | 8 +++---- test/tap/file-sighup-test.js | 3 +++ test/tap/fileAppender-test.js | 9 +++++++ test/tap/fileSyncAppender-test.js | 24 ++++++++++++++----- test/tap/multi-file-appender-test.js | 29 +++++++++++++++++++++++ test/tap/pause-test.js | 18 ++++++++++++++ 7 files changed, 93 insertions(+), 11 deletions(-) diff --git a/test/tap/configuration-validation-test.js b/test/tap/configuration-validation-test.js index fb965b3b..ec6bf227 100644 --- a/test/tap/configuration-validation-test.js +++ b/test/tap/configuration-validation-test.js @@ -7,6 +7,13 @@ const deepFreeze = require("deep-freeze"); 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( @@ -247,13 +254,17 @@ test("log4js configuration validation", batch => { ); batch.test("should not throw error if configure object is freezed", t => { + const testFile = "test/tap/freeze-date-file-test"; + t.teardown(() => { + removeFiles(testFile); + }); t.doesNotThrow(() => log4js.configure( deepFreeze({ appenders: { dateFile: { type: "dateFile", - filename: "test/tap/freeze-date-file-test", + filename: testFile, alwaysIncludePattern: false } }, diff --git a/test/tap/dateFileAppender-test.js b/test/tap/dateFileAppender-test.js index db4e6f97..8baa9688 100644 --- a/test/tap/dateFileAppender-test.js +++ b/test/tap/dateFileAppender-test.js @@ -98,10 +98,10 @@ 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); @@ -109,7 +109,7 @@ test("../../lib/appenders/dateFile", batch => { logger.warn("this should be written to the file with the appended date"); t.teardown(() => { - removeFile(existingFile); + removeFile(testFile); }); // wait for filesystem to catch up diff --git a/test/tap/file-sighup-test.js b/test/tap/file-sighup-test.js index 78057124..cf059b2b 100644 --- a/test/tap/file-sighup-test.js +++ b/test/tap/file-sighup-test.js @@ -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(() => { diff --git a/test/tap/fileAppender-test.js b/test/tap/fileAppender-test.js index be1791c2..0abe3515 100644 --- a/test/tap/fileAppender-test.js +++ b/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`), diff --git a/test/tap/fileSyncAppender-test.js b/test/tap/fileSyncAppender-test.js index d30b51f1..a69d08f1 100644 --- a/test/tap/fileSyncAppender-test.js +++ b/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,7 +243,7 @@ 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(); @@ -246,12 +251,19 @@ test("log4js fileSyncAppender", batch => { }); 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(); }); diff --git a/test/tap/multi-file-appender-test.js b/test/tap/multi-file-appender-test.js index 2b04e4d8..5df3200d 100644 --- a/test/tap/multi-file-appender-test.js +++ b/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,9 @@ test("multiFile appender", batch => { }); }); + batch.tearDown(() => { + fs.rmdirSync("logs"); + }); + batch.end(); }); diff --git a/test/tap/pause-test.js b/test/tap/pause-test.js index 0635c615..f405d0e1 100644 --- a/test/tap/pause-test.js +++ b/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: { @@ -66,5 +80,9 @@ tap.test("Drain event test", batch => { }); + batch.tearDown(() => { + fs.rmdirSync("logs"); + }); + batch.end(); }); From 43a219913736445ab8a05d1efa1ecdc766c63d41 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Wed, 19 Jan 2022 01:57:25 +0800 Subject: [PATCH 3/5] chore(test): Changed default TAP test suite timeout from 30s to 45s because Windows takes a long time --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1c597aa1..7dc4d71f 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ }, "scripts": { "pretest": "eslint \"lib/**/*.js\" \"test/**/*.js\"", - "test": "TAP_TIMEOUT=45 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" }, From a0baec23a84684d998ab4c704b1f19603550e3fa Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Wed, 19 Jan 2022 09:24:55 +0800 Subject: [PATCH 4/5] chore(test): fixed teardown() causing tests to fail due to fs errors on removal --- test/tap/multi-file-appender-test.js | 10 ++++++++-- test/tap/pause-test.js | 11 ++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/test/tap/multi-file-appender-test.js b/test/tap/multi-file-appender-test.js index 5df3200d..aa93f7c5 100644 --- a/test/tap/multi-file-appender-test.js +++ b/test/tap/multi-file-appender-test.js @@ -236,8 +236,14 @@ test("multiFile appender", batch => { }); }); - batch.tearDown(() => { - fs.rmdirSync("logs"); + 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(); diff --git a/test/tap/pause-test.js b/test/tap/pause-test.js index f405d0e1..37d12302 100644 --- a/test/tap/pause-test.js +++ b/test/tap/pause-test.js @@ -77,11 +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(() => { - fs.rmdirSync("logs"); + 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(); From 8cba85f91da6d122d4c559bf891a17b01e69b722 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Wed, 19 Jan 2022 10:31:25 +0800 Subject: [PATCH 5/5] chore(test): renamed tap.teardown() to tap.tearDown() for consistency (while both works, only tap.tearDown() is documented) --- test/tap/configuration-validation-test.js | 2 +- test/tap/dateFileAppender-test.js | 8 ++++---- test/tap/file-descriptor-leak-test.js | 2 +- test/tap/file-sighup-test.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/tap/configuration-validation-test.js b/test/tap/configuration-validation-test.js index e977c6d3..5350eac4 100644 --- a/test/tap/configuration-validation-test.js +++ b/test/tap/configuration-validation-test.js @@ -256,7 +256,7 @@ test("log4js configuration validation", batch => { batch.test("should not throw error if configure object is freezed", t => { const testFile = "test/tap/freeze-date-file-test"; - t.teardown(() => { + t.tearDown(() => { removeFiles(testFile); }); t.doesNotThrow(() => diff --git a/test/tap/dateFileAppender-test.js b/test/tap/dateFileAppender-test.js index 8baa9688..39649919 100644 --- a/test/tap/dateFileAppender-test.js +++ b/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"); }); }); @@ -108,7 +108,7 @@ test("../../lib/appenders/dateFile", batch => { const logger = log4js.getLogger("tests"); logger.warn("this should be written to the file with the appended date"); - t.teardown(() => { + t.tearDown(() => { removeFile(testFile); }); @@ -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"); }); diff --git a/test/tap/file-descriptor-leak-test.js b/test/tap/file-descriptor-leak-test.js index 7436a8e7..55a04b35 100644 --- a/test/tap/file-descriptor-leak-test.js +++ b/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); diff --git a/test/tap/file-sighup-test.js b/test/tap/file-sighup-test.js index cf059b2b..c9682d91 100644 --- a/test/tap/file-sighup-test.js +++ b/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,7 +120,7 @@ test("file appender SIGHUP handler leak", t => { }, categories: { default: { appenders: ["file"], level: "info" } } }); - t.teardown(async () => { + t.tearDown(async () => { await removeFiles("test.log"); }); t.plan(2);