From 619372c8000193870c9a30fa79cc40ec5bf828f7 Mon Sep 17 00:00:00 2001 From: RDW Date: Wed, 10 Nov 2021 11:27:01 +0100 Subject: [PATCH] Refactor: Flatten the hierarchy of test suite spec descriptors This allows adding individual test suites on a per-file basis and aggregating them under a single namespace. Refactor: Move builtin tests before API tests If tests for builtins fail, there is little point in going ahead and testing the APIs that rely on them. If anything, it might cause weird errors that are needlessly difficult to troubleshoot. --- Tests/API/C_Settings/validate.js | 2 +- .../API/C_Settings/validateDefaultSettings.js | 2 +- Tests/API/C_Settings/validateUserSettings.js | 2 +- Tests/Builtins/LocalCacheTests.js | 173 +++++++++--------- Tests/run-renderer-tests.js | 26 ++- 5 files changed, 105 insertions(+), 100 deletions(-) diff --git a/Tests/API/C_Settings/validate.js b/Tests/API/C_Settings/validate.js index 4141449..f4052b9 100644 --- a/Tests/API/C_Settings/validate.js +++ b/Tests/API/C_Settings/validate.js @@ -1,4 +1,4 @@ -describe("The C_Settings API", function () { +describe("validate", function () { it("should succeed validating arbitrary settings when the default settings are passed", function () { const defaultSettings = C_FileSystem.readJSON(C_Settings.DEFAULT_SETTINGS_FILE_PATH); assertTrue(C_Settings.validate(defaultSettings)); diff --git a/Tests/API/C_Settings/validateDefaultSettings.js b/Tests/API/C_Settings/validateDefaultSettings.js index 129302e..1943a8d 100644 --- a/Tests/API/C_Settings/validateDefaultSettings.js +++ b/Tests/API/C_Settings/validateDefaultSettings.js @@ -1,4 +1,4 @@ -describe("The C_Settings API", function () { +describe("validateDefaultSettings", function () { it("should always succeed in validating the default settings", function () { assertTrue(C_Settings.validateDefaultSettings()); }); diff --git a/Tests/API/C_Settings/validateUserSettings.js b/Tests/API/C_Settings/validateUserSettings.js index 7a3983d..14a59ce 100644 --- a/Tests/API/C_Settings/validateUserSettings.js +++ b/Tests/API/C_Settings/validateUserSettings.js @@ -1,4 +1,4 @@ -describe("The C_Settings API", () => { +describe("validateUserSettings", () => { it("should always succeed in validating the user settings", () => { assertTrue(C_Settings.validateUserSettings()); }); diff --git a/Tests/Builtins/LocalCacheTests.js b/Tests/Builtins/LocalCacheTests.js index 97a7540..5a38ec7 100644 --- a/Tests/Builtins/LocalCacheTests.js +++ b/Tests/Builtins/LocalCacheTests.js @@ -1,118 +1,115 @@ -describe("Builtins", () => { - describe("LocalCache", () => { - it("should be exported into the global environment", () => { - assertEquals(LocalCache.name, "LocalCache"); - }); - - let exportedApiSurface = ["getValue", "setValue", "clear", "evict", "load", "save", "getFilePath", "setFilePath"]; +describe("LocalCache", () => { + it("should be exported into the global environment", () => { + assertEquals(LocalCache.name, "LocalCache"); + }); - exportedApiSurface.forEach((namedExport) => { - it("should export function " + namedExport, () => { - const cache = new LocalCache(""); - assertEquals(typeof cache[namedExport], "function"); - }); - }); + let exportedApiSurface = ["getValue", "setValue", "clear", "evict", "load", "save", "getFilePath", "setFilePath"]; - it("should be able to set and retrieve values", () => { + exportedApiSurface.forEach((namedExport) => { + it("should export function " + namedExport, () => { const cache = new LocalCache(""); - cache.setValue("someKey", 42); - assertEquals(cache.getValue("someKey"), 42); + assertEquals(typeof cache[namedExport], "function"); }); + }); - it("should be able to evict specific stored values", () => { - const cache = new LocalCache(""); - cache.setValue("someKey", 42); - assertEquals(cache.getValue("someKey"), 42); - cache.evict("someKey"); - assertUndefined(cache.getValue("someKey")); - }); + it("should be able to set and retrieve values", () => { + const cache = new LocalCache(""); + cache.setValue("someKey", 42); + assertEquals(cache.getValue("someKey"), 42); + }); - it("should be able to evict all stored values at once", () => { - const cache = new LocalCache(""); + it("should be able to evict specific stored values", () => { + const cache = new LocalCache(""); + cache.setValue("someKey", 42); + assertEquals(cache.getValue("someKey"), 42); + cache.evict("someKey"); + assertUndefined(cache.getValue("someKey")); + }); - cache.setValue("key1", 42); - cache.setValue("key2", 43); + it("should be able to evict all stored values at once", () => { + const cache = new LocalCache(""); - assertEquals(cache.getValue("key1"), 42); - assertEquals(cache.getValue("key2"), 43); + cache.setValue("key1", 42); + cache.setValue("key2", 43); - cache.evict(); + assertEquals(cache.getValue("key1"), 42); + assertEquals(cache.getValue("key2"), 43); - assertUndefined(cache.getValue("key1")); - assertUndefined(cache.getValue("key2")); - }); + cache.evict(); - // TBD: This is identical to evict(undefined), so maybe it should be consolidated? - it("should be able to clear the internal key value store", () => { - const cache = new LocalCache(""); + assertUndefined(cache.getValue("key1")); + assertUndefined(cache.getValue("key2")); + }); - cache.setValue("key1", 42); - cache.setValue("key2", 43); + it("should be able to clear the internal key value store", () => { + const cache = new LocalCache(""); - assertEquals(cache.getValue("key1"), 42); - assertEquals(cache.getValue("key2"), 43); + cache.setValue("key1", 42); + cache.setValue("key2", 43); - cache.clear(); + assertEquals(cache.getValue("key1"), 42); + assertEquals(cache.getValue("key2"), 43); - assertUndefined(cache.getValue("key1")); - assertUndefined(cache.getValue("key2")); - }); + cache.clear(); - it("should be able to save the key value store's contents to disk", () => { - const filePath = "localTestCache_save.json"; - const cache = new LocalCache(filePath); + assertUndefined(cache.getValue("key1")); + assertUndefined(cache.getValue("key2")); + }); - cache.setValue("key1", 42); - cache.setValue("key2", 43); - assertFalse(C_FileSystem.fileExists(filePath)); + it("should be able to save the key value store's contents to disk", () => { + const filePath = "localTestCache_save.json"; + const cache = new LocalCache(filePath); - cache.save(); - assertTrue(C_FileSystem.fileExists(filePath)); + cache.setValue("key1", 42); + cache.setValue("key2", 43); + assertFalse(C_FileSystem.fileExists(filePath)); - const contents = C_FileSystem.readJSON(filePath); - assertEquals(contents["key1"], cache.getValue("key1")); - assertEquals(contents["key2"], cache.getValue("key2")); + cache.save(); + assertTrue(C_FileSystem.fileExists(filePath)); - // Cleanup - C_FileSystem.removeFile(filePath); - assertFalse(C_FileSystem.fileExists(filePath)); - }); + const contents = C_FileSystem.readJSON(filePath); + assertEquals(contents["key1"], cache.getValue("key1")); + assertEquals(contents["key2"], cache.getValue("key2")); - it("should be able to load the key value store's contents from disk", () => { - const filePath = "localTestCache_load.json"; - const cache = new LocalCache(filePath); + // Cleanup + C_FileSystem.removeFile(filePath); + assertFalse(C_FileSystem.fileExists(filePath)); + }); - const expectedCacheContents = { - keyThatDidNotExistBeforeLoading: 12345, - shouldHaveDifferentValue: "This is the value that should be present", - }; + it("should be able to load the key value store's contents from disk", () => { + const filePath = "localTestCache_load.json"; + const cache = new LocalCache(filePath); - // These value should be overwritten with a different one after loading - cache.setValue("shouldHaveDifferentValue", 42); - // This value should be discarded after loading - cache.setValue("shouldBeUndefinedAfterLoading", 43); - assertFalse(C_FileSystem.fileExists(filePath)); - C_FileSystem.writeJSON(filePath, expectedCacheContents); + const expectedCacheContents = { + keyThatDidNotExistBeforeLoading: 12345, + shouldHaveDifferentValue: "This is the value that should be present", + }; - cache.load(); + // These value should be overwritten with a different one after loading + cache.setValue("shouldHaveDifferentValue", 42); + // This value should be discarded after loading + cache.setValue("shouldBeUndefinedAfterLoading", 43); + assertFalse(C_FileSystem.fileExists(filePath)); + C_FileSystem.writeJSON(filePath, expectedCacheContents); - assertEquals(cache.getValue("keyThatDidNotExistBeforeLoading"), 12345); - assertEquals(cache.getValue("shouldHaveDifferentValue"), "This is the value that should be present"); - assertUndefined(cache.getValue("shouldBeUndefinedAfterLoading")); + cache.load(); - // Cleanup - C_FileSystem.removeFile(filePath); - assertFalse(C_FileSystem.fileExists(filePath)); - }); + assertEquals(cache.getValue("keyThatDidNotExistBeforeLoading"), 12345); + assertEquals(cache.getValue("shouldHaveDifferentValue"), "This is the value that should be present"); + assertUndefined(cache.getValue("shouldBeUndefinedAfterLoading")); - it("should allow adjusting the file path after initialization", () => { - const originalFilePath = "localTestCache_1.json"; - const cache = new LocalCache(originalFilePath); - assertEquals(cache.getFilePath(), originalFilePath); + // Cleanup + C_FileSystem.removeFile(filePath); + assertFalse(C_FileSystem.fileExists(filePath)); + }); - const updatedFilePath = "localTestCache_2.json"; - cache.setFilePath(updatedFilePath); - assertEquals(cache.getFilePath(), updatedFilePath); - }); + it("should allow adjusting the file path after initialization", () => { + const originalFilePath = "localTestCache_1.json"; + const cache = new LocalCache(originalFilePath); + assertEquals(cache.getFilePath(), originalFilePath); + + const updatedFilePath = "localTestCache_2.json"; + cache.setFilePath(updatedFilePath); + assertEquals(cache.getFilePath(), updatedFilePath); }); }); diff --git a/Tests/run-renderer-tests.js b/Tests/run-renderer-tests.js index e83e4ae..46771f7 100644 --- a/Tests/run-renderer-tests.js +++ b/Tests/run-renderer-tests.js @@ -1,10 +1,18 @@ -const testCases = [ - "API/C_Settings/validate.js", - "API/C_Settings/validateDefaultSettings.js", - "API/C_Settings/validateUserSettings.js", - "Builtins/LocalCacheTests.js", -]; +const testSuites = { + Builtins: ["Builtins/LocalCacheTests.js"], + C_Settings: [ + "API/C_Settings/validate.js", + "API/C_Settings/validateDefaultSettings.js", + "API/C_Settings/validateUserSettings.js", + ], +}; -testCases.forEach((fileName) => { - require("./" + fileName); -}); +for (const namespace in testSuites) { + const testCases = testSuites[namespace]; + + describe(namespace, () => { + testCases.forEach((fileName) => { + require("./" + fileName); + }); + }); +}