Remove unncessary CanvasFactory/CMapReaderFactory/FileReaderFactory duplication in unit-tests

Given that the API will now, after PR 12039, automatically pick the correct factories to use depending on the environment (browser vs. Node.js), we can utilize that in the unit-tests as well. This way we don't have to manually repeat the same initialization code in *multiple* unit-tests. *Note:* The *official* PDF.js API is defined in `src/pdf.js`, hence the new exports in `src/display/api.js` will not affect that. Also, updates the unit-test `FileReaderFactory` helpers similarily. *Drive-by change:* Fix the `CMapReaderFactory` usage in the annotation unit-tests, since the cache should only contain raw data and not a Promise. While this obviously works as-is, having unit-tests that "abuse" the intended data format can easily lead to unnecessary failures if changes are made to the relevant `src/core/` code.
2021-01-08 17:12:58 +01:00 · 2021-01-08 17:12:58 +01:00 · 941b65f683
commit 941b65f683
parent 35845d1bbb
6 changed files with 57 additions and 115 deletions
--- a/src/display/api.js
+++ b/src/display/api.js
@ -3060,4 +3060,6 @@ export {
  setPDFNetworkStreamFactory,
  version,
  build,
+  DefaultCanvasFactory,
+  DefaultCMapReaderFactory,
 };
--- a/test/unit/annotation_spec.js
+++ b/test/unit/annotation_spec.js
@ -29,21 +29,14 @@ import {
  stringToBytes,
  stringToUTF8String,
 } from "../../src/shared/util.js";
-import { createIdFactory, XRefMock } from "./test_utils.js";
+import { CMAP_PARAMS, createIdFactory, XRefMock } from "./test_utils.js";
 import { Dict, Name, Ref, RefSetCache } from "../../src/core/primitives.js";
 import { Lexer, Parser } from "../../src/core/parser.js";
-import { DOMCMapReaderFactory } from "../../src/display/display_utils.js";
-import { isNodeJS } from "../../src/shared/is_node.js";
-import { NodeCMapReaderFactory } from "../../src/display/node_utils.js";
+import { DefaultCMapReaderFactory } from "../../src/display/api.js";
 import { PartialEvaluator } from "../../src/core/evaluator.js";
 import { StringStream } from "../../src/core/stream.js";
 import { WorkerTask } from "../../src/core/worker.js";

