Skip to content

Commit 62da236

Browse files
aslushnikovaslushnikov
authored
chore: introduce //lib/api.js (puppeteer#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.
1 parent cd678fb commit 62da236

21 files changed

+139
-128
lines changed

Diff for: ‎index.js

+10
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ try {
2121
asyncawait = false;
2222
}
2323

24+
if (asyncawait) {
25+
const {helper} = require('./lib/helper');
26+
const api = require('./lib/api');
27+
for (const className in api) {
28+
// Puppeteer-web excludes certain classes from bundle, e.g. BrowserFetcher.
29+
if (typeof api[className] === 'function')
30+
helper.installAsyncStackHooks(api[className]);
31+
}
32+
}
33+
2434
// If node does not support async await, use the compiled version.
2535
const Puppeteer = asyncawait ? require('./lib/Puppeteer') : require('./node6/lib/Puppeteer');
2636
const packageJson = require('./package.json');

Diff for: ‎lib/Accessibility.js

-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
const {helper} = require('./helper');
1716

1817
/**
1918
* @typedef {Object} SerializedAXNode
@@ -390,4 +389,3 @@ class AXNode {
390389
}
391390

392391
module.exports = {Accessibility};
393-
helper.tracePublicAPI(Accessibility);

Diff for: ‎lib/Browser.js

-3
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,4 @@ class BrowserContext extends EventEmitter {
373373
}
374374
}
375375

376-
helper.tracePublicAPI(BrowserContext);
377-
helper.tracePublicAPI(Browser);
378-
379376
module.exports = {Browser, BrowserContext};

Diff for: ‎lib/Connection.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
const {helper, assert} = require('./helper');
16+
const {assert} = require('./helper');
1717
const {Events} = require('./Events');
1818
const debugProtocol = require('debug')('puppeteer:protocol');
1919
const EventEmitter = require('events');
@@ -216,8 +216,6 @@ class CDPSession extends EventEmitter {
216216
}
217217
}
218218

219-
helper.tracePublicAPI(CDPSession);
220-
221219
/**
222220
* @param {!Error} error
223221
* @param {string} method

Diff for: ‎lib/Coverage.js

-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ class Coverage {
6464
}
6565

6666
module.exports = {Coverage};
67-
helper.tracePublicAPI(Coverage);
6867

6968
class JSCoverage {
7069
/**

Diff for: ‎lib/Dialog.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
const {helper, assert} = require('./helper');
17+
const {assert} = require('./helper');
1818

1919
class Dialog {
2020
/**
@@ -81,4 +81,3 @@ Dialog.Type = {
8181
};
8282

8383
module.exports = {Dialog};
84-
helper.tracePublicAPI(Dialog);

Diff for: ‎lib/ExecutionContext.js

-2
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,4 @@ class ExecutionContext {
175175
}
176176
}
177177

178-
helper.tracePublicAPI(ExecutionContext);
179-
180178
module.exports = {ExecutionContext, EVALUATION_SCRIPT_URL};

Diff for: ‎lib/FrameManager.js

-1
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,6 @@ class Frame {
680680
this._parentFrame = null;
681681
}
682682
}
683-
helper.tracePublicAPI(Frame);
684683

685684
function assertNoLegacyNavigationOptions(options) {
686685
assert(options['networkIdleTimeout'] === undefined, 'ERROR: networkIdleTimeout option is no longer supported.');

Diff for: ‎lib/Input.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
const {helper, assert} = require('./helper');
17+
const {assert} = require('./helper');
1818
const keyDefinitions = require('./USKeyboardLayout');
1919

2020
/**
@@ -302,6 +302,3 @@ class Touchscreen {
302302
}
303303

304304
module.exports = { Keyboard, Mouse, Touchscreen};
305-
helper.tracePublicAPI(Keyboard);
306-
helper.tracePublicAPI(Mouse);
307-
helper.tracePublicAPI(Touchscreen);

Diff for: ‎lib/JSHandle.js

-3
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,6 @@ class JSHandle {
122122
}
123123
}
124124

125-
helper.tracePublicAPI(JSHandle);
126-
127125
class ElementHandle extends JSHandle {
128126
/**
129127
* @param {!Puppeteer.ExecutionContext} context
@@ -507,5 +505,4 @@ function computeQuadArea(quad) {
507505
* @property {number} height
508506
*/
509507

510-
helper.tracePublicAPI(ElementHandle);
511508
module.exports = {createJSHandle, JSHandle, ElementHandle};

