Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests(smokehouse): adopt URLSearchParams for querystring manipulation #3941

Merged
merged 5 commits into from
Dec 8, 2017
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ const getFlags = require('../../cli-flags').getFlags;

describe('CLI run', function() {
it('runLighthouse completes a LH round trip', () => {
const url = 'chrome://version';
const url = 'chrome://version/';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hang on, I just realized the comment around this. Are we doing something wrong here?

just tried this out in console, seems like a bad result if everything gets a trailing slash now

const URL = require('whatwg-url').URL
console.log(new URL('chrome://version').href) // 'chrome://version'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay looks like special urls had a trailing slash for a little bit in the spec but it was changed.

web-platform-tests/wpt#4586
whatwg/url#213
jsdom/whatwg-url@9ec5618

However Chrome doesn't yet implement this change. As a result, it adds the trailing slash when it delivers the canonicalized URL over the devtools protocol. In the future this'll probably change, but for now we'll have to normalize between the spec-violating Chrome and the correct whatwg-url.

Doesn't look like there's a crbug for this, though there's a sprinkling of other issues documenting Chrome's URL failures on webplatformtests.

exciting. 🤒

const filename = path.join(process.cwd(), 'run.ts.results.json');
const flags = getFlags(`--output=json --output-path=${filename} ${url}`);
return run.runLighthouse(url, flags, fastConfig).then(passedResults => {
28 changes: 13 additions & 15 deletions lighthouse-cli/test/fixtures/static-server.js
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ const path = require('path');
const fs = require('fs');
const parseQueryString = require('querystring').parse;
const parseURL = require('url').parse;
const URLSearchParams = require('../../../lighthouse-core/lib/url-shim').URLSearchParams;
const HEADER_SAFELIST = new Set(['x-robots-tag', 'link']);

const lhRootDirPath = path.join(__dirname, '../../../');
@@ -64,32 +65,29 @@ function requestHandler(request, response) {

let delay = 0;
if (queryString) {
const params = new URLSearchParams(queryString);
// set document status-code
if (typeof queryString.status_code !== 'undefined') {
statusCode = parseInt(queryString.status_code, 10);
if (params.has('status_code')) {
statusCode = parseInt(params.get('status_code'), 10);
}

// set delay of request when present
if (typeof queryString.delay !== 'undefined') {
delay = parseInt(queryString.delay, 10) || 2000;
if (params.has('delay')) {
delay = parseInt(params.get('delay'), 10) || 2000;
}

if (typeof queryString.extra_header !== 'undefined') {
let extraHeaders = queryString.extra_header;
extraHeaders = Array.isArray(extraHeaders) ? extraHeaders : [extraHeaders];

extraHeaders.forEach(header => {
const [headerName, ...headerValue] = header.split(':');

if (params.has('extra_header')) {
const extraHeaders = new URLSearchParams(params.get('extra_header'));
for (const [headerName, headerValue] of extraHeaders) {
if (HEADER_SAFELIST.has(headerName.toLowerCase())) {
headers[headerName] = headerValue.join(':');
headers[headerName] = headerValue;
}
});
}
}

// redirect url to new url if present
if (typeof queryString.redirect !== 'undefined') {
return setTimeout(sendRedirect, delay, queryString.redirect);
if (params.has('redirect')) {
return setTimeout(sendRedirect, delay, params.get('redirect'));
}
}

20 changes: 10 additions & 10 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Original file line number Diff line number Diff line change
@@ -5,20 +5,20 @@
*/
'use strict';
const BASE_URL = 'http://localhost:10200/seo/';
const URLSearchParams = require('../../../../lighthouse-core/lib/url-shim').URLSearchParams;

function headersParam(headers) {
return headers
.map(({name, value}) => `extra_header=${name}:${encodeURI(value)}`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice catch we should've been using encodeURIComponent here 👍

.join('&');
const headerString = new URLSearchParams(headers).toString();
return new URLSearchParams([['extra_header', headerString]]).toString();
}

const failureHeaders = headersParam([{
name: 'x-robots-tag',
value: 'none',
}, {
name: 'link',
value: '<http://example.com>;rel="alternate";hreflang="xx"',
}]);
const failureHeaders = headersParam([[
'x-robots-tag',
'none',
], [
'link',
'<http://example.com>;rel="alternate";hreflang="xx"',
]]);

/**
* Expected Lighthouse audit values for seo tests
3 changes: 3 additions & 0 deletions lighthouse-core/lib/url-shim.js
Original file line number Diff line number Diff line change
@@ -18,6 +18,9 @@ const Util = require('../report/v2/renderer/util.js');
// https://github.com/GoogleChrome/lighthouse/issues/1186
const URL = (typeof self !== 'undefined' && self.URL) || require('whatwg-url').URL;

URL.URLSearchParams = (typeof self !== 'undefined' && self.URLSearchParams) ||
require('whatwg-url').URLSearchParams;

URL.INVALID_URL_DEBUG_STRING =
'Lighthouse was unable to determine the URL of some script executions. ' +
'It\'s possible a Chrome extension or other eval\'d code is the source.';
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -96,7 +96,7 @@
"semver": "^5.3.0",
"speedline": "1.3.0",
"update-notifier": "^2.1.0",
"whatwg-url": "4.0.0",
"whatwg-url": "^6.3.0",
"ws": "3.3.2",
"yargs": "3.32.0",
"yargs-parser": "7.0.0"
33 changes: 26 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
@@ -2691,6 +2691,10 @@ lodash.restparam@^3.0.0:
version "3.6.1"
resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805"

lodash.sortby@^4.7.0:
version "4.7.0"
resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438"

lodash.template@^3.0.0:
version "3.6.2"
resolved "https://registry.yarnpkg.com/lodash.template/-/lodash.template-3.6.2.tgz#f8cdecc6169a255be9098ae8b0c53d378931d14f"
@@ -3266,6 +3270,10 @@ punycode@^1.4.1:
version "1.4.1"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.4.1.tgz#c0d5a63b2718800ad8e1eb0fa5269c84dd41845e"

punycode@^2.1.0:
version "2.1.0"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.0.tgz#5f863edc89b96db09074bad7947bf09056ca4e7d"

q@^1.4.1:
version "1.5.1"
resolved "https://registry.yarnpkg.com/q/-/q-1.5.1.tgz#7e32f75b41381291d04611f1bf14109ac00651d7"
@@ -3979,6 +3987,12 @@ tough-cookie@^2.3.2, tough-cookie@~2.3.0:
dependencies:
punycode "^1.4.1"

tr46@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/tr46/-/tr46-1.0.1.tgz#a8b13fd6bfd2489519674ccde55ba3693b706d09"
dependencies:
punycode "^2.1.0"

tr46@~0.0.3:
version "0.0.3"
resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a"
@@ -4168,26 +4182,31 @@ webidl-conversions@^4.0.0:
version "4.0.1"
resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-4.0.1.tgz#8015a17ab83e7e1b311638486ace81da6ce206a0"

webidl-conversions@^4.0.1:
version "4.0.2"
resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-4.0.2.tgz#a855980b1f0b6b359ba1d5d9fb39ae941faa63ad"

whatwg-encoding@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/whatwg-encoding/-/whatwg-encoding-1.0.1.tgz#3c6c451a198ee7aec55b1ec61d0920c67801a5f4"
dependencies:
iconv-lite "0.4.13"

whatwg-url@4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-4.0.0.tgz#5be362f0b6e2f8760f7260df6e0e1df536f5479c"
dependencies:
tr46 "~0.0.3"
webidl-conversions "^3.0.0"

whatwg-url@^4.3.0:
version "4.7.0"
resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-4.7.0.tgz#202035ac1955b087cdd20fa8b58ded3ab1cd2af5"
dependencies:
tr46 "~0.0.3"
webidl-conversions "^3.0.0"

whatwg-url@^6.3.0:
version "6.3.0"
resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-6.3.0.tgz#597ee5488371abe7922c843397ddec1ae94c048d"
dependencies:
lodash.sortby "^4.7.0"
tr46 "^1.0.0"
webidl-conversions "^4.0.1"

which@^1.1.1, which@^1.2.10, which@^1.2.8, which@^1.2.9:
version "1.2.11"
resolved "https://registry.yarnpkg.com/which/-/which-1.2.11.tgz#c8b2eeea6b8c1659fa7c1dd4fdaabe9533dc5e8b"