chore: introduce //lib/api.js (#3835)

Introduce `//lib/api.js` that declares a list of publicly exposed classes. The `//lib/api.js` list superceedes dynamic `helper.tracePublicAPI()` calls and is used in the following places: - [ASYNC STACKS]: generate "async stacks" for publicy exposed API in `//index.js` - [COVERAGE]: move coverage support from `//lib/helper` to `//test/utils` - [DOCLINT]: get rid of 'exluded classes' hardcoded list This will help us to re-use our coverage and doclint infrastructure for Puppeteer-Firefox. Drive-By: it turns out we didn't run coverage for `SecurityDetails` class, so we lack coverage for a few methods there. These are excluded for now, sanity tests will be added in a follow-up.
puppeteer · Jan 26, 2019 · 62da236 · 62da236
1 parent cd678fb
commit 62da236
Show file tree

Hide file tree

Showing 21 changed files with 139 additions and 128 deletions.
diff --git a/index.js b/index.js
@@ -21,6 +21,16 @@ try {
   asyncawait = false;
 }
 
+if (asyncawait) {
+  const {helper} = require('./lib/helper');
+  const api = require('./lib/api');
+  for (const className in api) {
+    // Puppeteer-web excludes certain classes from bundle, e.g. BrowserFetcher.
+    if (typeof api[className] === 'function')
+      helper.installAsyncStackHooks(api[className]);
+  }
+}
+
 // If node does not support async await, use the compiled version.
 const Puppeteer = asyncawait ? require('./lib/Puppeteer') : require('./node6/lib/Puppeteer');
 const packageJson = require('./package.json');

diff --git a/lib/Accessibility.js b/lib/Accessibility.js
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const {helper} = require('./helper');
 
 /**
  * @typedef {Object} SerializedAXNode
@@ -390,4 +389,3 @@ class AXNode {
 }
 
 module.exports = {Accessibility};
-helper.tracePublicAPI(Accessibility);
diff --git a/lib/Browser.js b/lib/Browser.js
@@ -373,7 +373,4 @@ class BrowserContext extends EventEmitter {
   }
 }
 
-helper.tracePublicAPI(BrowserContext);
-helper.tracePublicAPI(Browser);
-
 module.exports = {Browser, BrowserContext};
diff --git a/lib/Connection.js b/lib/Connection.js
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const {helper, assert} = require('./helper');
+const {assert} = require('./helper');
 const {Events} = require('./Events');
 const debugProtocol = require('debug')('puppeteer:protocol');
 const EventEmitter = require('events');
@@ -216,8 +216,6 @@ class CDPSession extends EventEmitter {
   }
 }
 
-helper.tracePublicAPI(CDPSession);
-
 /**
  * @param {!Error} error
  * @param {string} method

diff --git a/lib/Coverage.js b/lib/Coverage.js
@@ -64,7 +64,6 @@ class Coverage {
 }
 
 module.exports = {Coverage};
-helper.tracePublicAPI(Coverage);
 
 class JSCoverage {
   /**

diff --git a/lib/Dialog.js b/lib/Dialog.js
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-const {helper, assert} = require('./helper');
+const {assert} = require('./helper');
 
 class Dialog {
   /**
@@ -81,4 +81,3 @@ Dialog.Type = {
 };
 
 module.exports = {Dialog};
-helper.tracePublicAPI(Dialog);
diff --git a/lib/ExecutionContext.js b/lib/ExecutionContext.js
@@ -175,6 +175,4 @@ class ExecutionContext {
   }
 }
 
-helper.tracePublicAPI(ExecutionContext);
-
 module.exports = {ExecutionContext, EVALUATION_SCRIPT_URL};
diff --git a/lib/FrameManager.js b/lib/FrameManager.js
@@ -680,7 +680,6 @@ class Frame {
     this._parentFrame = null;
   }
 }
-helper.tracePublicAPI(Frame);
 
 function assertNoLegacyNavigationOptions(options) {
   assert(options['networkIdleTimeout'] === undefined, 'ERROR: networkIdleTimeout option is no longer supported.');

diff --git a/lib/Input.js b/lib/Input.js
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-const {helper, assert} = require('./helper');
+const {assert} = require('./helper');
 const keyDefinitions = require('./USKeyboardLayout');
 
 /**
@@ -302,6 +302,3 @@ class Touchscreen {
 }
 
 module.exports = { Keyboard, Mouse, Touchscreen};
-helper.tracePublicAPI(Keyboard);
-helper.tracePublicAPI(Mouse);
-helper.tracePublicAPI(Touchscreen);
diff --git a/lib/JSHandle.js b/lib/JSHandle.js
@@ -122,8 +122,6 @@ class JSHandle {
   }
 }
 
-helper.tracePublicAPI(JSHandle);
-
 class ElementHandle extends JSHandle {
   /**
    * @param {!Puppeteer.ExecutionContext} context
@@ -507,5 +505,4 @@ function computeQuadArea(quad) {
  * @property {number} height
  */
 
