From 10574a0f8a5df6d6e0b91385eb21b5c4427d82aa Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sat, 10 Apr 2021 20:21:31 +0200 Subject: [PATCH] Remove obsolete done callbacks from the unit tests The done callbacks are an outdated mechanism to signal Jasmine that a unit test is done, mostly in cases where a unit test needed to wait for an asynchronous operation to complete before doing its assertions. Nowadays a much better mechanism is in place for that, namely simply passing an asynchronous function to Jasmine, so we don't need callbacks anymore (which require more code and may be more difficult to reason about). In these particular cases though the done callbacks never had any real use since nothing asynchronous happens in these places. Synchronous functions don't need to use done callbacks since Jasmine simply knows it's done when the function reaches its normal end, so we can safely get rid of these callbacks. The telltale sign is if the done callback is used unconditionally at the end of the function. This is all done in an effort to over time get rid of all callbacks in the unit test code. --- test/unit/annotation_spec.js | 23 +++++++---------------- test/unit/api_spec.js | 6 ++---- test/unit/cff_parser_spec.js | 9 +++------ test/unit/cmap_spec.js | 3 +-- test/unit/colorspace_spec.js | 6 ++---- test/unit/crypto_spec.js | 13 ++++--------- test/unit/display_utils_spec.js | 6 ++---- test/unit/evaluator_spec.js | 3 +-- test/unit/node_stream_spec.js | 6 ++---- test/unit/primitives_spec.js | 7 ++----- test/unit/scripting_spec.js | 15 +++++---------- test/unit/ui_utils_spec.js | 3 +-- test/unit/unicode_spec.js | 9 +++------ test/unit/writer_spec.js | 9 +++------ 14 files changed, 38 insertions(+), 80 deletions(-) diff --git a/test/unit/annotation_spec.js b/test/unit/annotation_spec.js index 3e80e85aa..ebaeab8b1 100644 --- a/test/unit/annotation_spec.js +++ b/test/unit/annotation_spec.js @@ -194,10 +194,9 @@ describe("annotation", function () { describe("getQuadPoints", function () { let dict, rect; - beforeEach(function (done) { + beforeEach(function () { dict = new Dict(); rect = []; - done(); }); afterEach(function () { @@ -299,10 +298,9 @@ describe("annotation", function () { describe("Annotation", function () { let dict, ref; - beforeAll(function (done) { + beforeAll(function () { dict = new Dict(); ref = Ref.get(1, 0); - done(); }); afterAll(function () { @@ -497,10 +495,9 @@ describe("annotation", function () { describe("MarkupAnnotation", function () { let dict, ref; - beforeAll(function (done) { + beforeAll(function () { dict = new Dict(); ref = Ref.get(1, 0); - done(); }); afterAll(function () { @@ -1336,11 +1333,10 @@ describe("annotation", function () { describe("WidgetAnnotation", function () { let widgetDict; - beforeEach(function (done) { + beforeEach(function () { widgetDict = new Dict(); widgetDict.set("Type", Name.get("Annot")); widgetDict.set("Subtype", Name.get("Widget")); - done(); }); afterEach(function () { @@ -1438,7 +1434,7 @@ describe("annotation", function () { describe("TextWidgetAnnotation", function () { let textWidgetDict, helvRefObj, gothRefObj; - beforeEach(function (done) { + beforeEach(function () { textWidgetDict = new Dict(); textWidgetDict.set("Type", Name.get("Annot")); textWidgetDict.set("Subtype", Name.get("Widget")); @@ -1486,8 +1482,6 @@ describe("annotation", function () { textWidgetDict.set("DA", "/Helv 5 Tf"); textWidgetDict.set("DR", resourceDict); textWidgetDict.set("Rect", [0, 0, 32, 10]); - - done(); }); afterEach(function () { @@ -2315,12 +2309,11 @@ describe("annotation", function () { describe("ButtonWidgetAnnotation", function () { let buttonWidgetDict; - beforeEach(function (done) { + beforeEach(function () { buttonWidgetDict = new Dict(); buttonWidgetDict.set("Type", Name.get("Annot")); buttonWidgetDict.set("Subtype", Name.get("Widget")); buttonWidgetDict.set("FT", Name.get("Btn")); - done(); }); afterEach(function () { @@ -3275,7 +3268,7 @@ describe("annotation", function () { describe("ChoiceWidgetAnnotation", function () { let choiceWidgetDict, fontRefObj; - beforeEach(function (done) { + beforeEach(function () { choiceWidgetDict = new Dict(); choiceWidgetDict.set("Type", Name.get("Annot")); choiceWidgetDict.set("Subtype", Name.get("Widget")); @@ -3296,8 +3289,6 @@ describe("annotation", function () { choiceWidgetDict.set("DA", "/Helv 5 Tf"); choiceWidgetDict.set("DR", resourceDict); choiceWidgetDict.set("Rect", [0, 0, 32, 10]); - - done(); }); afterEach(function () { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 7267c078a..cc0548335 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -55,14 +55,12 @@ describe("api", function () { let CanvasFactory; - beforeAll(function (done) { + beforeAll(function () { CanvasFactory = new DefaultCanvasFactory(); - done(); }); - afterAll(function (done) { + afterAll(function () { CanvasFactory = null; - done(); }); function waitSome(callback) { diff --git a/test/unit/cff_parser_spec.js b/test/unit/cff_parser_spec.js index 1e08ffdab..6eaa0a0b8 100644 --- a/test/unit/cff_parser_spec.js +++ b/test/unit/cff_parser_spec.js @@ -41,7 +41,7 @@ describe("CFFParser", function () { let fontData, parser, cff; - beforeAll(function (done) { + beforeAll(function () { // This example font comes from the CFF spec: // http://www.adobe.com/content/dam/Adobe/en/devnet/font/pdfs/5176.CFF.pdf const exampleFont = @@ -61,22 +61,19 @@ describe("CFFParser", function () { fontArr.push(parseInt(hex, 16)); } fontData = new Stream(fontArr); - done(); }); afterAll(function () { fontData = null; }); - beforeEach(function (done) { + beforeEach(function () { parser = new CFFParser(fontData, {}, SEAC_ANALYSIS_ENABLED); cff = parser.parse(); - done(); }); - afterEach(function (done) { + afterEach(function () { parser = cff = null; - done(); }); it("parses header", function () { diff --git a/test/unit/cmap_spec.js b/test/unit/cmap_spec.js index 77df4f031..b20a4409d 100644 --- a/test/unit/cmap_spec.js +++ b/test/unit/cmap_spec.js @@ -22,7 +22,7 @@ import { StringStream } from "../../src/core/stream.js"; describe("cmap", function () { let fetchBuiltInCMap; - beforeAll(function (done) { + beforeAll(function () { // Allow CMap testing in Node.js, e.g. for Travis. const CMapReaderFactory = new DefaultCMapReaderFactory({ baseUrl: CMAP_PARAMS.cMapUrl, @@ -34,7 +34,6 @@ describe("cmap", function () { name, }); }; - done(); }); afterAll(function () { diff --git a/test/unit/colorspace_spec.js b/test/unit/colorspace_spec.js index ca56de381..ca1a742a4 100644 --- a/test/unit/colorspace_spec.js +++ b/test/unit/colorspace_spec.js @@ -52,14 +52,12 @@ describe("colorspace", function () { describe("ColorSpace caching", function () { let localColorSpaceCache = null; - beforeAll(function (done) { + beforeAll(function () { localColorSpaceCache = new LocalColorSpaceCache(); - done(); }); - afterAll(function (done) { + afterAll(function () { localColorSpaceCache = null; - done(); }); it("caching by Name", function () { diff --git a/test/unit/crypto_spec.js b/test/unit/crypto_spec.js index ce32bb809..822561da5 100644 --- a/test/unit/crypto_spec.js +++ b/test/unit/crypto_spec.js @@ -639,7 +639,7 @@ describe("CipherTransformFactory", function () { let fileId1, fileId2, dict1, dict2, dict3; let aes256Dict, aes256IsoDict, aes256BlankDict, aes256IsoBlankDict; - beforeAll(function (done) { + beforeAll(function () { fileId1 = unescape("%F6%C6%AF%17%F3rR%8DRM%9A%80%D1%EF%DF%18"); fileId2 = unescape("%3CL_%3AD%96%AF@%9A%9D%B3%3Cx%1Cv%AC"); @@ -775,8 +775,6 @@ describe("CipherTransformFactory", function () { P: -1084, R: 6, }); - - done(); }); afterAll(function () { @@ -839,7 +837,7 @@ describe("CipherTransformFactory", function () { }); describe("Encrypt and decrypt", function () { - it("should encrypt and decrypt using ARCFour", function (done) { + it("should encrypt and decrypt using ARCFour", function () { dict3.CF = buildDict({ Identity: buildDict({ CFM: Name.get("V2"), @@ -847,9 +845,8 @@ describe("CipherTransformFactory", function () { }); const dict = buildDict(dict3); ensureEncryptDecryptIsIdentity(dict, fileId1, "user", "hello world"); - done(); }); - it("should encrypt and decrypt using AES128", function (done) { + it("should encrypt and decrypt using AES128", function () { dict3.CF = buildDict({ Identity: buildDict({ CFM: Name.get("AESV2"), @@ -869,9 +866,8 @@ describe("CipherTransformFactory", function () { "user", "aaaaaaaaaaaaaaaaaaa" ); - done(); }); - it("should encrypt and decrypt using AES256", function (done) { + it("should encrypt and decrypt using AES256", function () { dict3.CF = buildDict({ Identity: buildDict({ CFM: Name.get("AESV3"), @@ -891,7 +887,6 @@ describe("CipherTransformFactory", function () { "user", "aaaaaaaaaaaaaaaaaaaaaa" ); - done(); }); }); }); diff --git a/test/unit/display_utils_spec.js b/test/unit/display_utils_spec.js index 3a679830f..bcb196e59 100644 --- a/test/unit/display_utils_spec.js +++ b/test/unit/display_utils_spec.js @@ -28,9 +28,8 @@ describe("display_utils", function () { describe("DOMCanvasFactory", function () { let canvasFactory; - beforeAll(function (done) { + beforeAll(function () { canvasFactory = new DOMCanvasFactory(); - done(); }); afterAll(function () { @@ -121,9 +120,8 @@ describe("display_utils", function () { describe("DOMSVGFactory", function () { let svgFactory; - beforeAll(function (done) { + beforeAll(function () { svgFactory = new DOMSVGFactory(); - done(); }); afterAll(function () { diff --git a/test/unit/evaluator_spec.js b/test/unit/evaluator_spec.js index cef3568f8..97e4907b9 100644 --- a/test/unit/evaluator_spec.js +++ b/test/unit/evaluator_spec.js @@ -59,14 +59,13 @@ describe("evaluator", function () { let partialEvaluator; - beforeAll(function (done) { + beforeAll(function () { partialEvaluator = new PartialEvaluator({ xref: new XRefMock(), handler: new HandlerMock(), pageIndex: 0, idFactory: createIdFactory(/* pageIndex = */ 0), }); - done(); }); afterAll(function () { diff --git a/test/unit/node_stream_spec.js b/test/unit/node_stream_spec.js index 5350e7519..354bb0e27 100644 --- a/test/unit/node_stream_spec.js +++ b/test/unit/node_stream_spec.js @@ -40,7 +40,7 @@ describe("node_stream", function () { ).href; const pdfLength = 1016315; - beforeAll(done => { + beforeAll(function () { // Create http server to serve pdf data for tests. server = http .createServer((request, response) => { @@ -77,13 +77,11 @@ describe("node_stream", function () { }) .listen(0); /* Listen on a random free port */ port = server.address().port; - done(); }); - afterAll(done => { + afterAll(function () { // Close the server from accepting new connections after all test finishes. server.close(); - done(); }); it("read both http(s) and filesystem pdf files", function (done) { diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js index 7ceb19e38..205b29291 100644 --- a/test/unit/primitives_spec.js +++ b/test/unit/primitives_spec.js @@ -90,7 +90,7 @@ describe("primitives", function () { const testFontFile2 = "file2"; const testFontFile3 = "file3"; - beforeAll(function (done) { + beforeAll(function () { emptyDict = new Dict(); dictWithSizeKey = new Dict(); @@ -100,8 +100,6 @@ describe("primitives", function () { dictWithManyKeys.set("FontFile", testFontFile); dictWithManyKeys.set("FontFile2", testFontFile2); dictWithManyKeys.set("FontFile3", testFontFile3); - - done(); }); afterAll(function () { @@ -431,9 +429,8 @@ describe("primitives", function () { const obj2 = Name.get("bar"); let cache; - beforeEach(function (done) { + beforeEach(function () { cache = new RefSetCache(); - done(); }); afterEach(function () { diff --git a/test/unit/scripting_spec.js b/test/unit/scripting_spec.js index 9841f895e..c6236761b 100644 --- a/test/unit/scripting_spec.js +++ b/test/unit/scripting_spec.js @@ -34,7 +34,7 @@ describe("Scripting", function () { }); } - beforeAll(function (done) { + beforeAll(function () { test_id = 0; ref = 1; send_queue = new Map(); @@ -70,7 +70,6 @@ describe("Scripting", function () { return promise.then(sbx => sbx.evalForTesting(code, key)); }, }; - done(); }); afterAll(function () { @@ -204,13 +203,12 @@ describe("Scripting", function () { }); describe("Util", function () { - beforeAll(function (done) { + beforeAll(function () { sandbox.createSandbox({ appInfo: { language: "en-US", platform: "Linux x86_64" }, objects: {}, calculationOrder: [], }); - done(); }); describe("printd", function () { @@ -486,13 +484,12 @@ describe("Scripting", function () { }); describe("Color", function () { - beforeAll(function (done) { + beforeAll(function () { sandbox.createSandbox({ appInfo: { language: "en-US", platform: "Linux x86_64" }, objects: {}, calculationOrder: [], }); - done(); }); function round(color) { @@ -566,13 +563,12 @@ describe("Scripting", function () { }); describe("App", function () { - beforeAll(function (done) { + beforeAll(function () { sandbox.createSandbox({ appInfo: { language: "en-US", platform: "Linux x86_64" }, objects: {}, calculationOrder: [], }); - done(); }); it("should test language", async () => { @@ -593,14 +589,13 @@ describe("Scripting", function () { }); describe("AForm", function () { - beforeAll(function (done) { + beforeAll(function () { sandbox.createSandbox({ appInfo: { language: "en-US", platform: "Linux x86_64" }, objects: {}, calculationOrder: [], dispatchEventName: "_dispatchMe", }); - done(); }); describe("AFExtractNums", function () { diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index baae83661..81f9e2ffb 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -257,9 +257,8 @@ describe("ui_utils", function () { describe("waitOnEventOrTimeout", function () { let eventBus; - beforeAll(function (done) { + beforeAll(function () { eventBus = new EventBus(); - done(); }); afterAll(function () { diff --git a/test/unit/unicode_spec.js b/test/unit/unicode_spec.js index e7b8f95e7..dd544c743 100644 --- a/test/unit/unicode_spec.js +++ b/test/unit/unicode_spec.js @@ -45,10 +45,9 @@ describe("unicode", function () { describe("getUnicodeForGlyph", function () { let standardMap, dingbatsMap; - beforeAll(function (done) { + beforeAll(function () { standardMap = getGlyphsUnicode(); dingbatsMap = getDingbatsGlyphsUnicode(); - done(); }); afterAll(function () { @@ -90,9 +89,8 @@ describe("unicode", function () { describe("getNormalizedUnicodes", function () { let NormalizedUnicodes; - beforeAll(function (done) { + beforeAll(function () { NormalizedUnicodes = getNormalizedUnicodes(); - done(); }); afterAll(function () { @@ -121,9 +119,8 @@ describe("unicode", function () { return char; } - beforeAll(function (done) { + beforeAll(function () { NormalizedUnicodes = getNormalizedUnicodes(); - done(); }); afterAll(function () { diff --git a/test/unit/writer_spec.js b/test/unit/writer_spec.js index ce89be814..ac0eb369f 100644 --- a/test/unit/writer_spec.js +++ b/test/unit/writer_spec.js @@ -20,7 +20,7 @@ import { StringStream } from "../../src/core/stream.js"; describe("Writer", function () { describe("Incremental update", function () { - it("should update a file with new objects", function (done) { + it("should update a file with new objects", function () { const originalData = new Uint8Array(); const newRefs = [ { ref: Ref.get(123, 0x2d), data: "abc\n" }, @@ -58,12 +58,11 @@ describe("Writer", function () { "%%EOF\n"; expect(data).toEqual(expected); - done(); }); }); describe("writeDict", function () { - it("should write a Dict", function (done) { + it("should write a Dict", function () { const dict = new Dict(null); dict.set("A", Name.get("B")); dict.set("B", Ref.get(123, 456)); @@ -93,10 +92,9 @@ describe("Writer", function () { "endstream\n>>>>"; expect(buffer.join("")).toEqual(expected); - done(); }); - it("should write a Dict in escaping PDF names", function (done) { + it("should write a Dict in escaping PDF names", function () { const dict = new Dict(null); dict.set("\xfeA#", Name.get("hello")); dict.set("B", Name.get("#hello")); @@ -108,7 +106,6 @@ describe("Writer", function () { const expected = "<< /#feA#23 /hello /B /#23hello /C /he#fello#ff>>"; expect(buffer.join("")).toEqual(expected); - done(); }); }); });