New Audit: using deprecated APIs #1470

ebidel · 2017-01-14T00:58:08Z

R: all

Part of #1458.

paulirish · 2017-01-14T01:40:13Z

lighthouse-core/gather/gatherers/console-violations.js

+
+const Gatherer = require('./gatherer');
+
+class ConsoleViolations extends Gatherer {


Afaict, this will gather all logs added to console rather than just violations, right?

That's right. All logging generated by chrome. Not those using the console api. Are you thinking a rename of the class?

Basically yeah. Console logs don't go through Log.entryAdded? Thought they did!

Apparently they've been broken out into https://chromedevtools.github.io/debugger-protocol-viewer/tot/Runtime/#event-consoleAPICalled

patrickhulce · 2017-01-17T17:16:32Z

lighthouse-core/gather/gatherers/console-log-entry.js

+
+  afterPass(options) {
+    return this.endConsoleMessageCollect(options.driver)
+        .then(entries => {


nit: remove this function and just go to catch?

patrickhulce · 2017-01-17T17:21:03Z

lighthouse-core/config/default.json

+      "name": "Avoiding deprecated APIs and browser interventions",
+      "audits": {
+        "deprecations": {
+          "expectedValue": true,


Won't these not do anything since it's unscored?

Yep. Just adding as a creative of habit. Removed.

ebidel · 2017-01-17T18:48:41Z

PTAL

brendankenny

looking good

brendankenny · 2017-01-18T01:39:29Z

lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.css

@@ -17,4 +17,4 @@

 body {
  background-color: #eee;
-}
+}


new line was bad?

not sure why vscode stripped it. Done.

brendankenny · 2017-01-18T01:41:54Z

lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html

+function deprecationsTest() {
+  stampTemplate('deprecations-tmpl', document.head);
+
+  // Some APIs atm are not triggering Chrome's deprecation messags. https://crbug.com/680832.


messages, but maybe something more like // TODO: some deprecated API messages are not currently being logged correctly. See: https://crbug.com/680832

brendankenny · 2017-01-18T01:47:57Z

lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html

+
+  // FAIL
+  const xhr = new XMLHttpRequest();
+  xhr.open('GET', 'https://lighthouse-viewer.appspot.com/data', false);


can you do this to the local server? Ideally we wouldn't have any external connections for npm run smoke

brendankenny · 2017-01-18T01:53:30Z

lighthouse-core/gather/gatherers/console-log-entry.js

+
+const Gatherer = require('./gatherer');
+
+class ConsoleLogEntry extends Gatherer {


this sounds like "console.log entries" now :) What about just LogEntries to mirror debugger protocol's Log domain? (also made it plural to match e.g. tags-blocking-first-paint, styles, all-event-listeners, etc

Not a great name, is it? Hmm. We need something in between console messages and console logs entries only produced by chrome. LogEntries is better but it's still ambiguous. What about ChromeConsoleLogs?

I feel like having console + log still associates with console.log. ChromeConsoleMessages?

brendankenny · 2017-01-18T01:54:16Z

lighthouse-core/gather/gatherers/console-log-entry.js

+    this._logEntries.push(entry);
+  }
+
+  beginConsoleMessageCollect(driver) {


just inline beginConsoleMessageCollect and endConsoleMessageCollect? They're really short

brendankenny · 2017-01-18T01:56:07Z

lighthouse-core/audits/deprecations.js

+'use strict';
+
+/**
+ * @fileoverview Audits a page to determine if it is calling deprecated APIs.


maybe slightly more specific about auditing the log messages to surface deprecated API warnings or something?

brendankenny · 2017-01-18T02:01:14Z

lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html

+  //   MediaStreamTrack.getSources(); // FAIL
+  // } catch(e) {}
+
+  // console.timeline(); // FAIL


would it be crazy to uncomment these so we'd be forced to keep up to date as they're fixed and so the expectations file would no longer match? :)

I don't think we want to do that b/c smokehouse tests will fail indefinitely until Chrome fixes it. That could be an annoying while :\

oh, I meant uncomment these but keep smokehouse expectation at 4 and let chrome eventually fixing the broken deprecations break the test, then update the expectations. But no problem

Oh got it. Done.

brendankenny · 2017-01-18T18:54:04Z

lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html

+
+  // FAIL
+  const xhr = new XMLHttpRequest();
+  xhr.open('GET', 'dbw_tester.html', false);


sorry, forgot to include earlier: on the topic of cross origin requests, can you change the https://unknown.com (which appears to be a real site, just not https) to something like https://unknown.invalid, which is reserved for testing?

brendankenny · 2017-01-18T18:57:13Z

lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js

@@ -117,7 +117,7 @@ module.exports = [
        score: false,
        extendedInfo: {
          value: {
-            length: 4
+            length: 5


how did this one change?

The xhr got switched to include another http1.1 resource: xhr.open('GET', 'dbw_tester.html', false);. Moved from using an http2 endpoint (lighthouse-viewer.appspot.com).

ebidel · 2017-01-18T19:14:31Z

PTAL

ebidel · 2017-01-19T03:03:44Z

Travis is happy now.

brendankenny

about time, travis. LGTM
🐴 🚃 🎚 🏣 ➡️ 🚨

ebidel · 2017-01-19T07:11:23Z

Horse powerful IOT warning?

…

On Wed, Jan 18, 2017, 8:55 PM Brendan Kenny ***@***.***> wrote: Merged #1470 <#1470>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1470 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOigCQX-c8PkbvLRCJRLZT47hPlxo3Lks5rTuyigaJpZM4Ljdw-> .

ebidel force-pushed the deprecations branch from d55c111 to e8ae52d Compare January 14, 2017 01:03

ebidel added the new_audit label Jan 14, 2017

ebidel changed the title ~~Add "using deprecated APIs" section.~~ New Audit: using deprecated APIs Jan 14, 2017

paulirish reviewed Jan 14, 2017

View reviewed changes

patrickhulce reviewed Jan 17, 2017

View reviewed changes

brendankenny requested changes Jan 18, 2017

View reviewed changes

ebidel force-pushed the deprecations branch from ca6dabb to 71dd485 Compare January 18, 2017 06:18

ebidel added 10 commits January 18, 2017 09:36

First go at deprecations audit

bf2b5fe

cleanup of deprecations audit

b47ef4e

Tests

146d494

Remove typos

577941a

Finish renaming

9d380ea

feedback

e7b902a

feedback

1469e2b

make smokehouse pass

a4344f7

fix smoke test

f6c1304

try again

6bde7b3

ebidel force-pushed the deprecations branch from 70ed63a to 6bde7b3 Compare January 18, 2017 17:36

already did this...

d977ce2

ebidel mentioned this pull request Jan 18, 2017

Audit: CSS used selectors improvements #1456

Closed

brendankenny reviewed Jan 18, 2017

View reviewed changes

Rename

7139998

brendankenny approved these changes Jan 19, 2017

View reviewed changes

brendankenny merged commit 3c8f57c into master Jan 19, 2017

brendankenny deleted the deprecations branch January 19, 2017 04:55


		const Gatherer = require('./gatherer');

		class ConsoleViolations extends Gatherer {


		const Gatherer = require('./gatherer');

		class ConsoleLogEntry extends Gatherer {

New Audit: using deprecated APIs #1470

New Audit: using deprecated APIs #1470

Conversation

ebidel commented Jan 14, 2017

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

ebidel commented Jan 17, 2017

brendankenny 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

ebidel Jan 18, 2017 • edited Loading

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

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

ebidel Jan 18, 2017 • edited Loading

Choose a reason for hiding this comment

ebidel commented Jan 18, 2017

ebidel commented Jan 19, 2017

brendankenny left a comment

Choose a reason for hiding this comment

ebidel commented Jan 19, 2017 via email

ebidel Jan 18, 2017 •

edited

Loading

ebidel Jan 18, 2017 •

edited

Loading