Skip to content

Commit d5d1201

Browse files
OrKoNDevtools-frontend LUCI CQ
OrKoN
authored and
Devtools-frontend LUCI CQ
committed
[Refactor] Move mocha-interface to conductor
This allows using the same implementation to instrument test functions, thus, removing watchForHang. Bug: none Change-Id: Ibe1868bb0bd31fcdece342fac1d7b56fedf16506 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5891210 Reviewed-by: Simon Zünd <[email protected]> Commit-Queue: Alex Rudenko <[email protected]>
1 parent a33d953 commit d5d1201

16 files changed

+169
-195
lines changed

Diff for: ‎.eslintrc.js

-5
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,6 @@ module.exports = {
203203
importName: 'Trace'
204204
}]
205205
}
206-
}, {
207-
files: 'test/shared/mocha-interface.ts',
208-
rules: {
209-
'rulesdir/es_modules_import': 'off',
210-
}
211206
}, {
212207
files : ['*.ts'],
213208
rules : {

Diff for: ‎extensions/cxx_debugging/e2e/runner.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ def __call__(self, options):
372372
'ui':
373373
repo_path(options.build_root,
374374
get_artifact_dir('devtools-frontend'),
375-
'gen/test/shared/mocha-interface.js'),
375+
'gen/test/conductor/mocha-interface.js'),
376376
'spec': [
377377
repo_path(
378378
options.build_root,

Diff for: ‎test/conductor/BUILD.gn

+4
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,21 @@ group("conductor") {
1414

1515
node_ts_library("implementation") {
1616
sources = [
17+
"async-scope.ts",
1718
"commandline.ts",
1819
"custom-query-handlers.ts",
1920
"events.ts",
2021
"frontend_tab.ts",
2122
"hooks.ts",
2223
"karma-resultsdb-reporter.ts",
24+
"mocha-interface-helpers.ts",
25+
"mocha-interface.ts",
2326
"mocha_hooks.ts",
2427
"paths.ts",
2528
"pool.ts",
2629
"puppeteer-state.ts",
2730
"resultsdb.ts",
31+
"screenshot-error.ts",
2832
"target_tab.ts",
2933
"test_config.ts",
3034
"test_server.ts",

Diff for: ‎test/shared/async-scope.ts renamed to ‎test/conductor/async-scope.ts

File renamed without changes.

Diff for: ‎test/conductor/hooks.ts

+9-37
Original file line numberDiff line numberDiff line change
@@ -63,34 +63,6 @@ let targetTab: TargetTab;
6363

6464
const envChromeFeatures = process.env['CHROME_FEATURES'];
6565

66-
export async function watchForHang<T>(
67-
currentTest: string|undefined, stepFn: (currentTest: string|undefined) => Promise<T>): Promise<T> {
68-
const stepName = stepFn.name || stepFn.toString();
69-
const stackTrace = new Error().stack;
70-
function logTime(label: string) {
71-
const end = performance.now();
72-
console.error(`\n${stepName} ${label} ${end - start}ms\nTrace: ${stackTrace}\nTest: ${currentTest}\n`);
73-
}
74-
let tripped = false;
75-
const timerId = setTimeout(() => {
76-
logTime('takes at least');
77-
tripped = true;
78-
}, 10000);
79-
const start = performance.now();
80-
try {
81-
const result = await stepFn(currentTest);
82-
if (tripped) {
83-
logTime('succeded after');
84-
}
85-
return result;
86-
} catch (err) {
87-
logTime('errored after');
88-
throw err;
89-
} finally {
90-
clearTimeout(timerId);
91-
}
92-
}
93-
9466
function launchChrome() {
9567
// Use port 0 to request any free port.
9668
const enabledFeatures = [
@@ -195,24 +167,24 @@ export async function unregisterAllServiceWorkers() {
195167
});
196168
}
197169

198-
export async function setupPages(currentTest: string|undefined) {
170+
export async function setupPages() {
199171
const {frontend} = getBrowserAndPages();
200-
await watchForHang(currentTest, () => throttleCPUIfRequired(frontend));
201-
await watchForHang(currentTest, () => delayPromisesIfRequired(frontend));
172+
await throttleCPUIfRequired(frontend);
173+
await delayPromisesIfRequired(frontend);
202174
}
203175

204-
export async function resetPages(currentTest: string|undefined) {
176+
export async function resetPages() {
205177
const {frontend, target} = getBrowserAndPages();
206178

207-
await watchForHang(currentTest, () => target.bringToFront());
208-
await watchForHang(currentTest, () => targetTab.reset());
209-
await watchForHang(currentTest, () => frontend.bringToFront());
179+
await target.bringToFront();
180+
await targetTab.reset();
181+
await frontend.bringToFront();
210182

211183
if (TestConfig.serverType === 'hosted-mode') {
212-
await watchForHang(currentTest, () => frontendTab.reset());
184+
await frontendTab.reset();
213185
} else if (TestConfig.serverType === 'component-docs') {
214186
// Reset the frontend back to an empty page for the component docs server.
215-
await watchForHang(currentTest, () => loadEmptyPageAndWaitForContent(frontend));
187+
await loadEmptyPageAndWaitForContent(frontend);
216188
}
217189
}
218190

Diff for: ‎test/conductor/mocha-interface-helpers.ts

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Copyright 2024 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import type * as Mocha from 'mocha';
6+
import * as os from 'os';
7+
8+
import {AsyncScope} from './async-scope.js';
9+
import {getBrowserAndPages} from './puppeteer-state.js';
10+
import {ScreenshotError} from './screenshot-error.js';
11+
import {TestConfig} from './test_config.js';
12+
13+
declare global {
14+
/*
15+
* For tests containing screenshots.
16+
*/
17+
let itScreenshot: {
18+
(title: string, fn: Mocha.AsyncFunc): void,
19+
20+
skip: (title: string, fn: Mocha.AsyncFunc) => void,
21+
22+
skipOnPlatforms: (platforms: Array<Platform>, title: string, fn: Mocha.AsyncFunc) => void,
23+
};
24+
namespace Mocha {
25+
export interface TestFunction {
26+
skipOnPlatforms: (platforms: Array<Platform>, title: string, fn: Mocha.AsyncFunc) => void;
27+
}
28+
}
29+
}
30+
31+
export type Platform = 'mac'|'win32'|'linux';
32+
export let platform: Platform;
33+
switch (os.platform()) {
34+
case 'darwin':
35+
platform = 'mac';
36+
break;
37+
38+
case 'win32':
39+
platform = 'win32';
40+
break;
41+
42+
default:
43+
platform = 'linux';
44+
break;
45+
}
46+
47+
async function takeScreenshots(): Promise<{target?: string, frontend?: string}> {
48+
try {
49+
const {target, frontend} = getBrowserAndPages();
50+
const opts = {
51+
encoding: 'base64' as 'base64',
52+
};
53+
await target.bringToFront();
54+
const targetScreenshot = await target.screenshot(opts);
55+
await frontend.bringToFront();
56+
const frontendScreenshot = await frontend.screenshot(opts);
57+
return {target: targetScreenshot, frontend: frontendScreenshot};
58+
} catch (err) {
59+
console.error('Error taking a screenshot', err);
60+
return {};
61+
}
62+
}
63+
64+
async function createScreenshotError(error: Error): Promise<Error> {
65+
console.error('Taking screenshots for the error', error);
66+
if (!TestConfig.debug) {
67+
const screenshotTimeout = 5_000;
68+
const {target, frontend} = await Promise.race([
69+
takeScreenshots(),
70+
new Promise(resolve => {
71+
setTimeout(resolve, screenshotTimeout);
72+
}).then(() => {
73+
console.error(`Could not take screenshots within ${screenshotTimeout}ms.`);
74+
return {target: undefined, frontend: undefined};
75+
}),
76+
]);
77+
return ScreenshotError.fromBase64Images(error, target, frontend);
78+
}
79+
return error;
80+
}
81+
82+
export function makeInstrumentedTestFunction(fn: Mocha.AsyncFunc) {
83+
return async function testFunction(this: Mocha.Context) {
84+
const abortController = new AbortController();
85+
let resolver;
86+
let rejecter: (reason?: unknown) => void;
87+
const testPromise = new Promise((resolve, reject) => {
88+
resolver = resolve;
89+
rejecter = reject;
90+
});
91+
// AbortSignal for the current test function.
92+
AsyncScope.abortSignal = abortController.signal;
93+
// Promisify the function in case it is sync.
94+
const promise = (async () => fn.call(this))();
95+
const timeout = this.timeout();
96+
// Disable test timeout.
97+
this.timeout(0);
98+
const actualTimeout = timeout;
99+
const t = actualTimeout !== 0 ? setTimeout(async () => {
100+
abortController.abort();
101+
const stacks = [];
102+
const scopes = AsyncScope.scopes;
103+
for (const scope of scopes.values()) {
104+
const {descriptions, stack} = scope;
105+
if (stack) {
106+
const stepDescription = descriptions ? `${descriptions.join(' > ')}:\n` : '';
107+
stacks.push(`${stepDescription}${stack.join('\n')}\n`);
108+
}
109+
}
110+
const err = new Error('Test timed out');
111+
if (stacks.length > 0) {
112+
const msg = `Pending async operations during timeout:\n${stacks.join('\n\n')}`;
113+
err.cause = new Error(msg);
114+
}
115+
rejecter(await createScreenshotError(err));
116+
}, actualTimeout) : 0;
117+
promise
118+
.then(
119+
resolver,
120+
async err => {
121+
// Suppress errors after the test was aborted.
122+
if (abortController.signal.aborted) {
123+
return;
124+
}
125+
rejecter(await createScreenshotError(err));
126+
})
127+
.finally(() => {
128+
clearTimeout(t);
129+
});
130+
return testPromise;
131+
};
132+
}

Diff for: ‎test/shared/mocha-interface.ts renamed to ‎test/conductor/mocha-interface.ts

+3-94
Original file line numberDiff line numberDiff line change
@@ -4,106 +4,15 @@
44

55
import * as Mocha from 'mocha';
66
// @ts-expect-error
7-
import * as commonInterface from 'mocha/lib/interfaces/common.js';
7+
import * as commonInterface from 'mocha/lib/interfaces/common.js'; // eslint-disable-line rulesdir/es_modules_import
88
import * as Path from 'path';
99

10-
import {getBrowserAndPages} from '../conductor/puppeteer-state.js';
11-
import {TestConfig} from '../conductor/test_config.js';
12-
import {ScreenshotError} from '../shared/screenshot-error.js';
13-
14-
import {AsyncScope} from './async-scope.js';
15-
import {platform, type Platform} from './helper.js';
10+
import {makeInstrumentedTestFunction, platform, type Platform} from './mocha-interface-helpers.js';
11+
import {TestConfig} from './test_config.js';
1612

1713
type SuiteFunction = ((this: Mocha.Suite) => void)|undefined;
1814
type ExclusiveSuiteFunction = (this: Mocha.Suite) => void;
1915

20-
async function takeScreenshots(): Promise<{target?: string, frontend?: string}> {
21-
try {
22-
const {target, frontend} = getBrowserAndPages();
23-
const opts = {
24-
encoding: 'base64' as 'base64',
25-
};
26-
await target.bringToFront();
27-
const targetScreenshot = await target.screenshot(opts);
28-
await frontend.bringToFront();
29-
const frontendScreenshot = await frontend.screenshot(opts);
30-
return {target: targetScreenshot, frontend: frontendScreenshot};
31-
} catch (err) {
32-
console.error('Error taking a screenshot', err);
33-
return {};
34-
}
35-
}
36-
37-
async function createScreenshotError(error: Error): Promise<Error> {
38-
console.error('Taking screenshots for the error', error);
39-
if (!TestConfig.debug) {
40-
const screenshotTimeout = 5_000;
41-
const {target, frontend} = await Promise.race([
42-
takeScreenshots(),
43-
new Promise(resolve => {
44-
setTimeout(resolve, screenshotTimeout);
45-
}).then(() => {
46-
console.error(`Could not take screenshots within ${screenshotTimeout}ms.`);
47-
return {target: undefined, frontend: undefined};
48-
}),
49-
]);
50-
return ScreenshotError.fromBase64Images(error, target, frontend);
51-
}
52-
return error;
53-
}
54-
55-
function makeInstrumentedTestFunction(fn: Mocha.AsyncFunc) {
56-
return async function testFunction(this: Mocha.Context) {
57-
const abortController = new AbortController();
58-
let resolver;
59-
let rejecter: (reason?: unknown) => void;
60-
const testPromise = new Promise((resolve, reject) => {
61-
resolver = resolve;
62-
rejecter = reject;
63-
});
64-
// AbortSignal for the current test function.
65-
AsyncScope.abortSignal = abortController.signal;
66-
// Promisify the function in case it is sync.
67-
const promise = (async () => fn.call(this))();
68-
const timeout = this.timeout();
69-
// Disable test timeout.
70-
this.timeout(0);
71-
const actualTimeout = timeout;
72-
const t = actualTimeout !== 0 ? setTimeout(async () => {
73-
abortController.abort();
74-
const stacks = [];
75-
const scopes = AsyncScope.scopes;
76-
for (const scope of scopes.values()) {
77-
const {descriptions, stack} = scope;
78-
if (stack) {
79-
const stepDescription = descriptions ? `${descriptions.join(' > ')}:\n` : '';
80-
stacks.push(`${stepDescription}${stack.join('\n')}\n`);
81-
}
82-
}
83-
const err = new Error('Test timed out');
84-
if (stacks.length > 0) {
85-
const msg = `Pending async operations during timeout:\n${stacks.join('\n\n')}`;
86-
err.cause = new Error(msg);
87-
}
88-
rejecter(await createScreenshotError(err));
89-
}, actualTimeout) : 0;
90-
promise
91-
.then(
92-
resolver,
93-
async err => {
94-
// Suppress errors after the test was aborted.
95-
if (abortController.signal.aborted) {
96-
return;
97-
}
98-
rejecter(await createScreenshotError(err));
99-
})
100-
.finally(() => {
101-
clearTimeout(t);
102-
});
103-
return testPromise;
104-
};
105-
}
106-
10716
function devtoolsTestInterface(suite: Mocha.Suite) {
10817
const suites: [Mocha.Suite] = [suite];
10918
suite.on(

0 commit comments

Comments
 (0)