Skip to content

Commit 66818b3

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored and
Devtools-frontend LUCI CQ
committed
tests: enable the no-widget-show-document-body rule and fix violations
Bug: none Change-Id: I4433ad84d72aceffdd5f4533ad8006b1125a4ca6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6579410 Reviewed-by: Alex Rudenko <alexrudenko@chromium.org> Commit-Queue: Alex Rudenko <alexrudenko@chromium.org> Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Reviewed-by: Victor Porof <victorporof@chromium.org>
1 parent 4e16d93 commit 66818b3

26 files changed

+87
-99
lines changed

eslint.config.mjs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
// found in the LICENSE file.
44

55
import stylisticPlugin from '@stylistic/eslint-plugin';
6-
import { defineConfig, globalIgnores } from 'eslint/config';
76
import eslintPlugin from 'eslint-plugin-eslint-plugin';
87
import importPlugin from 'eslint-plugin-import';
98
import jsdocPlugin from 'eslint-plugin-jsdoc';
109
import mochaPlugin from 'eslint-plugin-mocha';
10+
import {defineConfig, globalIgnores} from 'eslint/config';
1111
import globals from 'globals';
12-
import { join } from 'path';
12+
import {join} from 'path';
1313
import typescriptEslint from 'typescript-eslint';
1414

