Skip to content

Commit

Permalink
Introduce Puppeteer for handling browsers during tests
Browse files Browse the repository at this point in the history
This commit replaces our own infrastructure for handling browsers during
tests with Puppeteer. Using our own infrastructure for this had a few
downsides:

- It has proven to not always be reliable, especially when closing the
  browser, causing failures on the bots because browsers were still
  running even though they should have been stopped. Puppeteer should do
  a better job with this because it uses the browser's test built-in
  instrumentation tools for this (the devtools protocol) which our code
  didn't. This also means that we don't have to pass
  parameters/preferences to tweak browser behavior anymore.
- It requires the browsers under test to be installed on the system,
  whereas Puppeteer downloads the browsers before the test. This means
  that setup is much easier (no more manual installations and browser
  manifest files) as well as testing with different browser versions
  (since they can be provisioned on demand). Moreover, this ensures that
  contributors always run the tests in both Firefox and Chrome,
  regardless of which browsers they have installed locally.
- It's all code we have to maintain, so Puppeteer abstracts away how the
  browsers start/stop for us so we don't have to keep that code.
  • Loading branch information
timvandermeij committed Apr 12, 2020
1 parent 702fec5 commit 9b80d76
Show file tree
Hide file tree
Showing 12 changed files with 379 additions and 452 deletions.
32 changes: 0 additions & 32 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,21 +417,6 @@ function createTestSource(testsName, bot) {
console.log("### Running " + testsName + " tests");

var PDF_TEST = process.env["PDF_TEST"] || "test_manifest.json";
var PDF_BROWSERS =
process.env["PDF_BROWSERS"] ||
"resources/browser_manifests/browser_manifest.json";

if (!checkFile("test/" + PDF_BROWSERS)) {
console.log(
"Browser manifest file test/" + PDF_BROWSERS + " does not exist."
);
console.log(
"Copy and adjust the example in test/resources/browser_manifests."
);
this.emit("error", new Error("Missing manifest file"));
return null;
}

var args = ["test.js"];
switch (testsName) {
case "browser":
Expand All @@ -450,7 +435,6 @@ function createTestSource(testsName, bot) {
this.emit("error", new Error("Unknown name: " + testsName));
return null;
}
args.push("--browserManifestFile=" + PDF_BROWSERS);
if (bot) {
args.push("--strictVerify");
}
Expand All @@ -468,26 +452,10 @@ function makeRef(done, bot) {
console.log();
console.log("### Creating reference images");

var PDF_BROWSERS =
process.env["PDF_BROWSERS"] ||
"resources/browser_manifests/browser_manifest.json";

if (!checkFile("test/" + PDF_BROWSERS)) {
console.log(
"Browser manifest file test/" + PDF_BROWSERS + " does not exist."
);
console.log(
"Copy and adjust the example in test/resources/browser_manifests."
);
done(new Error("Missing manifest file"));
return;
}

var args = ["test.js", "--masterMode"];
if (bot) {
args.push("--noPrompts", "--strictVerify");
}
args.push("--browserManifestFile=" + PDF_BROWSERS);
var testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" });
testProcess.on("close", function(code) {
done();
Expand Down

0 comments on commit 9b80d76

Please sign in to comment.