Diff for: ‎lib/NetworkManager.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,6 @@ const errorReasons = {
507507
'failed': 'Failed',
508508
};
509509

510-
helper.tracePublicAPI(Request);
511-
512510
class Response {
513511
/**
514512
* @param {!Puppeteer.CDPSession} client
@@ -649,7 +647,6 @@ class Response {
649647
return this._request.frame();
650648
}
651649
}
652-
helper.tracePublicAPI(Response);
653650

654651
const IGNORED_HEADERS = new Set(['accept', 'referer', 'x-devtools-emulate-network-conditions-client-id', 'cookie', 'origin', 'content-type', 'intervention']);
655652

@@ -798,4 +795,4 @@ const statusTexts = {
798795
'511': 'Network Authentication Required',
799796
};
800797

801-
module.exports = {Request, Response, NetworkManager};
798+
module.exports = {Request, Response, NetworkManager, SecurityDetails};

Diff for: ‎lib/Page.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -1270,5 +1270,4 @@ class ConsoleMessage {
12701270
}
12711271

12721272

1273-
module.exports = {Page};
1274-
helper.tracePublicAPI(Page);
1273+
module.exports = {Page, ConsoleMessage};

Diff for: ‎lib/Puppeteer.js

-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
const {helper} = require('./helper');
1716
const Launcher = require('./Launcher');
1817
const BrowserFetcher = require('./BrowserFetcher');
1918

@@ -68,4 +67,3 @@ module.exports = class {
6867
}
6968
};
7069

71-
helper.tracePublicAPI(module.exports, 'Puppeteer');

Diff for: ‎lib/Target.js

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,21 @@
1+
/**
2+
* Copyright 2019 Google Inc. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
117
const {Events} = require('./Events');
218
const {Page} = require('./Page');
3-
const {helper} = require('./helper');
419

520
class Target {
621
/**
@@ -113,6 +128,4 @@ class Target {
113128
}
114129
}
115130

116-
helper.tracePublicAPI(Target);
117-
118131
module.exports = {Target};

Diff for: ‎lib/Tracing.js

-1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,5 @@ class Tracing {
101101
}
102102
}
103103
}
104-
helper.tracePublicAPI(Tracing);
105104

106105
module.exports = Tracing;

Diff for: ‎lib/Worker.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616
const EventEmitter = require('events');
17-
const {helper, debugError} = require('./helper');
17+
const {debugError} = require('./helper');
1818
const {ExecutionContext} = require('./ExecutionContext');
1919
const {JSHandle} = require('./JSHandle');
2020

@@ -78,4 +78,3 @@ class Worker extends EventEmitter {
7878
}
7979

8080
module.exports = {Worker};
81-
helper.tracePublicAPI(Worker);

Diff for: ‎lib/api.js

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Copyright 2019 Google Inc. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
module.exports = {
18+
Accessibility: require('./Accessibility').Accessibility,
19+
Browser: require('./Browser').Browser,
20+
BrowserContext: require('./Browser').BrowserContext,
21+
BrowserFetcher: require('./BrowserFetcher'),
22+
CDPSession: require('./Connection').CDPSession,
23+
ConsoleMessage: require('./Page').ConsoleMessage,
24+
Coverage: require('./Coverage').Coverage,
25+
Dialog: require('./Dialog').Dialog,
26+
ElementHandle: require('./JSHandle').ElementHandle,
27+
ExecutionContext: require('./ExecutionContext').ExecutionContext,
28+
Frame: require('./FrameManager').Frame,
29+
JSHandle: require('./JSHandle').JSHandle,
30+
Keyboard: require('./Input').Keyboard,
31+
Mouse: require('./Input').Mouse,
32+
Page: require('./Page').Page,
33+
Puppeteer: require('./Puppeteer'),
34+
Request: require('./NetworkManager').Request,
35+
Response: require('./NetworkManager').Response,
36+
SecurityDetails: require('./NetworkManager').SecurityDetails,
37+
Target: require('./Target').Target,
38+
TimeoutError: require('./Errors').TimeoutError,
39+
Touchscreen: require('./Input').Touchscreen,
40+
Tracing: require('./Tracing'),
41+
Worker: require('./Worker').Worker,
42+
};

Diff for: ‎lib/helper.js

+1-50
Original file line numberDiff line numberDiff line change
@@ -16,41 +16,6 @@
1616
const {TimeoutError} = require('./Errors');
1717

1818
const debugError = require('debug')(`puppeteer:error`);
19-
/** @type {?Map<string, boolean>} */
20-
let apiCoverage = null;
21-
22-
/**
23-
* @param {!Object} classType
24-
* @param {string=} publicName
25-
*/
26-
function traceAPICoverage(classType, publicName) {
27-
if (!apiCoverage)
28-
return;
29-
30-
let className = publicName || classType.prototype.constructor.name;
31-
className = className.substring(0, 1).toLowerCase() + className.substring(1);
32-
for (const methodName of Reflect.ownKeys(classType.prototype)) {
33-
const method = Reflect.get(classType.prototype, methodName);
34-
if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function')
35-
continue;
36-
apiCoverage.set(`${className}.${methodName}`, false);
37-
Reflect.set(classType.prototype, methodName, function(...args) {
38-
apiCoverage.set(`${className}.${methodName}`, true);
39-
return method.call(this, ...args);
40-
});
41-
}
42-
43-
if (classType.Events) {
44-
for (const event of Object.values(classType.Events))
45-
apiCoverage.set(`${className}.emit(${JSON.stringify(event)})`, false);
46-
const method = Reflect.get(classType.prototype, 'emit');
47-
Reflect.set(classType.prototype, 'emit', function(event, ...args) {
48-
if (this.listenerCount(event))
49-
apiCoverage.set(`${className}.emit(${JSON.stringify(event)})`, true);
50-
return method.call(this, event, ...args);
51-
});
52-
}
53-
}
5419

5520
class Helper {
5621
/**
@@ -133,9 +98,8 @@ class Helper {
13398

13499
/**
135100
* @param {!Object} classType
136-
* @param {string=} publicName
137101
*/
138-
static tracePublicAPI(classType, publicName) {
102+
static installAsyncStackHooks(classType) {
139103
for (const methodName of Reflect.ownKeys(classType.prototype)) {
140104
const method = Reflect.get(classType.prototype, methodName);
141105
if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function' || method.constructor.name !== 'AsyncFunction')
@@ -151,8 +115,6 @@ class Helper {
151115
});
152116
});
153117
}
154-
155-
traceAPICoverage(classType, publicName);
156118
}
157119