-helper.tracePublicAPI(ElementHandle);
 module.exports = {createJSHandle, JSHandle, ElementHandle};
diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js
@@ -507,8 +507,6 @@ const errorReasons = {
   'failed': 'Failed',
 };
 
-helper.tracePublicAPI(Request);
-
 class Response {
   /**
    * @param {!Puppeteer.CDPSession} client
@@ -649,7 +647,6 @@ class Response {
     return this._request.frame();
   }
 }
-helper.tracePublicAPI(Response);
 
 const IGNORED_HEADERS = new Set(['accept', 'referer', 'x-devtools-emulate-network-conditions-client-id', 'cookie', 'origin', 'content-type', 'intervention']);
 
@@ -798,4 +795,4 @@ const statusTexts = {
   '511': 'Network Authentication Required',
 };
 
-module.exports = {Request, Response, NetworkManager};
+module.exports = {Request, Response, NetworkManager, SecurityDetails};
diff --git a/lib/Page.js b/lib/Page.js
@@ -1270,5 +1270,4 @@ class ConsoleMessage {
 }
 
 
-module.exports = {Page};
-helper.tracePublicAPI(Page);
+module.exports = {Page, ConsoleMessage};
diff --git a/lib/Puppeteer.js b/lib/Puppeteer.js
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const {helper} = require('./helper');
 const Launcher = require('./Launcher');
 const BrowserFetcher = require('./BrowserFetcher');
 
@@ -68,4 +67,3 @@ module.exports = class {
   }
 };
 
-helper.tracePublicAPI(module.exports, 'Puppeteer');
diff --git a/lib/Target.js b/lib/Target.js
@@ -1,6 +1,21 @@
+/**
+ * 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.
+ */
+
 const {Events} = require('./Events');
 const {Page} = require('./Page');
-const {helper} = require('./helper');
 
 class Target {
   /**
@@ -113,6 +128,4 @@ class Target {
   }
 }
 
-helper.tracePublicAPI(Target);
-
 module.exports = {Target};
diff --git a/lib/Tracing.js b/lib/Tracing.js
@@ -101,6 +101,5 @@ class Tracing {
     }
   }
 }
-helper.tracePublicAPI(Tracing);
 
 module.exports = Tracing;
diff --git a/lib/Worker.js b/lib/Worker.js
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 const EventEmitter = require('events');
-const {helper, debugError} = require('./helper');
+const {debugError} = require('./helper');
 const {ExecutionContext} = require('./ExecutionContext');
 const {JSHandle} = require('./JSHandle');
 
@@ -78,4 +78,3 @@ class Worker extends EventEmitter {
 }
 
 module.exports = {Worker};
-helper.tracePublicAPI(Worker);
diff --git a/lib/api.js b/lib/api.js
@@ -0,0 +1,42 @@
+/**
+ * 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.
+ */
+
+module.exports = {
+  Accessibility: require('./Accessibility').Accessibility,
+  Browser: require('./Browser').Browser,
+  BrowserContext: require('./Browser').BrowserContext,
+  BrowserFetcher: require('./BrowserFetcher'),
+  CDPSession: require('./Connection').CDPSession,
+  ConsoleMessage: require('./Page').ConsoleMessage,
+  Coverage: require('./Coverage').Coverage,
+  Dialog: require('./Dialog').Dialog,
+  ElementHandle: require('./JSHandle').ElementHandle,
+  ExecutionContext: require('./ExecutionContext').ExecutionContext,
+  Frame: require('./FrameManager').Frame,
+  JSHandle: require('./JSHandle').JSHandle,
+  Keyboard: require('./Input').Keyboard,
+  Mouse: require('./Input').Mouse,
+  Page: require('./Page').Page,
+  Puppeteer: require('./Puppeteer'),
+  Request: require('./NetworkManager').Request,
+  Response: require('./NetworkManager').Response,
+  SecurityDetails: require('./NetworkManager').SecurityDetails,
+  Target: require('./Target').Target,
+  TimeoutError: require('./Errors').TimeoutError,
+  Touchscreen: require('./Input').Touchscreen,
+  Tracing: require('./Tracing'),
+  Worker: require('./Worker').Worker,
+};
diff --git a/lib/helper.js b/lib/helper.js
@@ -16,41 +16,6 @@
 const {TimeoutError} = require('./Errors');
 
 const debugError = require('debug')(`puppeteer:error`);
-/** @type {?Map<string, boolean>} */
-let apiCoverage = null;
-
-/**
- * @param {!Object} classType
- * @param {string=} publicName
- */
-function traceAPICoverage(classType, publicName) {
-  if (!apiCoverage)
-    return;
-
-  let className = publicName || classType.prototype.constructor.name;
-  className = className.substring(0, 1).toLowerCase() + className.substring(1);
-  for (const methodName of Reflect.ownKeys(classType.prototype)) {
-    const method = Reflect.get(classType.prototype, methodName);
-    if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function')
-      continue;
-    apiCoverage.set(`${className}.${methodName}`, false);
-    Reflect.set(classType.prototype, methodName, function(...args) {
-      apiCoverage.set(`${className}.${methodName}`, true);
-      return method.call(this, ...args);
-    });
-  }
-
-  if (classType.Events) {
-    for (const event of Object.values(classType.Events))
-      apiCoverage.set(`${className}.emit(${JSON.stringify(event)})`, false);
-    const method = Reflect.get(classType.prototype, 'emit');
-    Reflect.set(classType.prototype, 'emit', function(event, ...args) {
-      if (this.listenerCount(event))
-        apiCoverage.set(`${className}.emit(${JSON.stringify(event)})`, true);
-      return method.call(this, event, ...args);
-    });
-  }
-}
 
 class Helper {
   /**
@@ -133,9 +98,8 @@ class Helper {
 
   /**
    * @param {!Object} classType
-   * @param {string=} publicName
    */
-  static tracePublicAPI(classType, publicName) {
+  static installAsyncStackHooks(classType) {
     for (const methodName of Reflect.ownKeys(classType.prototype)) {
       const method = Reflect.get(classType.prototype, methodName);
       if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function' || method.constructor.name !== 'AsyncFunction')
@@ -151,8 +115,6 @@ class Helper {
         });
       });
     }
-
-    traceAPICoverage(classType, publicName);
   }
 
   /**
@@ -175,17 +137,6 @@ class Helper {
     listeners.splice(0, listeners.length);
   }
 
-  /**
-   * @return {?Map<string, boolean>}
-   */
-  static publicAPICoverage() {
-    return apiCoverage;
-  }
-
-  static recordPublicAPICoverage() {
-    apiCoverage = new Map();
-  }
-
   /**
    * @param {!Object} obj
    * @return {boolean}

diff --git a/test/test.js b/test/test.js
@@ -75,32 +75,24 @@ beforeEach(async({server, httpsServer}) => {
   httpsServer.reset();
 });
 
-describe('Chromium', () => {
-  const {helper} = require('../lib/helper');
-  if (process.env.COVERAGE)
-    helper.recordPublicAPICoverage();
+const CHROMIUM_NO_COVERAGE = new Set([
+  'page.bringToFront',
+  'securityDetails.subjectName',
+  'securityDetails.issuer',
+  'securityDetails.validFrom',
+  'securityDetails.validTo',
+  ...(headless ? [] : ['page.pdf']),
+]);
 
+describe('Chromium', () => {
   require('./puppeteer.spec.js').addTests({
     product: 'Chromium',
     puppeteer: utils.requireRoot('index'),
     defaultBrowserOptions,
     testRunner,
   });
-  if (process.env.COVERAGE) {
-    describe('COVERAGE', function() {
-      const coverage = helper.publicAPICoverage();
-      const disabled = new Set(['page.bringToFront']);
-      if (!headless)
-        disabled.add('page.pdf');
-
-      for (const method of coverage.keys()) {
-        (disabled.has(method) ? xit : it)(`public api '${method}' should be called`, async({page, server}) => {
-          if (!coverage.get(method))
-            throw new Error('NOT CALLED!');
-        });
-      }
-    });
-  }
+  if (process.env.COVERAGE)
+    utils.recordAPICoverage(testRunner, require('../lib/api'), CHROMIUM_NO_COVERAGE);
 });
 
 if (process.env.CI && testRunner.hasFocusedTestsOrSuites()) {