Skip to content

Commit

Permalink
♻️ Use node's http server for snapshot api and tests (#46)
Browse files Browse the repository at this point in the history
* ✅ Use node server for testing

* ✨ Use node server for snapshot api

* ✨ Use serve-handler to serve static directories

* ✅ Update tests to use included node-server

* ✅ Test cors preflight request

* 🔥 Remove unneeded headers for requests

* 📦 Distribute core test-server helper

* ✅ Rename test helper

* ✅ Use alias to achieve coverage of testing server
  • Loading branch information
Wil Wilsman authored Sep 25, 2020
1 parent 4fd9d6e commit b28c085
Show file tree
Hide file tree
Showing 18 changed files with 298 additions and 336 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@babel/preset-env": "^7.11.5",
"@babel/register": "^7.11.5",
"@oclif/dev-cli": "^1.22.2",
"babel-plugin-module-resolver": "^4.0.0",
"babel-plugin-istanbul": "^6.0.0",
"cross-env": "^7.0.2",
"eslint": "^7.9.0",
Expand Down
3 changes: 1 addition & 2 deletions packages/cli-snapshot/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@
"@percy/core": "^1.0.0-beta.13",
"@percy/dom": "^1.0.0-beta.13",
"@percy/logger": "^1.0.0-beta.13",
"cors": "^2.8.5",
"express": "^4.17.1",
"globby": "^11.0.1",
"serve-handler": "^6.1.3",
"yaml": "^1.10.0"
}
}
11 changes: 5 additions & 6 deletions packages/cli-snapshot/src/commands/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,13 @@ export class Snapshot extends Command {

// Serves a static directory at a base-url and resolves when listening.
async serve(staticDir, baseUrl) {
let express = require('express');
let app = express();

app.use(require('cors')());
app.use(baseUrl, express.static(staticDir));
let http = require('http');
let serve = require('serve-handler');

return new Promise(resolve => {
this.server = app.listen(() => {
this.server = http.createServer((req, res) => {
serve(req, res, { public: staticDir });
}).listen(() => {
let { port } = this.server.address();
resolve(`http://localhost:${port}`);
});
Expand Down
5 changes: 2 additions & 3 deletions packages/cli-snapshot/test/snapshot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,8 @@ describe('percy snapshot', () => {
let server;

beforeEach(async () => {
server = await createTestServer();
server.app.get('/', (req, res) => {
res.set('Content-Type', 'text/html').send('<p>Test</p>');
server = await createTestServer({
default: () => [200, 'text/html', '<p>Test</p>']
});
});

Expand Down
3 changes: 1 addition & 2 deletions packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ await percy.start()
#### API Server

Starting a `Percy` instance will start a local API server unless `server` is `false`. The server can
be found at `http://localhost:5338/` or at the provided `port` number. All POST requests accept a
JSON body with the `application/json` content-type.
be found at `http://localhost:5338/` or at the provided `port` number.

- GET `/percy/healthcheck` – Responds with information about the running instance
- GET `/percy/dom.js` – Responds with the [`@percy/dom`](./packages/dom) library
Expand Down
6 changes: 2 additions & 4 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"types": "types/index.d.ts",
"files": [
"dist",
"types/index.d.ts"
"types/index.d.ts",
"test/helpers/server.js"
],
"scripts": {
"build": "babel --root-mode upward src --out-dir dist",
Expand All @@ -27,9 +28,6 @@
"@percy/client": "^1.0.0-beta.13",
"@percy/dom": "^1.0.0-beta.13",
"@percy/logger": "^1.0.0-beta.13",
"body-parser": "^1.19.0",
"cors": "^2.8.5",
"express": "^4.17.1",
"node-fetch": "^2.6.1",
"progress": "^2.0.3",
"puppeteer-core": "^5.3.1"
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/percy.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import log from '@percy/logger';
import { schema } from './config';
import Discoverer from './discoverer';
import injectPercyCSS from './percy-css';
import { createServerApp, startServer } from './server';
import createPercyServer from './server';
import Queue from './queue';
import assert from './utils/assert';
import { createRootResource, createLogResource } from './utils/resources';
Expand Down Expand Up @@ -54,7 +54,7 @@ export default class Percy {

if (server) {
this.port = port;
this.app = createServerApp(this);
this.server = createPercyServer(this);
}

this.#snapshots = new Queue();
Expand Down Expand Up @@ -90,10 +90,8 @@ export default class Percy {
this.client.getToken();

try {
// if there is an exress app, a server should be started
if (this.app) {
this.server = await startServer(this.app, this.port);
}
// if there is a server, start listening
await this.server?.listen(this.port);

// launch the discoverer browser and create a percy build
await this.discoverer.launch();
Expand Down
154 changes: 97 additions & 57 deletions packages/core/src/server.js
Original file line number Diff line number Diff line change
@@ -1,64 +1,104 @@
// Handles async routes with a middleware pattern to catch and forward errors
function asyncRoute(handler) {
return (req, res, next) => handler(req, res, next).catch(next);
import http from 'http';
import fs from 'fs';

async function getReply(routes, request) {
let route = routes[request.url] || routes.default;
let reply;

// cors preflight
if (request.method === 'OPTIONS') {
reply = [204, { 'Access-Control-Allow-Methods': 'GET,POST' }];
} else {
reply = await Promise.resolve()
.then(() => routes.middleware?.(request))
.then(() => route?.(request))
.catch(routes.catch);
}

// default 404 when reply is not an array
let [status, headers, body] = Array.isArray(reply) ? reply : [404, {}];
// support content-type header shortcut
if (typeof headers === 'string') headers = { 'Content-Type': headers };
// auto stringify json
if (headers['Content-Type']?.includes('json')) body = JSON.stringify(body);
// add content length and cors headers
headers['Content-Length'] = body?.length ?? 0;
headers['Access-Control-Allow-Origin'] = '*';

return [status, headers, body];
}

// Lazily creates and returns an express app for communicating with the Percy
// instance using a local API
export function createServerApp(percy) {
// lazily required to speed up imports when the server is not needed
let express = require('express');
let cors = require('cors');
let bodyParser = require('body-parser');

return express()
.use(cors())
.use(bodyParser.urlencoded({ extended: true }))
.use(bodyParser.json({ limit: '50mb' }))
// healthcheck returns meta info as well
.get('/percy/healthcheck', (_, res) => {
res.json({
success: true,
config: percy.config,
loglevel: percy.loglevel(),
build: percy.client.build
});
})
// responds when idle
.get('/percy/idle', asyncRoute(async (_, res) => {
await percy.idle();
res.json({ success: true });
}))
// serves @percy/dom as a convenience
.get('/percy/dom.js', (_, res) => {
res.sendFile(require.resolve('@percy/dom'));
})
// forward snapshot requests
.post('/percy/snapshot', asyncRoute(async (req, res) => {
await percy.snapshot(req.body);
res.json({ success: true });
}))
// stops the instance
.post('/percy/stop', asyncRoute(async (_, res) => {
await percy.stop();
res.json({ success: true });
}))
// other routes 404
.use('*', (_, res) => {
res.status(404).json({ success: false, error: 'Not found' });
})
// generic error handler
.use(({ message }, req, res, next) => {
res.status(500).json({ success: false, error: message });
export function createServer(routes) {
let context = {
get listening() {
return context.server.listening;
}
};

context.server = http.createServer((request, response) => {
request.on('data', chunk => {
request.body = (request.body || '') + chunk;
});

request.on('end', async () => {
try { request.body = JSON.parse(request.body); } catch {}
let [status, headers, body] = await getReply(routes, request);
response.writeHead(status, headers).end(body);
});
});

context.close = () => context.server.close();
context.listen = port => new Promise((resolve, reject) => {
context.server.on('listening', () => resolve(context));
context.server.on('error', reject);
context.server.listen(port);
});

context.reply = (url, handler) => {
routes[url] = handler;
return context;
};

return context;
}

// Promised based helper for starting an app at the specified port. Resolves
// when the server is listening, rejects if there are any errors when starting.
export function startServer(app, port) {
return new Promise((resolve, reject) => {
let server = app.listen(port);
server.once('listening', () => resolve(server));
server.once('error', reject);
export default function createPercyServer(percy) {
return createServer({
// healthcheck returns meta info on success
'/percy/healthcheck': () => [200, 'application/json', {
success: true,
config: percy.config,
loglevel: percy.loglevel(),
build: percy.client.build
}],

// responds when idle
'/percy/idle': () => percy.idle()
.then(() => [200, 'application/json', { success: true }]),

// serves @percy/dom as a convenience
'/percy/dom.js': () => fs.promises
.readFile(require.resolve('@percy/dom'), 'utf-8')
.then(content => [200, 'applicaton/javascript', content]),

// forward snapshot requests
'/percy/snapshot': ({ body }) => percy.snapshot(body)
.then(() => [200, 'application/json', { success: true }]),

// stops the instance
'/percy/stop': () => percy.stop()
.then(() => [200, 'application/json', { success: true }]),

// other routes 404
default: () => [404, 'application/json', {
error: 'Not found',
success: false
}],

// generic error handler
catch: ({ message }) => [500, 'application/json', {
error: message,
success: false
}]
});
}
47 changes: 17 additions & 30 deletions packages/core/test/asset-discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,11 @@ describe('Asset Discovery', () => {
return [201, { data: { id: '4567' } }];
});

server = await createTestServer();

server.app
.get('/', (req, res) => {
res.set('Content-Type', 'text/html').send(testDOM);
})
.get('/style.css', (req, res) => {
res.set('Content-Type', 'text/css').send(testCSS);
})
.get('/img.gif', (req, res) => setTimeout(() => {
res.set('Content-Type', 'image/gif').send(pixel);
}, 10));
server = await createTestServer({
'/': () => [200, 'text/html', testDOM],
'/style.css': () => [200, 'text/css', testCSS],
'/img.gif': () => [200, 'image/gif', pixel]
});

percy = await Percy.start({
token: 'PERCY_TOKEN',
Expand All @@ -75,7 +68,7 @@ describe('Asset Discovery', () => {
});

await percy.idle();
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
// does not request the root url (serves domSnapshot instead)
expect(paths).not.toContain('/');
expect(paths).toContain('/style.css');
Expand Down Expand Up @@ -118,7 +111,7 @@ describe('Asset Discovery', () => {
});

await percy.idle();
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
expect(paths).toContain('/style.css');

expect(captured[0]).toEqual([
Expand Down Expand Up @@ -163,9 +156,7 @@ describe('Asset Discovery', () => {
});

it('follows redirects', async () => {
server.app.get('/stylesheet.css', (req, res) => {
res.redirect('/style.css');
});
server.reply('/stylesheet.css', () => [301, { Location: '/style.css' }]);

await percy.snapshot({
name: 'test snapshot',
Expand All @@ -174,7 +165,7 @@ describe('Asset Discovery', () => {
});

await percy.idle();
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
expect(paths).toContain('/stylesheet.css');
expect(paths).toContain('/style.css');

Expand All @@ -190,9 +181,7 @@ describe('Asset Discovery', () => {
});

it('skips capturing large files', async () => {
server.app.get('/large.css', (req, res) => {
res.set('Content-Type', 'text/stylesheet').send('A'.repeat(16000000));
});
server.reply('/large.css', () => [200, 'text/css', 'A'.repeat(16_000_000)]);

percy.loglevel('debug');
await stdio.capture(() => (
Expand Down Expand Up @@ -314,7 +303,7 @@ describe('Asset Discovery', () => {
await snapshot(2);

// only one request for each resource should be made
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
expect(paths.sort()).toEqual(['/img.gif', '/style.css']);

// both snapshots' captured resources should match
Expand All @@ -332,7 +321,7 @@ describe('Asset Discovery', () => {
await snapshot(2);

// two requests for each resource should be made (opposite of prev test)
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
expect(paths.sort()).toEqual(['/img.gif', '/img.gif', '/style.css', '/style.css']);

// bot snapshots' captured resources should match
Expand All @@ -343,7 +332,6 @@ describe('Asset Discovery', () => {
});
});

// these caches helpers are no longer used
describe('with unhandled errors', async () => {
it('logs unhandled request errors gracefully', async () => {
// sabotage this property to trigger unexpected error handling
Expand Down Expand Up @@ -397,10 +385,9 @@ describe('Asset Discovery', () => {
let server2;

beforeEach(async () => {
server2 = await createTestServer(8001);
server2.app.get('/img.gif', (req, res) => {
res.set('Content-Type', 'image/gif').send(pixel);
});
server2 = await createTestServer({
'/img.gif': () => [200, 'image/gif', pixel]
}, 8001);
});

afterEach(() => {
Expand All @@ -415,10 +402,10 @@ describe('Asset Discovery', () => {
});

await percy.idle();
let paths = server.requests.map(r => r.path);
let paths = server.requests.map(r => r[0]);
expect(paths).toContain('/style.css');
expect(paths).not.toContain('/img.gif');
let paths2 = server2.requests.map(r => r.path);
let paths2 = server2.requests.map(r => r[0]);
expect(paths2).not.toContain('/img.gif');

expect(captured[0]).toEqual([
Expand Down
Loading

0 comments on commit b28c085

Please sign in to comment.