1515
import rulesdirPlugin from './scripts/eslint_rules/rules-dir.mjs';
@@ -193,7 +193,7 @@ export default defineConfig([
193193
radix: 'error',
194194
'valid-typeof': 'error',
195195
'no-return-assign': ['error', 'always'],
196-
'no-implicit-coercion': ['error', { allow: ['!!'] }],
196+
'no-implicit-coercion': ['error', {allow: ['!!']}],
197197

198198
'no-array-constructor': 'error',
199199

@@ -304,11 +304,11 @@ export default defineConfig([
304304
parserOptions: {
305305
allowAutomaticSingleRunInference: true,
306306
project: join(
307-
import.meta.dirname,
308-
'config',
309-
'typescript',
310-
'tsconfig.eslint.json',
311-
),
307+
import.meta.dirname,
308+
'config',
309+
'typescript',
310+
'tsconfig.eslint.json',
311+
),
312312
},
313313
},
314314

@@ -550,12 +550,12 @@ export default defineConfig([
550550
{
551551
// Enforce that any import of models/trace/trace.js names the import Trace.
552552
modulePath: join(
553-
import.meta.dirname,
554-
'front_end',
555-
'models',
556-
'trace',
557-
'trace.js',
558-
),
553+
import.meta.dirname,
554+
'front_end',
555+
'models',
556+
'trace',
557+
'trace.js',
558+
),
559559
importName: 'Trace',
560560
},
561561
],
@@ -699,6 +699,7 @@ export default defineConfig([
699699
'rulesdir/prefer-sinon-assert': 'error',
700700
'rulesdir/prefer-url-string': 'error',
701701
'rulesdir/trace-engine-test-timeouts': 'error',
702+
'rulesdir/no-widget-show-document-body': 'error',
702703
'rulesdir/enforce-custom-element-definitions-location': 'off',
703704
},
704705

front_end/panels/accessibility/AccessibilitySidebarView.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import * as SDK from '../../core/sdk/sdk.js';
66
import type * as Protocol from '../../generated/protocol.js';
7+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
78
import {createTarget, stubNoopSettings} from '../../testing/EnvironmentHelpers.js';
89
import {describeWithMockConnection, setMockConnectionResponseHandler} from '../../testing/MockConnection.js';
910

@@ -39,8 +40,7 @@ describeWithMockConnection('AccessibilitySidebarView', () => {
3940

4041
view = Accessibility.AccessibilitySidebarView.AccessibilitySidebarView.instance(
4142
{forceNew: true, throttlingTimeout: 0});
42-
view.markAsRoot();
43-
view.show(document.body);
43+
renderElementIntoDOM(view);
4444
view.setNode(node);
4545
await new Promise<void>(resolve => setTimeout(resolve, 0));
4646

front_end/panels/animation/AnimationTimeline.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import * as Common from '../../core/common/common.js';
66
import * as SDK from '../../core/sdk/sdk.js';
77
import * as Protocol from '../../generated/protocol.js';
8+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
89
import {
910
createTarget,
1011
stubNoopSettings,
@@ -166,7 +167,7 @@ describeWithMockConnection('AnimationTimeline', () => {
166167

167168
view = Animation.AnimationTimeline.AnimationTimeline.instance({forceNew: true});
168169
view.markAsRoot();
169-
view.show(document.body);
170+
renderElementIntoDOM(view);
170171
await new Promise<void>(resolve => setTimeout(resolve, 0));
171172

172173
const previewContainer = (view.contentElement.querySelector('.animation-timeline-buffer') as HTMLElement);
@@ -203,7 +204,7 @@ describeWithMockConnection('AnimationTimeline', () => {
203204
it('updates --timeline-controls-width and calls onResize', async () => {
204205
view = Animation.AnimationTimeline.AnimationTimeline.instance({forceNew: true});
205206
view.markAsRoot();
206-
view.show(document.body);
207+
renderElementIntoDOM(view);
207208
const onResizeStub = sinon.stub(view, 'onResize');
208209
await new Promise<void>(resolve => setTimeout(resolve, 0));
209210

@@ -246,7 +247,7 @@ describeWithMockConnection('AnimationTimeline', () => {
246247
beforeEach(async () => {
247248
view = Animation.AnimationTimeline.AnimationTimeline.instance({forceNew: true});
248249
view.markAsRoot();
249-
view.show(document.body);
250+
renderElementIntoDOM(view);
250251

251252
sinon.stub(view, 'animationGroupSelectedForTest').callsFake(() => {
252253
waitForAnimationGroupSelectedPromise.resolve();
@@ -396,7 +397,7 @@ describeWithMockConnection('AnimationTimeline', () => {
396397
beforeEach(async () => {
397398
view = Animation.AnimationTimeline.AnimationTimeline.instance({forceNew: true});
398399
view.markAsRoot();
399-
view.show(document.body);
400+
renderElementIntoDOM(view);
400401

401402
sinon.stub(view, 'animationGroupSelectedForTest').callsFake(() => {
402403
waitForAnimationGroupSelectedPromise.resolve();
@@ -491,7 +492,7 @@ describeWithMockConnection('AnimationTimeline', () => {
491492

492493
view = Animation.AnimationTimeline.AnimationTimeline.instance({forceNew: true});
493494
view.markAsRoot();
494-
view.show(document.body);
495+
renderElementIntoDOM(view);
495496

496497
sinon.stub(view, 'animationGroupSelectedForTest').callsFake(() => {
497498
waitForAnimationGroupSelectedPromise.resolve();
@@ -687,7 +688,7 @@ describeWithMockConnection('AnimationTimeline', () => {
687688

688689
// Render into document in order to see the computed styles.
689690
view.markAsRoot();
690-
view.show(document.body);
691+
renderElementIntoDOM(view);
691692
assert.deepEqual(window.getComputedStyle(placeholder).display, 'flex');
692693

693694
assert.deepEqual(placeholder.querySelector('.empty-state-header')?.textContent, 'Currently waiting for animations');
@@ -710,7 +711,7 @@ describeWithMockConnection('AnimationTimeline', () => {
710711
// Render into document in order to update the shown empty state.
711712
const view = Animation.AnimationTimeline.AnimationTimeline.instance();
712713
view.markAsRoot();
713-
view.show(document.body);
714+
renderElementIntoDOM(view);
714715

715716
const previewUpdatePromise = new ManualPromise();
716717
sinon.stub(view, 'previewsCreatedForTest').callsFake(() => {

front_end/panels/application/AppManifestView.test.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as Common from '../../core/common/common.js';
66
import * as Platform from '../../core/platform/platform.js';
77
import * as SDK from '../../core/sdk/sdk.js';
88
import type * as Protocol from '../../generated/protocol.js';
9-
import {getCleanTextContentFromElements} from '../../testing/DOMHelpers.js';
9+
import {getCleanTextContentFromElements, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
1010
import {createTarget, stubNoopSettings} from '../../testing/EnvironmentHelpers.js';
1111
import {describeWithMockConnection} from '../../testing/MockConnection.js';
1212
import * as UI from '../../ui/legacy/legacy.js';
@@ -52,8 +52,7 @@ describeWithMockConnection('AppManifestView', () => {
5252
sinon.stub(resourceTreeModel, 'getAppId').resolves({} as Protocol.Page.GetAppIdResponse);
5353

5454
view = new Application.AppManifestView.AppManifestView(emptyView, reportView, throttler);
55-
view.markAsRoot();
56-
view.show(document.body);
55+
renderElementIntoDOM(view);
5756

5857
await new Promise(resolve => {
5958
view.addEventListener(Application.AppManifestView.Events.MANIFEST_DETECTED, resolve, {once: true});
@@ -88,8 +87,7 @@ describeWithMockConnection('AppManifestView', () => {
8887
sinon.stub(resourceTreeModel, 'getAppId').resolves({} as Protocol.Page.GetAppIdResponse);
8988

9089
view = new Application.AppManifestView.AppManifestView(emptyView, reportView, throttler);
91-
view.markAsRoot();
92-
view.show(document.body);
90+
renderElementIntoDOM(view);
9391

9492
resourceTreeModel.dispatchEventToListeners(SDK.ResourceTreeModel.Events.DOMContentLoaded, 42);
9593
await new Promise(resolve => {
@@ -132,8 +130,7 @@ describeWithMockConnection('AppManifestView', () => {
132130
sinon.stub(resourceTreeModel, 'getAppId').resolves({} as Protocol.Page.GetAppIdResponse);
133131

134132
view = new Application.AppManifestView.AppManifestView(emptyView, reportView, throttler);
135-
view.markAsRoot();
136-
view.show(document.body);
133+
renderElementIntoDOM(view);
137134

138135
await new Promise(resolve => {
139136
view.addEventListener(Application.AppManifestView.Events.MANIFEST_RENDERED, resolve, {once: true});

front_end/panels/application/ServiceWorkersView.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import * as SDK from '../../core/sdk/sdk.js';
66
import * as Protocol from '../../generated/protocol.js';
7+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
78
import {createTarget} from '../../testing/EnvironmentHelpers.js';
89
import {describeWithMockConnection} from '../../testing/MockConnection.js';
910

@@ -25,8 +26,7 @@ describeWithMockConnection('ServiceWorkersView', () => {
2526

2627
it('shows service worker registrations', async () => {
2728
view = new Application.ServiceWorkersView.ServiceWorkersView();
28-
view.markAsRoot();
29-
view.show(document.body);
29+
renderElementIntoDOM(view);
3030
const serviceWorkersManager = target.model(SDK.ServiceWorkerManager.ServiceWorkerManager);
3131
assert.exists(serviceWorkersManager);
3232
const securityOriginManager = target.model(SDK.SecurityOriginManager.SecurityOriginManager);
@@ -72,8 +72,7 @@ describeWithMockConnection('ServiceWorkersView', () => {
7272
beforeEach(() => {
7373
Application.ServiceWorkersView.setThrottleDisabledForDebugging(true);
7474
view = new Application.ServiceWorkersView.ServiceWorkersView();
75-
view.markAsRoot();
76-
view.show(document.body);
75+
renderElementIntoDOM(view);
7776

7877
serviceWorkersManager = target.model(SDK.ServiceWorkerManager.ServiceWorkerManager);
7978
assert.exists(serviceWorkersManager);

front_end/panels/application/StorageBucketsTreeElement.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ describeWithMockConnection('StorageBucketsTreeElement', function() {
111111

112112
const panel = Application.ResourcesPanel.ResourcesPanel.instance({forceNew: true});
113113
panel.markAsRoot();
114-
panel.show(document.body);
114+
renderElementIntoDOM(panel);
115115

116116
const parentTreeElement = new Application.StorageBucketsTreeElement.StorageBucketsTreeParentElement(panel);
117117
const appendChildSpy = sinon.spy(parentTreeElement, 'appendChild');

front_end/panels/console/ConsoleView.test.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as SDK from '../../core/sdk/sdk.js';
1010
import * as Protocol from '../../generated/protocol.js';
1111
import * as IssuesManager from '../../models/issues_manager/issues_manager.js';
1212
import {findMenuItemWithLabel, getContextMenuForElement} from '../../testing/ContextMenuHelpers.js';
13-
import {dispatchPasteEvent} from '../../testing/DOMHelpers.js';
13+
import {dispatchPasteEvent, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
1414
import {createTarget, registerNoopActions} from '../../testing/EnvironmentHelpers.js';
1515
import {expectCall, expectCalled} from '../../testing/ExpectStubCall.js';
1616
import {stubFileManager} from '../../testing/FileManagerHelpers.js';
@@ -37,7 +37,6 @@ describeWithMockConnection('ConsoleView', () => {
3737
consoleView.element.querySelector('devtools-toolbar')!.querySelectorAll('devtools-checkbox');
3838
if (!consoleSettingsCheckboxes) {
3939
assert.fail('No checkbox found in console settings');
40-
return;
4140
}
4241
for (const checkbox of consoleSettingsCheckboxes) {
4342
assert.isTrue(checkbox.shadowRoot?.querySelector('.devtools-checkbox-text')?.hasAttribute('title'));
@@ -123,7 +122,7 @@ describeWithMockConnection('ConsoleView', () => {
123122
target = createTarget();
124123
SDK.TargetManager.TargetManager.instance().setScopeTarget(inScope ? target : null);
125124
consoleView.markAsRoot();
126-
consoleView.show(document.body);
125+
renderElementIntoDOM(consoleView);
127126
});
128127

129128
it('adds messages', async () => {
@@ -162,7 +161,7 @@ describeWithMockConnection('ConsoleView', () => {
162161
SDK.TargetManager.TargetManager.instance().setScopeTarget(target);
163162
const anotherTarget = createTarget();
164163
consoleView.markAsRoot();
165-
consoleView.show(document.body);
164+
renderElementIntoDOM(consoleView);
166165

167166
const consoleModel = target.model(SDK.ConsoleModel.ConsoleModel);
168167
assert.exists(consoleModel);
@@ -190,7 +189,7 @@ describeWithMockConnection('ConsoleView', () => {
190189
target = createTarget();
191190
SDK.TargetManager.TargetManager.instance().setScopeTarget(target);
192191
consoleView.markAsRoot();
193-
consoleView.show(document.body);
192+
renderElementIntoDOM(consoleView);
194193
});
195194

196195
it('shows', async () => {
@@ -244,7 +243,7 @@ describeWithMockConnection('ConsoleView', () => {
244243
const target = createTarget();
245244
SDK.TargetManager.TargetManager.instance().setScopeTarget(target);
246245
consoleView.markAsRoot();
247-
consoleView.show(document.body);
246+
renderElementIntoDOM(consoleView);
248247

249248
const consoleModel = target.model(SDK.ConsoleModel.ConsoleModel);
250249
assert.exists(consoleModel);
@@ -270,7 +269,7 @@ describeWithMockConnection('ConsoleView', () => {
270269
sinon.assert.calledOnce(spy);
271270

272271
// Continues updating the issue counter
273-
consoleView.show(document.body);
272+
renderElementIntoDOM(consoleView);
274273
issuesManager.dispatchEventToListeners(IssuesManager.IssuesManager.Events.ISSUES_COUNT_UPDATED);
275274
sinon.assert.calledTwice(spy);
276275
});

front_end/panels/coverage/CoverageView.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as SDK from '../../core/sdk/sdk.js';
66
import * as Protocol from '../../generated/protocol.js';
77
import * as Bindings from '../../models/bindings/bindings.js';
88
import * as Workspace from '../../models/workspace/workspace.js';
9-
import {dispatchClickEvent} from '../../testing/DOMHelpers.js';
9+
import {dispatchClickEvent, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
1010
import {createTarget, registerNoopActions} from '../../testing/EnvironmentHelpers.js';
1111
import {describeWithMockConnection} from '../../testing/MockConnection.js';
1212
import {activate, getMainFrame, navigate} from '../../testing/ResourceTreeHelpers.js';
@@ -112,8 +112,7 @@ describeWithMockConnection('CoverageView', () => {
112112
it('can handle back/forward cache navigations', async () => {
113113
const {startSpy, stopSpy, target} = setupTargetAndModels();
114114
const view = Coverage.CoverageView.CoverageView.instance();
115-
view.markAsRoot();
116-
view.show(document.body);
115+
renderElementIntoDOM(view);
117116
assert.isTrue(isShowingLandingPage(view));
118117
assert.isFalse(isShowingResults(view));
119118
assert.isFalse(isShowingPrerenderPage(view));
@@ -155,8 +154,7 @@ describeWithMockConnection('CoverageView', () => {
155154
it('can handle prerender activations', async () => {
156155
const {startSpy, stopSpy} = setupTargetAndModels();
157156
const view = Coverage.CoverageView.CoverageView.instance();
158-
view.markAsRoot();
159-
view.show(document.body);
157+
renderElementIntoDOM(view);
160158
assert.isTrue(isShowingLandingPage(view));
161159
assert.isFalse(isShowingResults(view));
162160
assert.isFalse(isShowingPrerenderPage(view));

front_end/panels/elements/AccessibilityTreeView.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import * as SDK from '../../core/sdk/sdk.js';
66
import type * as Protocol from '../../generated/protocol.js';
7+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
78
import {createTarget, stubNoopSettings} from '../../testing/EnvironmentHelpers.js';
89
import {describeWithMockConnection} from '../../testing/MockConnection.js';
910
import * as TreeOutline from '../../ui/components/tree_outline/tree_outline.js';
@@ -27,8 +28,7 @@ describeWithMockConnection('AccessibilityTreeView', () => {
2728
const updatesUiOnEvent = (inScope: boolean) => async () => {
2829
SDK.TargetManager.TargetManager.instance().setScopeTarget(inScope ? target : null);
2930
const view = new Elements.AccessibilityTreeView.AccessibilityTreeView(toggleButoon, treeComponent);
30-
view.markAsRoot();
31-
view.show(document.body);
31+
renderElementIntoDOM(view);
3232

3333
const model = target.model(SDK.AccessibilityModel.AccessibilityModel);
3434
const treeComponentDataSet = sinon.spy(treeComponent, 'data', ['set']);

front_end/panels/elements/ClassesPaneWidget.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as SDK from '../../core/sdk/sdk.js';
6+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
67
import {createTarget, stubNoopSettings} from '../../testing/EnvironmentHelpers.js';
78
import {describeWithMockConnection} from '../../testing/MockConnection.js';
89
import * as UI from '../../ui/legacy/legacy.js';
@@ -27,8 +28,7 @@ describeWithMockConnection('ClassesPaneWidget', () => {
2728
const updatesUiOnEvent = (inScope: boolean) => async () => {
2829
SDK.TargetManager.TargetManager.instance().setScopeTarget(inScope ? target : null);
2930
view = new Elements.ClassesPaneWidget.ClassesPaneWidget();
30-
view.markAsRoot();
31-
view.show(document.body);
31+
renderElementIntoDOM(view);
3232

3333
const model = target.model(SDK.DOMModel.DOMModel);
3434
assert.exists(model);

front_end/panels/elements/ComputedStyleWidget.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44
import * as SDK from '../../core/sdk/sdk.js';
55
import type * as Protocol from '../../generated/protocol.js';
6+
import {renderElementIntoDOM} from '../../testing/DOMHelpers.js';
67
import {describeWithMockConnection} from '../../testing/MockConnection.js';
78
import type * as TreeOutline from '../../ui/components/tree_outline/tree_outline.js';
89

@@ -66,8 +67,7 @@ describeWithMockConnection('ComputedStyleWidget', () => {
6667
node,
6768
});
6869
const computedStyleWidget = new Elements.ComputedStyleWidget.ComputedStyleWidget(computedStyleModel);
69-
computedStyleWidget.markAsRoot();
70-
computedStyleWidget.show(document.body);
70+
renderElementIntoDOM(computedStyleWidget);
7171

7272
return computedStyleWidget;
7373
}

0 commit comments

Comments
 (0)