From 28b0220bc2ababce340ee0a1e8a80026a6999446 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sun, 24 Nov 2024 23:05:59 +0100 Subject: [PATCH] Replace createTemporaryNodeServer with TestPdfsServer Some tests rely on the presence of a server that serves PDF files. When tests are run from a web browser, the test files and PDF files are served by the same server (WebServer), but in Node.js that server is not around. Currently, the tests that depend on it start a minimal Node.js server that re-implements part of the functionality from WebServer. To avoid code duplication when tests depend on more complex behaviors, this patch replaces createTemporaryNodeServer with the existing WebServer, wrapped in a new test utility that has the same interface in Node.js and non-Node.js environments (=TestPdfsServer). This patch has been tested by running the refactored tests in the following three configurations: 1. From the browser: - http://localhost:8888/test/unit/unit_test.html?spec=api - http://localhost:8888/test/unit/unit_test.html?spec=fetch_stream 2. Run specific tests directly with jasmine without legacy bundling: `JASMINE_CONFIG_PATH=test/unit/clitests.json ./node_modules/.bin/jasmine --filter='^api|^fetch_stream'` 3. `gulp unittestcli` --- test/unit/api_spec.js | 24 ++---- test/unit/fetch_stream_spec.js | 27 ++----- test/unit/test_utils.js | 141 ++++++++++++++++++++------------- 3 files changed, 98 insertions(+), 94 deletions(-) diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index d73d23225..412ddc918 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -31,9 +31,9 @@ import { import { buildGetDocumentParams, CMAP_URL, - createTemporaryNodeServer, DefaultFileReaderFactory, TEST_PDFS_PATH, + TestPdfsServer, } from "./test_utils.js"; import { DefaultCanvasFactory, @@ -67,27 +67,17 @@ describe("api", function () { buildGetDocumentParams(tracemonkeyFileName); let CanvasFactory; - let tempServer = null; - beforeAll(function () { + beforeAll(async function () { CanvasFactory = new DefaultCanvasFactory({}); - if (isNodeJS) { - tempServer = createTemporaryNodeServer(); - } + await TestPdfsServer.ensureStarted(); }); - afterAll(function () { + afterAll(async function () { CanvasFactory = null; - if (isNodeJS) { - // Close the server from accepting new connections after all test - // finishes. - const { server } = tempServer; - server.close(); - - tempServer = null; - } + await TestPdfsServer.ensureStopped(); }); function waitSome(callback) { @@ -148,9 +138,7 @@ describe("api", function () { }); it("creates pdf doc from URL-object", async function () { - const urlObj = isNodeJS - ? new URL(`http://127.0.0.1:${tempServer.port}/${basicApiFileName}`) - : new URL(TEST_PDFS_PATH + basicApiFileName, window.location); + const urlObj = TestPdfsServer.resolveURL(basicApiFileName); const loadingTask = getDocument(urlObj); expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); diff --git a/test/unit/fetch_stream_spec.js b/test/unit/fetch_stream_spec.js index 3fbe1efc1..cc9fcbbb5 100644 --- a/test/unit/fetch_stream_spec.js +++ b/test/unit/fetch_stream_spec.js @@ -13,35 +13,22 @@ * limitations under the License. */ -import { AbortException, isNodeJS } from "../../src/shared/util.js"; -import { createTemporaryNodeServer } from "./test_utils.js"; +import { AbortException } from "../../src/shared/util.js"; import { PDFFetchStream } from "../../src/display/fetch_stream.js"; +import { TestPdfsServer } from "./test_utils.js"; describe("fetch_stream", function () { - let tempServer = null; - function getPdfUrl() { - return isNodeJS - ? `http://127.0.0.1:${tempServer.port}/tracemonkey.pdf` - : new URL("../pdfs/tracemonkey.pdf", window.location).href; + return TestPdfsServer.resolveURL("tracemonkey.pdf").href; } const pdfLength = 1016315; - beforeAll(function () { - if (isNodeJS) { - tempServer = createTemporaryNodeServer(); - } + beforeAll(async function () { + await TestPdfsServer.ensureStarted(); }); - afterAll(function () { - if (isNodeJS) { - // Close the server from accepting new connections after all test - // finishes. - const { server } = tempServer; - server.close(); - - tempServer = null; - } + afterAll(async function () { + await TestPdfsServer.ensureStopped(); }); it("read with streaming", async function () { diff --git a/test/unit/test_utils.js b/test/unit/test_utils.js index db2fdc2fd..d08098fdc 100644 --- a/test/unit/test_utils.js +++ b/test/unit/test_utils.js @@ -122,73 +122,102 @@ function createIdFactory(pageIndex) { return page._localIdFactory; } -function createTemporaryNodeServer() { - assert(isNodeJS, "Should only be used in Node.js environments."); +// Some tests rely on special behavior from webserver.mjs. When loaded in the +// browser, the page is already served from WebServer. When running from +// Node.js, that is not the case. This helper starts the WebServer if needed, +// and offers a mechanism to resolve the URL in a uniform way. +class TestPdfsServer { + static #webServer; - const fs = process.getBuiltinModule("fs"), - http = process.getBuiltinModule("http"); - function isAcceptablePath(requestUrl) { - try { - // Reject unnormalized paths, to protect against path traversal attacks. - const url = new URL(requestUrl, "https://localhost/"); - return url.pathname === requestUrl; - } catch { - return false; + static #startCount = 0; + + static #startPromise; + + static async ensureStarted() { + if (this.#startCount++) { + // Already started before. E.g. from another beforeAll call. + return this.#startPromise; + } + if (!isNodeJS) { + // In web browsers, tests are presumably served by webserver.mjs. + return undefined; } - } - // Create http server to serve pdf data for tests. - const server = http - .createServer((request, response) => { - if (!isAcceptablePath(request.url)) { - response.writeHead(400); - response.end("Invalid path"); - return; - } - const filePath = process.cwd() + "/test/pdfs" + request.url; - fs.promises.lstat(filePath).then( - stat => { - if (!request.headers.range) { - const contentLength = stat.size; - const stream = fs.createReadStream(filePath); - response.writeHead(200, { - "Content-Type": "application/pdf", - "Content-Length": contentLength, - "Accept-Ranges": "bytes", - }); - stream.pipe(response); - } else { - const [start, end] = request.headers.range - .split("=")[1] - .split("-") - .map(x => Number(x)); - const stream = fs.createReadStream(filePath, { start, end }); - response.writeHead(206, { - "Content-Type": "application/pdf", - }); - stream.pipe(response); - } - }, - error => { - response.writeHead(404); - response.end(`File ${request.url} not found!`); - } - ); - }) - .listen(0); /* Listen on a random free port */ - return { - server, - port: server.address().port, - }; + this.#startPromise = this.#startServer().finally(() => { + this.#startPromise = null; + }); + return this.#startPromise; + } + + static async #startServer() { + // WebServer from webserver.mjs is imported dynamically instead of + // statically because we do not need it when running from the browser. + let WebServer; + if (import.meta.url.endsWith("/lib-legacy/test/unit/test_utils.js")) { + // When "gulp unittestcli" is used to run tests, the tests are run from + // pdf.js/build/lib-legacy/test/ instead of directly from pdf.js/test/. + // eslint-disable-next-line import/no-unresolved + ({ WebServer } = await import("../../../../test/webserver.mjs")); + } else { + ({ WebServer } = await import("../webserver.mjs")); + } + this.#webServer = new WebServer({ + host: "127.0.0.1", + root: TEST_PDFS_PATH, + }); + await new Promise(resolve => { + this.#webServer.start(resolve); + }); + } + + static async ensureStopped() { + assert(this.#startCount > 0, "ensureStarted() should be called first"); + assert(!this.#startPromise, "ensureStarted() should have resolved"); + if (--this.#startCount) { + // Keep server alive as long as there is an ensureStarted() that was not + // followed by an ensureStopped() call. + // This could happen if ensureStarted() was called again before + // ensureStopped() was called from afterAll(). + return; + } + if (!isNodeJS) { + // Web browsers cannot stop the server. + return; + } + + await new Promise(resolve => { + this.#webServer.stop(resolve); + this.#webServer = null; + }); + } + + /** + * @param {string} path - path to file within test/unit/pdf/ (TEST_PDFS_PATH). + * @returns {URL} + */ + static resolveURL(path) { + assert(this.#startCount > 0, "ensureStarted() should be called first"); + assert(!this.#startPromise, "ensureStarted() should have resolved"); + + if (isNodeJS) { + // Note: TestPdfsServer.ensureStarted() should be called first. + return new URL(path, `http://127.0.0.1:${this.#webServer.port}/`); + } + // When "gulp server" is used, our URL looks like + // http://localhost:8888/test/unit/unit_test.html + // The PDFs are served from: + // http://localhost:8888/test/pdfs/ + return new URL(TEST_PDFS_PATH + path, window.location); + } } export { buildGetDocumentParams, CMAP_URL, createIdFactory, - createTemporaryNodeServer, DefaultFileReaderFactory, STANDARD_FONT_DATA_URL, TEST_PDFS_PATH, + TestPdfsServer, XRefMock, };