158120
/**
@@ -175,17 +137,6 @@ class Helper {
175137
listeners.splice(0, listeners.length);
176138
}
177139

178-
/**
179-
* @return {?Map<string, boolean>}
180-
*/
181-
static publicAPICoverage() {
182-
return apiCoverage;
183-
}
184-
185-
static recordPublicAPICoverage() {
186-
apiCoverage = new Map();
187-
}
188-
189140
/**
190141
* @param {!Object} obj
191142
* @return {boolean}

Diff for: ‎test/test.js

+11-19
Original file line numberDiff line numberDiff line change
@@ -75,32 +75,24 @@ beforeEach(async({server, httpsServer}) => {
7575
httpsServer.reset();
7676
});
7777

78-
describe('Chromium', () => {
79-
const {helper} = require('../lib/helper');
80-
if (process.env.COVERAGE)
81-
helper.recordPublicAPICoverage();
78+
const CHROMIUM_NO_COVERAGE = new Set([
79+
'page.bringToFront',
80+
'securityDetails.subjectName',
81+
'securityDetails.issuer',
82+
'securityDetails.validFrom',
83+
'securityDetails.validTo',
84+
...(headless ? [] : ['page.pdf']),
85+
]);
8286

87+
describe('Chromium', () => {
8388
require('./puppeteer.spec.js').addTests({
8489
product: 'Chromium',
8590
puppeteer: utils.requireRoot('index'),
8691
defaultBrowserOptions,
8792
testRunner,
8893
});
89-
if (process.env.COVERAGE) {
90-
describe('COVERAGE', function() {
91-
const coverage = helper.publicAPICoverage();
92-
const disabled = new Set(['page.bringToFront']);
93-
if (!headless)
94-
disabled.add('page.pdf');
95-
96-
for (const method of coverage.keys()) {
97-
(disabled.has(method) ? xit : it)(`public api '${method}' should be called`, async({page, server}) => {
98-
if (!coverage.get(method))
99-
throw new Error('NOT CALLED!');
100-
});
101-
}
102-
});
103-
}
94+
if (process.env.COVERAGE)
95+
utils.recordAPICoverage(testRunner, require('../lib/api'), CHROMIUM_NO_COVERAGE);
10496
});
10597

10698
if (process.env.CI && testRunner.hasFocusedTestsOrSuites()) {

0 commit comments

Comments
 (0)