Skip to content

Commit

Permalink
use express instead of serve-handler for snapshots (#766)
Browse files Browse the repository at this point in the history
This pr updates our tests to use express instead of serve-handler.
serve (likely due to using serve-handler) currently has a bug where,
if any redirects occur, it will lose any query params you had in the url.
See: vercel/serve#643

On top of that, serve will automatically redirect urls like
/index.html to just /, which breaks our iframe integration for universal.

J=SLAP-1313
TEST=auto

percy snapshots for universal on iframe are now corrected and
show the correct query
  • Loading branch information
oshi97 committed May 12, 2021
1 parent 1a2e170 commit 2eda3c4
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 22 deletions.
11 changes: 1 addition & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"babel-jest": "^25.5.1",
"comment-json": "^4.1.0",
"cross-env": "^7.0.2",
"express": "^4.17.1",
"file-system": "^2.2.2",
"full-icu": "^1.3.1",
"handlebars": "^4.7.6",
Expand Down
4 changes: 2 additions & 2 deletions tests/percy/iframepagenavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ class IframePageNavigator extends PageNavigator {

async gotoUniversalPage(queryParams = {}) {
const queryParamsString = getQueryParamsString(queryParams);
const url = `${this._siteUrl}/${this._iframePage}?${queryParamsString}`;
const url = `${this._siteUrl}/${this._iframePage}.html?${queryParamsString}`;
await this._page.goto(url);
await waitTillHTMLRendered(this._page);
}

async gotoVerticalPage(vertical, queryParams = {}) {
const queryParamsString = getQueryParamsString(queryParams);
const url = `${this._siteUrl}/${this._iframePage}?verticalUrl=${vertical}&${queryParamsString}`;
const url = `${this._siteUrl}/${this._iframePage}.html?verticalUrl=${vertical}.html&${queryParamsString}`;
await this._page.goto(url);
await waitTillHTMLRendered(this._page);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/percy/standardpagenavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class StandardPageNavigator extends PageNavigator {

async gotoVerticalPage(vertical, queryParams = {}) {
const queryParamsString = getQueryParamsString(queryParams);
const url = `${this._siteUrl}/${vertical}?${queryParamsString}`;
const url = `${this._siteUrl}/${vertical}.html?${queryParamsString}`;
await this._page.goto(url);
await waitTillHTMLRendered(this._page);
}
Expand Down
15 changes: 6 additions & 9 deletions tests/test-utils/server.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const http = require('http');
const handler = require('serve-handler');
const express = require('express');

/**
* A simple http server
Expand All @@ -13,20 +12,18 @@ class HttpServer {
* @param {number} config.port - The port to serve at
*/
constructor ({dir, port}) {
this._server = http.createServer((request, response) => {
return handler(request, response, {
"public": dir
});
});

this._app = express();
this._app.use(express.static(dir));
this._port = port;
}

/**
* Starts the server
*/
start () {
this._server.listen(this._port);
this._server = this._app.listen(this._port, () => {
console.log(`listening at http://localhost:${this._port}`);
});
}

/**
Expand Down

0 comments on commit 2eda3c4

Please sign in to comment.