Skip to content

Commit b28725c

Browse files
authored
fix(screenshots): prevent infinite blocking (#3)
Global functions alert and confirm block and never return without user action which won't happen in headless mode. This fix makes these functions return immediately. This also improves tests to catch more issues with screenshots (like timing out or not generating one per example).
1 parent 71dd0c3 commit b28725c

File tree

9 files changed

+111
-48
lines changed

9 files changed

+111
-48
lines changed

.circleci/config.yml

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@
44
#
55
version: 2.1
66

7+
commands:
8+
install-deps-for-screenshot-taking:
9+
description: Install headless Chromium dependencies
10+
steps:
11+
- run:
12+
name: Install Headless Chromium dependencies
13+
command: |
14+
sudo apt-get install -yq \
15+
ca-certificates fonts-liberation gconf-service libappindicator1 \
16+
libasound2 libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 \
17+
libdbus-1-3 libexpat1 libfontconfig1 libgcc1 libgconf-2-4 \
18+
libgdk-pixbuf2.0-0 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 \
19+
libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 \
20+
libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 \
21+
libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release wget \
22+
xdg-utils
23+
724
executors:
825
node:
926
docker:
@@ -39,7 +56,7 @@ jobs:
3956
- persist_to_workspace:
4057
root: .
4158
paths:
42-
- '*'
59+
- "*"
4360

4461
build:
4562
executor: node
@@ -53,7 +70,7 @@ jobs:
5370
- persist_to_workspace:
5471
root: .
5572
paths:
56-
- 'bin'
73+
- "bin"
5774

5875
lint:
5976
executor: node
@@ -71,6 +88,8 @@ jobs:
7188
- attach_workspace:
7289
at: .
7390

91+
- install-deps-for-screenshot-taking
92+
7493
- run: npm run test
7594

7695
release:

@types/index.d.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,8 @@ declare module "*.html" {
77
const value: string;
88
export default value;
99
}
10+
11+
declare module "*.txt" {
12+
const value: string;
13+
export default value;
14+
}

rollup.config.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ export default ["generate-examples-index"].map(name => {
2626
external,
2727
plugins: [
2828
nodeResolve({
29-
extensions: [".ts", ".js", ".json", ".css", ".html"],
29+
extensions: [".css", ".html", ".js", ".json", ".ts", ".txt"],
3030
preferBuiltins: true
3131
}),
3232
string({
33-
include: ["**/*.{css,html}"]
33+
include: ["**/*.{css,html,txt}"]
3434
}),
3535
typescript({
3636
objectHashIgnoreUnknownHack: true,

src/generate-examples-index/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import Pageres from "pageres";
21
import $ from "cheerio";
2+
import Pageres from "pageres";
33
import childProcess from "child_process";
4+
import commonScreenshotScript from "./screenshot-script.js.txt";
45
import crypto from "crypto";
56
import fs from "fs";
67
import globby from "globby";
@@ -530,6 +531,9 @@ class ContentBuilder {
530531
);
531532
const screenshotPage = $.load(example.html);
532533
screenshotPage("head").prepend(
534+
$("<script>")
535+
.attr("type", "text/javascript")
536+
.text(commonScreenshotScript),
533537
$("<script>")
534538
.attr("type", "text/javascript")
535539
.text(this._screenshotScript)

src/generate-examples-index/screenshot-script.js

Lines changed: 0 additions & 25 deletions
This file was deleted.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/* global window: false */
2+
3+
/**
4+
* Simulates confirmation usind window.confirm and window.alert otherwise the
5+
* screenshot generator would just hang forever.
6+
*/
7+
(function() {
8+
window.alert = function() {};
9+
window.confirm = function() {
10+
return true;
11+
};
12+
})();

test/generate-examples-index/basic.test.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,60 +3,76 @@ import childProcess from "child_process";
33
import fs from "fs";
44
import path from "path";
55
import tmp from "tmp-promise";
6+
import util from "util";
67
import { expect } from "chai";
78

8-
const exec = childProcess.execSync;
9+
const exec = util.promisify(childProcess.exec);
910
const executable = path.resolve(
1011
`${exec("npm root")}/../bin/generate-examples-index.js`
1112
);
1213
const examplesPath = path.resolve(`${__dirname}/examples`);
1314

1415
const baseURL = "https://visjs.github.io/vis-test";
16+
const nmExamples = 6;
1517
const title = "Test Examples";
1618

1719
describe("generate-examples-index", function(): void {
18-
const { name: output } = tmp.fileSync({ mode: 0o600, postfix: "html" });
20+
const { name: outputDir } = tmp.dirSync({ mode: 0o700 });
21+
const outputIndex = path.join(outputDir, "index.html");
22+
const outputScreenshots = path.join(outputDir, "thumbnails");
1923
let $index: CheerioStatic;
2024

21-
it("built", function(): void {
25+
it("is executable built", function(): void {
2226
expect(
2327
fs.existsSync(executable),
2428
"The built executable has to be present for any of the following tests to pass."
2529
).to.be.true;
2630
});
2731

28-
it("generate index", function(): void {
29-
this.timeout(60000);
32+
it("generate index", async function(): Promise<void> {
33+
this.timeout(10 * 60 * 1000);
3034

31-
exec(
35+
await exec(
3236
[
3337
"node",
3438
executable,
35-
"-c test-element",
36-
"-i",
37-
`-d "${examplesPath}"`,
38-
`-o ${output}`,
39-
`-t "${title}"`,
40-
`-w "${baseURL}"`
39+
"--container-id test-element",
40+
"--index",
41+
"--screenshots",
42+
`--examples-directory "${examplesPath}"`,
43+
`--output "${outputIndex}"`,
44+
`--title "${title}"`,
45+
`--web-url "${baseURL}"`
4146
].join(" ")
4247
);
43-
44-
$index = $.load(fs.readFileSync(output, "utf-8"));
4548
});
4649

4750
describe("verify index", function(): void {
51+
it("valid HTML", function(): void {
52+
$index = $.load(fs.readFileSync(outputIndex, "utf-8"));
53+
});
54+
4855
it("title", function(): void {
4956
expect($index("title").text()).to.equal(title);
5057
});
5158

52-
it("examples", function(): void {
59+
it("links", function(): void {
5360
expect(
5461
$index(".example-link").length,
55-
"There should be 4 links to examples."
56-
).to.equal(4);
62+
`There should be ${nmExamples} links to examples.`
63+
).to.equal(nmExamples);
64+
});
65+
66+
it("screenshots", function(): void {
67+
expect(
68+
fs.readdirSync(outputScreenshots),
69+
`There should be ${nmExamples} screenshots.`
70+
).have.lengthOf(nmExamples);
5771
});
5872

59-
for (const i of [0, 1, 2, 3]) {
73+
for (const i of new Array(nmExamples)
74+
.fill(null)
75+
.map((_value, i): number => i)) {
6076
describe(`example ${i + 1}`, function(): void {
6177
function getNthExample(n: number): Cheerio {
6278
return $index(".example-link").eq(n);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Test | Alert</title>
5+
6+
<script type="text/javascript" src="../dist/test.min.js"></script>
7+
8+
<script type="text/javascript">
9+
alert("This is just a test.");
10+
</script>
11+
</head>
12+
13+
<body>
14+
<div id="test-element"></div>
15+
</body>
16+
</html>
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Test | Confirm</title>
5+
6+
<script type="text/javascript" src="../dist/test.min.js"></script>
7+
8+
<script type="text/javascript">
9+
Confirm("This is just a test.");
10+
</script>
11+
</head>
12+
13+
<body>
14+
<div id="test-element"></div>
15+
</body>
16+
</html>

0 commit comments

Comments
 (0)