-const cMapUrl = {
-  dom: "../../external/bcmaps/",
-  node: "./external/bcmaps/",
-};
-
 describe("annotation", function () {
  class PDFManagerMock {
    constructor(params) {
@ -86,32 +79,24 @@ describe("annotation", function () {

  let pdfManagerMock, idFactoryMock, partialEvaluator;

-  beforeAll(function (done) {
+  beforeAll(async function (done) {
    pdfManagerMock = new PDFManagerMock({
      docBaseUrl: null,
    });

-    let CMapReaderFactory;
-    if (isNodeJS) {
-      CMapReaderFactory = new NodeCMapReaderFactory({
-        baseUrl: cMapUrl.node,
-        isCompressed: true,
+    const CMapReaderFactory = new DefaultCMapReaderFactory({
+      baseUrl: CMAP_PARAMS.cMapUrl,
+      isCompressed: CMAP_PARAMS.cMapPacked,
    });
-    } else {
-      CMapReaderFactory = new DOMCMapReaderFactory({
-        baseUrl: cMapUrl.dom,
-        isCompressed: true,
-      });
-    }

    const builtInCMapCache = new Map();
    builtInCMapCache.set(
      "UniJIS-UTF16-H",
-      CMapReaderFactory.fetch({ name: "UniJIS-UTF16-H" })
+      await CMapReaderFactory.fetch({ name: "UniJIS-UTF16-H" })
    );
    builtInCMapCache.set(
      "Adobe-Japan1-UCS2",
-      CMapReaderFactory.fetch({ name: "Adobe-Japan1-UCS2" })
+      await CMapReaderFactory.fetch({ name: "Adobe-Japan1-UCS2" })
    );

    idFactoryMock = createIdFactory(/* pageIndex = */ 0);
--- a/test/unit/api_spec.js
+++ b/test/unit/api_spec.js
@ -15,8 +15,7 @@

 import {
  buildGetDocumentParams,
-  DOMFileReaderFactory,
-  NodeFileReaderFactory,
+  DefaultFileReaderFactory,
  TEST_PDFS_PATH,
 } from "./test_utils.js";
 import {
@ -32,23 +31,22 @@ import {
  StreamType,
 } from "../../src/shared/util.js";
 import {
-  DOMCanvasFactory,
-  RenderingCancelledException,
-  StatTimer,
-} from "../../src/display/display_utils.js";
-import {
+  DefaultCanvasFactory,
  getDocument,
  PDFDataRangeTransport,
  PDFDocumentProxy,
  PDFPageProxy,
  PDFWorker,
 } from "../../src/display/api.js";
+import {
+  RenderingCancelledException,
+  StatTimer,
+} from "../../src/display/display_utils.js";
 import { AutoPrintRegExp } from "../../web/ui_utils.js";
 import { GlobalImageCache } from "../../src/core/image_utils.js";
 import { GlobalWorkerOptions } from "../../src/display/worker_options.js";
 import { isNodeJS } from "../../src/shared/is_node.js";
 import { Metadata } from "../../src/display/metadata.js";
-import { NodeCanvasFactory } from "../../src/display/node_utils.js";

 describe("api", function () {
  const basicApiFileName = "basicapi.pdf";
@ -58,11 +56,7 @@ describe("api", function () {
  let CanvasFactory;

  beforeAll(function (done) {
-    if (isNodeJS) {
-      CanvasFactory = new NodeCanvasFactory();
-    } else {
-      CanvasFactory = new DOMCanvasFactory();
-    }
+    CanvasFactory = new DefaultCanvasFactory();
    done();
  });

@ -132,16 +126,9 @@ describe("api", function () {
        .catch(done.fail);
    });
    it("creates pdf doc from typed array", function (done) {
-      let typedArrayPdfPromise;
-      if (isNodeJS) {
-        typedArrayPdfPromise = NodeFileReaderFactory.fetch({
-          path: TEST_PDFS_PATH.node + basicApiFileName,
+      const typedArrayPdfPromise = DefaultFileReaderFactory.fetch({
+        path: TEST_PDFS_PATH + basicApiFileName,
      });
-      } else {
-        typedArrayPdfPromise = DOMFileReaderFactory.fetch({
-          path: TEST_PDFS_PATH.dom + basicApiFileName,
-        });
-      }

      typedArrayPdfPromise
        .then(typedArrayPdf => {
@ -2197,15 +2184,9 @@ describe("api", function () {

    beforeAll(function (done) {
      const fileName = "tracemonkey.pdf";
-      if (isNodeJS) {
-        dataPromise = NodeFileReaderFactory.fetch({
-          path: TEST_PDFS_PATH.node + fileName,
+      dataPromise = DefaultFileReaderFactory.fetch({
+        path: TEST_PDFS_PATH + fileName,
      });
-      } else {
-        dataPromise = DOMFileReaderFactory.fetch({
-          path: TEST_PDFS_PATH.dom + fileName,
-        });
-      }
      done();
    });

--- a/test/unit/cmap_spec.js
+++ b/test/unit/cmap_spec.js
@ -14,35 +14,20 @@
 */

 import { CMap, CMapFactory, IdentityCMap } from "../../src/core/cmap.js";
-import { DOMCMapReaderFactory } from "../../src/display/display_utils.js";
-import { isNodeJS } from "../../src/shared/is_node.js";
+import { CMAP_PARAMS } from "./test_utils.js";
+import { DefaultCMapReaderFactory } from "../../src/display/api.js";
 import { Name } from "../../src/core/primitives.js";
-import { NodeCMapReaderFactory } from "../../src/display/node_utils.js";
 import { StringStream } from "../../src/core/stream.js";

-const cMapUrl = {
-  dom: "../../external/bcmaps/",
-  node: "./external/bcmaps/",
-};
-const cMapPacked = true;
-
 describe("cmap", function () {
  let fetchBuiltInCMap;

  beforeAll(function (done) {
    // Allow CMap testing in Node.js, e.g. for Travis.
-    let CMapReaderFactory;
-    if (isNodeJS) {
-      CMapReaderFactory = new NodeCMapReaderFactory({
-        baseUrl: cMapUrl.node,
-        isCompressed: cMapPacked,
+    const CMapReaderFactory = new DefaultCMapReaderFactory({
+      baseUrl: CMAP_PARAMS.cMapUrl,
+      isCompressed: CMAP_PARAMS.cMapPacked,
    });
-    } else {
-      CMapReaderFactory = new DOMCMapReaderFactory({
-        baseUrl: cMapUrl.dom,
-        isCompressed: cMapPacked,
-      });
-    }

    fetchBuiltInCMap = function (name) {
      return CMapReaderFactory.fetch({
@ -298,9 +283,8 @@ describe("cmap", function () {

  it("attempts to load a built-in CMap without the necessary API parameters", function (done) {
    function tmpFetchBuiltInCMap(name) {
-      const CMapReaderFactory = isNodeJS
-        ? new NodeCMapReaderFactory({})
-        : new DOMCMapReaderFactory({});
+      const CMapReaderFactory = new DefaultCMapReaderFactory({});
+
      return CMapReaderFactory.fetch({
        name,
      });
@ -328,18 +312,11 @@ describe("cmap", function () {

  it("attempts to load a built-in CMap with inconsistent API parameters", function (done) {
    function tmpFetchBuiltInCMap(name) {
-      let CMapReaderFactory;
-      if (isNodeJS) {
-        CMapReaderFactory = new NodeCMapReaderFactory({
-          baseUrl: cMapUrl.node,
+      const CMapReaderFactory = new DefaultCMapReaderFactory({
+        baseUrl: CMAP_PARAMS.cMapUrl,
        isCompressed: false,
      });
-      } else {
-        CMapReaderFactory = new DOMCMapReaderFactory({
-          baseUrl: cMapUrl.dom,
-          isCompressed: false,
-        });
-      }
+
      return CMapReaderFactory.fetch({
        name,
      });
--- a/test/unit/custom_spec.js
+++ b/test/unit/custom_spec.js
@ -13,11 +13,8 @@
 * limitations under the License.
 */

+import { DefaultCanvasFactory, getDocument } from "../../src/display/api.js";
 import { buildGetDocumentParams } from "./test_utils.js";
-import { DOMCanvasFactory } from "../../src/display/display_utils.js";
-import { getDocument } from "../../src/display/api.js";
-import { isNodeJS } from "../../src/shared/is_node.js";
-import { NodeCanvasFactory } from "../../src/display/node_utils.js";

 function getTopLeftPixel(canvasContext) {
  const imgData = canvasContext.getImageData(0, 0, 1, 1);
@ -39,11 +36,8 @@ describe("custom canvas rendering", function () {
  let page;

  beforeAll(function (done) {
-    if (isNodeJS) {
-      CanvasFactory = new NodeCanvasFactory();
-    } else {
-      CanvasFactory = new DOMCanvasFactory();
-    }
+    CanvasFactory = new DefaultCanvasFactory();
+
    loadingTask = getDocument(transparentGetDocumentParams);
    loadingTask.promise
      .then(function (doc) {
@ -156,10 +150,8 @@ describe("custom ownerDocument", function () {
        getElementsByTagName: () => [{ appendChild: () => {} }],
      },
    };
+    const CanvasFactory = new DefaultCanvasFactory({ ownerDocument });

-    const CanvasFactory = isNodeJS
-      ? new NodeCanvasFactory()
-      : new DOMCanvasFactory({ ownerDocument });
    return {
      elements,
      ownerDocument,
--- a/test/unit/test_utils.js
+++ b/test/unit/test_utils.js
@ -19,6 +19,13 @@ import { assert } from "../../src/shared/util.js";
 import { isNodeJS } from "../../src/shared/is_node.js";
 import { StringStream } from "../../src/core/stream.js";

+const TEST_PDFS_PATH = isNodeJS ? "./test/pdfs/" : "../pdfs/";
+
+const CMAP_PARAMS = {
+  cMapUrl: isNodeJS ? "./external/bcmaps/" : "../../external/bcmaps/",
+  cMapPacked: true,
+};
+
 class DOMFileReaderFactory {
  static async fetch(params) {
    const response = await fetch(params.path);
@ -45,18 +52,16 @@ class NodeFileReaderFactory {
  }
 }

-const TEST_PDFS_PATH = {
-  dom: "../pdfs/",
-  node: "./test/pdfs/",
-};
+const DefaultFileReaderFactory = isNodeJS
+  ? NodeFileReaderFactory
+  : DOMFileReaderFactory;

 function buildGetDocumentParams(filename, options) {
  const params = Object.create(null);
-  if (isNodeJS) {
-    params.url = TEST_PDFS_PATH.node + filename;
-  } else {
-    params.url = new URL(TEST_PDFS_PATH.dom + filename, window.location).href;
-  }
+  params.url = isNodeJS
+    ? TEST_PDFS_PATH + filename
+    : new URL(TEST_PDFS_PATH + filename, window.location).href;
+
  for (const option in options) {
    params[option] = options[option];
  }
@ -136,11 +141,11 @@ function isEmptyObj(obj) {
 }

 export {
-  DOMFileReaderFactory,
-  NodeFileReaderFactory,
+  DefaultFileReaderFactory,
  XRefMock,
  buildGetDocumentParams,
  TEST_PDFS_PATH,
+  CMAP_PARAMS,
  createIdFactory,
  isEmptyObj,
 };