From fe205efd8d5f70a909d3682257bb596a79f1ce6f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 5 Nov 2021 14:29:02 +0100 Subject: [PATCH] Add a couple of basic unit-tests for `PDFPageViewBuffer` The `PDFPageViewBuffer`-code is very important for the correct function of the viewer, but it's currently not tested at all. While the `PDFPageViewBuffer` is obviously intended to be used with `PDFPageView`-instances, it only accesses a couple of `PDFPageView` properties/methods and consequently it's fairly easy to unit-test this code with dummy-data. These unit-tests should help improve our confidence in this code, and will also come in handy with other changes that I'm working on (regarding modernizing and re-factoring the `PDFPageViewBuffer`-code). --- test/unit/base_viewer_spec.js | 162 ++++++++++++++++++++++++++++++++++ test/unit/clitests.json | 1 + test/unit/jasmine-boot.js | 1 + web/base_viewer.js | 13 ++- 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 test/unit/base_viewer_spec.js diff --git a/test/unit/base_viewer_spec.js b/test/unit/base_viewer_spec.js new file mode 100644 index 000000000..c66a1a5fc --- /dev/null +++ b/test/unit/base_viewer_spec.js @@ -0,0 +1,162 @@ +/* Copyright 2021 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { PDFPageViewBuffer } from "../../web/base_viewer.js"; + +describe("BaseViewer", function () { + describe("PDFPageViewBuffer", function () { + function createViewsMap(startId, endId) { + const map = new Map(); + + for (let id = startId; id <= endId; id++) { + map.set(id, { + id, + destroy: () => {}, + }); + } + return map; + } + + it("handles `push` correctly", function () { + const buffer = new PDFPageViewBuffer(3); + + const viewsMap = createViewsMap(1, 5), + iterator = viewsMap.values(); + + for (let i = 0; i < 3; i++) { + const view = iterator.next().value; + buffer.push(view); + } + // Ensure that the correct views are inserted. + expect(buffer._buffer).toEqual([ + viewsMap.get(1), + viewsMap.get(2), + viewsMap.get(3), + ]); + + for (let i = 3; i < 5; i++) { + const view = iterator.next().value; + buffer.push(view); + } + // Ensure that the correct views are evicted. + expect(buffer._buffer).toEqual([ + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + ]); + }); + + it("handles `resize` correctly", function () { + const buffer = new PDFPageViewBuffer(5); + + const viewsMap = createViewsMap(1, 5), + iterator = viewsMap.values(); + + for (let i = 0; i < 5; i++) { + const view = iterator.next().value; + buffer.push(view); + } + // Ensure that keeping the size constant won't evict any views. + buffer.resize(5); + + expect(buffer._buffer).toEqual([ + viewsMap.get(1), + viewsMap.get(2), + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + ]); + + // Ensure that increasing the size won't evict any views. + buffer.resize(10); + + expect(buffer._buffer).toEqual([ + viewsMap.get(1), + viewsMap.get(2), + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + ]); + + // Ensure that decreasing the size will evict the correct views. + buffer.resize(3); + + expect(buffer._buffer).toEqual([ + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + ]); + }); + + it("handles `resize` correctly, with `idsToKeep` provided", function () { + const buffer = new PDFPageViewBuffer(5); + + const viewsMap = createViewsMap(1, 5), + iterator = viewsMap.values(); + + for (let i = 0; i < 5; i++) { + const view = iterator.next().value; + buffer.push(view); + } + // Ensure that keeping the size constant won't evict any views, + // while re-ordering them correctly. + buffer.resize(5, new Set([1, 2])); + + expect(buffer._buffer).toEqual([ + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + viewsMap.get(1), + viewsMap.get(2), + ]); + + // Ensure that increasing the size won't evict any views, + // while re-ordering them correctly. + buffer.resize(10, new Set([3, 4, 5])); + + expect(buffer._buffer).toEqual([ + viewsMap.get(1), + viewsMap.get(2), + viewsMap.get(3), + viewsMap.get(4), + viewsMap.get(5), + ]); + + // Ensure that decreasing the size will evict the correct views, + // while re-ordering the remaining ones correctly. + buffer.resize(3, new Set([1, 2, 3])); + + expect(buffer._buffer).toEqual([ + viewsMap.get(1), + viewsMap.get(2), + viewsMap.get(3), + ]); + }); + + it("handles `has` correctly", function () { + const buffer = new PDFPageViewBuffer(3); + + const viewsMap = createViewsMap(1, 2), + iterator = viewsMap.values(); + + for (let i = 0; i < 1; i++) { + const view = iterator.next().value; + buffer.push(view); + } + expect(buffer.has(viewsMap.get(1))).toEqual(true); + expect(buffer.has(viewsMap.get(2))).toEqual(false); + }); + }); +}); diff --git a/test/unit/clitests.json b/test/unit/clitests.json index 315d03a79..9d7352d5a 100644 --- a/test/unit/clitests.json +++ b/test/unit/clitests.json @@ -9,6 +9,7 @@ "annotation_spec.js", "annotation_storage_spec.js", "api_spec.js", + "base_viewer_spec.js", "bidi_spec.js", "cff_parser_spec.js", "cmap_spec.js", diff --git a/test/unit/jasmine-boot.js b/test/unit/jasmine-boot.js index 5dcbd8992..f73287ec2 100644 --- a/test/unit/jasmine-boot.js +++ b/test/unit/jasmine-boot.js @@ -54,6 +54,7 @@ async function initializePDFJS(callback) { "pdfjs-test/unit/annotation_spec.js", "pdfjs-test/unit/annotation_storage_spec.js", "pdfjs-test/unit/api_spec.js", + "pdfjs-test/unit/base_viewer_spec.js", "pdfjs-test/unit/bidi_spec.js", "pdfjs-test/unit/cff_parser_spec.js", "pdfjs-test/unit/cmap_spec.js", diff --git a/web/base_viewer.js b/web/base_viewer.js index ed7be475e..bab737122 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -127,6 +127,17 @@ function PDFPageViewBuffer(size) { this.has = function (view) { return data.includes(view); }; + + if ( + typeof PDFJSDev === "undefined" || + PDFJSDev.test("!PRODUCTION || TESTING") + ) { + Object.defineProperty(this, "_buffer", { + get() { + return data.slice(); + }, + }); + } } function isSameScale(oldScale, newScale) { @@ -1879,4 +1890,4 @@ class BaseViewer { } } -export { BaseViewer }; +export { BaseViewer, PDFPageViewBuffer };