Skip to content

Commit 663d507

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
Set timeout for all SnapshotTester tests.
First attempt was reverted (https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7100799). This one actually sets the timeout correctly by creating the SnapshotTester directly in the `describe` closure, rather than in the `before` (which is too late to modify the timeout). Also moved the `before` and `after` hooks into the SnapshotTester ctor. Bug: 449129415 Change-Id: I4dc0e918a67b9b24219c48d648cbfde3e593f0ff Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7102919 Auto-Submit: Connor Clark <[email protected]> Commit-Queue: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent a42bb00 commit 663d507

File tree

10 files changed

+32
-91
lines changed

10 files changed

+32
-91
lines changed

front_end/core/sdk/RehydratingConnection.test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,21 +193,17 @@ describe('RehydratingSession', () => {
193193
});
194194
});
195195

196-
describeWithEnvironment('RehydratingConnection emittance', () => {
197-
let snapshotTester: SnapshotTester;
196+
describeWithEnvironment('RehydratingConnection emittance', function() {
197+
const snapshotTester = new SnapshotTester(this, import.meta);
198198

199199
before(async () => {
200-
snapshotTester = new SnapshotTester(import.meta);
201-
await snapshotTester.load();
202-
203200
// Create fake popup opener as rehydrating connection needs it.
204201
window.opener = {
205202
postMessage: sinon.stub(),
206203
};
207204
});
208205

209206
after(async () => {
210-
await snapshotTester.finish();
211207
delete window.opener;
212208
});
213209

front_end/models/ai_assistance/AiHistoryStorage.test.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -546,16 +546,8 @@ describe('AiHistoryStorage', () => {
546546
});
547547
});
548548

549-
describe('getConversationMarkdown', () => {
550-
let snapshotTester: SnapshotTester;
551-
before(async () => {
552-
snapshotTester = new SnapshotTester(import.meta);
553-
await snapshotTester.load();
554-
});
555-
556-
after(async () => {
557-
await snapshotTester.finish();
558-
});
549+
describe('getConversationMarkdown', function() {
550+
const snapshotTester = new SnapshotTester(this, import.meta);
559551

560552
it('should generate markdown from a conversation', function() {
561553
const fakeTime = new Date('2024-01-01T00:00:00.000Z');

front_end/models/ai_assistance/agents/StylingAgent.test.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import * as AiAssistance from '../ai_assistance.js';
1717

1818
const {StylingAgent, AiAgent} = AiAssistance;
1919

20-
describeWithEnvironment('StylingAgent', () => {
20+
describeWithEnvironment('StylingAgent', function() {
21+
const snapshotTester = new SnapshotTester(this, import.meta);
22+
2123
function mockHostConfig(
2224
modelId?: string, temperature?: number, userTier?: string,
2325
executionMode?: Root.Runtime.HostConfigFreestylerExecutionMode, multimodal?: boolean) {
@@ -60,16 +62,6 @@ describeWithEnvironment('StylingAgent', () => {
6062
element.backendNodeId.returns(99 as unknown as ReturnType<SDK.DOMModel.DOMNode['backendNodeId']>);
6163
});
6264

63-
let snapshotTester: SnapshotTester;
64-
before(async () => {
65-
snapshotTester = new SnapshotTester(import.meta);
66-
await snapshotTester.load();
67-
});
68-
69-
after(async () => {
70-
await snapshotTester.finish();
71-
});
72-
7365
describe('describeElement', () => {
7466
it('should describe an element with no children, siblings, or parent', async function() {
7567
element.simpleSelector.returns('div#myElement');

front_end/models/ai_assistance/data_formatters/PerformanceInsightFormatter.test.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,7 @@ import type * as Trace from '../../trace/trace.js';
1212
import {AIContext, PerformanceInsightFormatter} from '../ai_assistance.js';
1313

1414
describeWithLocale('PerformanceInsightFormatter', function() {
15-
this.timeout(20000);
16-
17-
let snapshotTester: SnapshotTester;
18-
before(async () => {
19-
snapshotTester = new SnapshotTester(import.meta);
20-
await snapshotTester.load();
21-
});
22-
23-
after(async () => {
24-
await snapshotTester.finish();
25-
});
15+
const snapshotTester = new SnapshotTester(this, import.meta);
2616

2717
setupRuntimeHooks();
2818
setupSettingsHooks();

front_end/models/ai_assistance/data_formatters/PerformanceTraceFormatter.test.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,7 @@ async function createFormatter(context: Mocha.Context|Mocha.Suite|null, name: st
2323
}
2424

2525
describeWithLocale('PerformanceTraceFormatter', function() {
26-
this.timeout(20000);
27-
28-
let snapshotTester: SnapshotTester;
29-
before(async () => {
30-
snapshotTester = new SnapshotTester(import.meta);
31-
await snapshotTester.load();
32-
});
33-
34-
after(async () => {
35-
await snapshotTester.finish();
36-
});
26+
const snapshotTester = new SnapshotTester(this, import.meta);
3727

3828
setupRuntimeHooks();
3929
setupSettingsHooks();

front_end/models/ai_assistance/performance/AICallTree.test.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,8 @@ import {AICallTree} from '../ai_assistance.js';
1111

1212
const NODE_NAME_INDEX = 2;
1313

14-
describeWithEnvironment('AICallTree', () => {
15-
let snapshotTester: SnapshotTester;
16-
before(async () => {
17-
snapshotTester = new SnapshotTester(import.meta);
18-
await snapshotTester.load();
19-
});
20-
21-
after(async () => {
22-
await snapshotTester.finish();
23-
});
14+
describeWithEnvironment('AICallTree', function() {
15+
const snapshotTester = new SnapshotTester(this, import.meta);
2416

2517
it('will not build a tree from non-main-thread events', async function() {
2618
const parsedTrace = await TraceLoader.traceEngine(this, 'cls-single-frame.json.gz');

front_end/panels/ai_assistance/AiAssistancePanel.test.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,16 +1738,8 @@ describeWithMockConnection('AI Assistance Panel', () => {
17381738
});
17391739
});
17401740

1741-
describe('getResponseMarkdown', () => {
1742-
let snapshotTester: SnapshotTester;
1743-
before(async () => {
1744-
snapshotTester = new SnapshotTester(import.meta);
1745-
await snapshotTester.load();
1746-
});
1747-
1748-
after(async () => {
1749-
await snapshotTester.finish();
1750-
});
1741+
describe('getResponseMarkdown', function() {
1742+
const snapshotTester = new SnapshotTester(this, import.meta);
17511743

17521744
it('should generate correct markdown from a message object', function() {
17531745
const message: AiAssistancePanel.ModelChatMessage = {

front_end/testing/README.md

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,10 @@ For Karma unit tests, you can add regression tests for string output (similar to
9696
9797
Results for all snapshots for a test file `MyFile.test.ts` are written to `MyFile.snapshot.txt`.
9898

99-
To add snapshot tests to a new test file, add a before and after hook to the describe test suite:
99+
To add snapshot tests to a new test file, add this within the `describe` test suite:
100100

101101
```
102-
let snapshotTester: SnapshotTester;
103-
before(async () => {
104-
snapshotTester = new SnapshotTester(import.meta);
105-
await snapshotTester.load();
106-
});
107-
108-
after(async () => {
109-
await snapshotTester.finish();
110-
});
102+
const snapshotTester = new SnapshotTester(this, import.meta);
111103
```
112104

113105
Then inside a test, call `snapshotTester.assert(this, output)`. For an example, see SnapshotTester.test.ts.

front_end/testing/SnapshotTester.test.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,8 @@
44

55
import {SnapshotTester} from './SnapshotTester.js';
66

7-
// Started failing and blocking CfT rolls since 143.0.7447.0
8-
describe.skip('[crbug.com/449129415]: SnapshotTester', () => {
9-
let snapshotTester: SnapshotTester;
10-
before(async () => {
11-
snapshotTester = new SnapshotTester(import.meta);
12-
await snapshotTester.load();
13-
});
14-
15-
after(async () => {
16-
await snapshotTester.finish();
17-
});
7+
describe('SnapshotTester', function() {
8+
const snapshotTester = new SnapshotTester(this, import.meta);
189

1910
it('example snapshot', function() {
2011
snapshotTester.assert(this, 'hello world');

front_end/testing/SnapshotTester.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,21 @@ class BaseSnapshotTester {
3737
#anyFailures = false;
3838
#newTests = false;
3939

40-
constructor(meta: ImportMeta) {
40+
constructor(context: Mocha.Suite, meta: ImportMeta) {
41+
if (context.timeout() > 0) {
42+
// The first usage of SnapshotTester in a test seems to take an extraordinary
43+
// amount of time, so let's bump the timeout for all snapshot tests.
44+
context.timeout(Math.max(context.timeout(), 45000));
45+
}
46+
47+
context.beforeAll(async () => {
48+
await this.load();
49+
});
50+
51+
context.afterAll(async () => {
52+
await this.finish();
53+
});
54+
4155
// out/Default/gen/third_party/devtools-frontend/src/front_end/testing/SnapshotTester.test.js?8ee4f2b123e221040a4aa075a28d0e5b41d3d3ed
4256
// ->
4357
// front_end/testing/SnapshotTester.snapshot.txt

0 commit comments

Comments
 (0)