tests(smokehouse): refactor to enable Smokerider #7284

connorjclark · 2019-02-20T18:40:24Z

A few refactors to enable running smoke tests against Lightrider. Namely:

Move all test definitions to their own module, with a way to uncompact the references to other modules (the expectations / config).
Move all the LHR comparison / reporting code to a module.
LR entry option keepRawValues, since many smoke tests assert on that.

See internal bug b/124882661.

brendankenny · 2019-02-20T18:46:38Z

is it possible to make the brainstorming doc public? It would be nice to see the motivations/constraints before settling on a new design

connorjclark · 2019-02-20T18:47:37Z

lighthouse-cli/test/smokehouse/smokehouse-report.js

@@ -0,0 +1,224 @@
+/* eslint-disable no-console */


Only changes here are 1) module exports at the bottom and 2) using Smokehouse. type definitions.

connorjclark · 2019-02-20T19:18:37Z

It's easier to share it individually. I sent @patrickhulce a link. I can do the same for anyone else who requests it.

patrickhulce

~~I'm probably missing the bigger picture here how this helps.~~ Doc you shared helped a lot :)

patrickhulce · 2019-02-21T04:02:24Z

lighthouse-cli/test/smokehouse/run-smoke.js

@@ -106,7 +35,7 @@ function displaySmokehouseOutput(result) {
 /**
 * Run smokehouse in child processes for selected smoketests
 * Display output from each as soon as they finish, but resolve function when ALL are complete
- * @param {Array<SmoketestDfn>} smokes
+ * @param {Array<typeof SMOKETESTS[0]>} smokes


can we not use SmoketestDfn directly somehow?

import('./smoke-test-dfn').SmoketestDefn or better yet a new Smokehouse namespace?

Moved to the new .d.ts.

patrickhulce · 2019-02-21T04:05:32Z

lighthouse-cli/test/smokehouse/smoke-test-dfns.js

+const path = require('path');
+
+/**
+ * @typedef {object} SmoketestDfn


☠️ 1️⃣ ✉️ 🕐

patrickhulce · 2019-02-21T04:07:03Z

lighthouse-cli/test/smokehouse/smoke-test-dfns.js

+  return require(expectationsPath);
+}
+
+module.exports.getCompactSmokeTests = () => {


we don't typically do piecemeal exports as we go, and I must admit the explicit collection of what's exported at the bottom has somewhat grown on me.

anything in particular that prompted this style or just more used to it?

also this function feels a little more like getUncompactedSmokeTests, no?

good point, changed to module.exports = {...}.

Doh, that is a better name. Although I do not like the word 'compact' for this. Got any better ideas?

resolved, expanded, loaded... like any of those? I think that's all I got :)

patrickhulce

LGTM % last typedef

patrickhulce · 2019-02-25T19:12:41Z

lighthouse-core/lib/proto-preprocessor.js

@@ -53,8 +56,8 @@ function processForProto(result) {
          audit.scoreDisplayMode = 'notApplicable';
        }
      }
-      // Drop raw values. #6199
-      if ('rawValue' in audit) {
+      // Drop raw values. https://github.com/GoogleChrome/lighthouse/issues/6199


patrickhulce · 2019-02-25T19:21:33Z

lighthouse-cli/test/smokehouse/run-smoke.js

@@ -139,7 +68,7 @@ async function runSmokehouse(smokes) {
 /**
 * Determine batches of smoketests to run, based on argv
 * @param {string[]} argv
- * @return {Map<string|undefined, Array<SmoketestDfn>>}
+ * @return {Map<string|undefined, Array<typeof SMOKETESTS[0]>>}


upate this one?

connorjclark · 2019-02-26T22:43:40Z

@brendankenny any questions or concerns about these changes after reviewing how it's utilized in cl/234828413?

paulirish

works for me. only small nits

paulirish · 2019-03-05T21:09:56Z

lighthouse-cli/test/smokehouse/smoke-test-dfns.js

+const smokehouseDir = 'lighthouse-cli/test/smokehouse/';
+
+/** @type {Array<Smokehouse.TestDfn>} */
+const SMOKE_TESTS = [{


rename to SMOME_TEST_DFNS ?

paulirish · 2019-03-05T21:10:20Z

lighthouse-cli/test/smokehouse/smoke-test-dfns.js

+  return require(expectationsPath);
+}
+
+function getUncompactedSmokeTests() {


This name is funny to me. It means the config/expectations are not filenames but the files are read as modules?

if we rename the above to DFNS, then this module can be getSmokeTests()

exterkamp

LGTM!

exterkamp · 2019-03-06T17:35:05Z

lighthouse-cli/test/smokehouse/smoke-test-dfns.js

@@ -0,0 +1,128 @@
+'use strict';


/** * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */

exterkamp · 2019-03-06T17:35:43Z

lighthouse-cli/test/smokehouse/smokehouse-report.js

@@ -0,0 +1,229 @@
+/* eslint-disable no-console */


/** * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */

exterkamp · 2019-03-06T17:36:30Z

types/smokehouse.d.ts

@@ -0,0 +1,30 @@
+declare module Smokehouse {


/** * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */

tests(smokehouse): Refactor so that Smokerider can run smoke tests.

47554b1

connorjclark requested review from brendankenny, patrickhulce and paulirish as code owners February 20, 2019 18:40

Merge remote-tracking branch 'origin/master' into lr-smokerider

5fb69b9

connorjclark commented Feb 20, 2019

View reviewed changes

fix typo

3d2b752

default value for processForProto opts

b8f4c08

connorjclark mentioned this pull request Feb 21, 2019

tests(smokehouse): gzip test to assert transfer and resource sizes #7286

Merged

patrickhulce reviewed Feb 21, 2019

View reviewed changes

connorjclark added 2 commits February 20, 2019 21:41

move test dfn type to .d.ts

2b241cd

refactor exports

19032ca

patrickhulce approved these changes Feb 25, 2019

View reviewed changes

fix typecheck

529f45c

paulirish approved these changes Mar 5, 2019

View reviewed changes

connorjclark added 2 commits March 5, 2019 13:49

rename

38da2ec

Merge remote-tracking branch 'origin/master' into lr-smokerider

6671d53

exterkamp approved these changes Mar 6, 2019

View reviewed changes

licensey

c242d24

paulirish changed the title ~~tests(smokehouse): Refactor so that Smokerider can run smoke tests.~~ tests(smokehouse): refactor to enable Smokerider Mar 6, 2019

paulirish merged commit fd86d94 into master Mar 6, 2019

paulirish deleted the lr-smokerider branch March 6, 2019 18:29

connorjclark added a commit that referenced this pull request Mar 12, 2019

tests(smokehouse): refactor to enable Smokerider (#7284)

3a404c1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests(smokehouse): refactor to enable Smokerider #7284

tests(smokehouse): refactor to enable Smokerider #7284

connorjclark commented Feb 20, 2019

brendankenny commented Feb 20, 2019

connorjclark Feb 20, 2019

connorjclark commented Feb 20, 2019

patrickhulce left a comment

patrickhulce Feb 21, 2019

connorjclark Feb 21, 2019

patrickhulce Feb 21, 2019

connorjclark Feb 21, 2019

patrickhulce Feb 21, 2019

patrickhulce Feb 21, 2019

connorjclark Feb 21, 2019

patrickhulce Feb 21, 2019

patrickhulce left a comment

patrickhulce Feb 25, 2019

patrickhulce Feb 25, 2019

connorjclark commented Feb 26, 2019

paulirish left a comment

paulirish Mar 5, 2019

paulirish Mar 5, 2019

exterkamp left a comment

exterkamp Mar 6, 2019

exterkamp Mar 6, 2019

exterkamp Mar 6, 2019

tests(smokehouse): refactor to enable Smokerider #7284

tests(smokehouse): refactor to enable Smokerider #7284

Conversation

connorjclark commented Feb 20, 2019

brendankenny commented Feb 20, 2019

Choose a reason for hiding this comment

connorjclark commented Feb 20, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark commented Feb 26, 2019

paulirish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

exterkamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment