chrome/test/data/webui/extensions/cr_extensions_browsertest.js - Issue 2974583003: MD Extensions: Show messages for no extensions, and for no search results - Code Review

Chromium Code Reviews

chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out

(95)

My Issues | Starred Open | Closed | All

Side by Side Diff: chrome/test/data/webui/extensions/cr_extensions_browsertest.js

Issue 2974583003: MD Extensions: Show messages for no extensions, and for no search results (Closed)

Patch Set: update tests Created 3 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« chrome/browser/resources/md_extensions/item_list.html ('K') | « chrome/browser/ui/webui/extensions/extensions_ui.cc ('k') | chrome/test/data/webui/extensions/extension_item_list_test.js » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright 2015 The Chromium Authors. All rights reserved.	1 // Copyright 2015 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 /** @fileoverview Runs the Polymer Settings tests. */	5 /** @fileoverview Runs the Polymer Settings tests. */

6	6

7 /** @const {string} Path to source root. */	7 /** @const {string} Path to source root. */

8 var ROOT_PATH = '../../../../../';	8 var ROOT_PATH = '../../../../../';

9	9

10 // Polymer BrowserTest fixture.	10 // Polymer BrowserTest fixture.

(...skipping 172 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
183 extension_detail_view_tests.registerTests();	183 extension_detail_view_tests.registerTests();

184 mocha.grep(	184 mocha.grep(

185 assert(extension_detail_view_tests.TestNames.ClickableElements)).run();	185 assert(extension_detail_view_tests.TestNames.ClickableElements)).run();

186 });	186 });

187	187

188 ////////////////////////////////////////////////////////////////////////////////	188 ////////////////////////////////////////////////////////////////////////////////

189 // Extension Item List Tests	189 // Extension Item List Tests

190	190

191 TEST_F('CrExtensionsBrowserTest', 'ExtensionItemList', function() {	191 TEST_F('CrExtensionsBrowserTest', 'ExtensionItemList', function() {

192 extension_item_list_tests.registerTests();	192 extension_item_list_tests.registerTests();

193 mocha.grep(	193 mocha.run();

194 assert(extension_item_list_tests.TestNames.ItemListFiltering)).run();
Devlin 2017/07/10 20:23:53 Way back when we first started, Dan was really wor Way back when we first started, Dan was really worried about tests timing out or interfering with one another, and requested that each test case have a separate browser test. Has that guidance changed? If so, we should probably update these all in a separate CL, rather than have this deviate. If not, we should just add a new TEST_F for the new tests. scottchen 2017/07/10 22:13:40 Hmm, I just noticed the tests are registered a bit Show quoted text On 2017/07/10 20:23:53, Devlin wrote: > Way back when we first started, Dan was really worried about tests timing out or > interfering with one another, and requested that each test case have a separate > browser test. Has that guidance changed? If so, we should probably update > these all in a separate CL, rather than have this deviate. If not, we should > just add a new TEST_F for the new tests. Hmm, I just noticed the tests are registered a bit differently here than in settings, so I'll just grep for all 3 tests for now. We can do a sweeping change in a separate CL to unify with how we do it in settings later on. dpapad 2017/07/11 01:36:56 Timing out tests is still a concern. Adding too ma Show quoted text On 2017/07/10 at 22:13:40, scottchen wrote: > On 2017/07/10 20:23:53, Devlin wrote: > > Way back when we first started, Dan was really worried about tests timing out or > > interfering with one another, and requested that each test case have a separate > > browser test. Has that guidance changed? If so, we should probably update > > these all in a separate CL, rather than have this deviate. If not, we should > > just add a new TEST_F for the new tests. > > Hmm, I just noticed the tests are registered a bit differently here than in settings, so I'll just grep for all 3 tests for now. We can do a sweeping change in a separate CL to unify with how we do it in settings later on. Timing out tests is still a concern. Adding too many mocha test cases behind a single TEST_F will eventually trigger some timeouts (on some bots). Regarding leaking state between tests, theoretically (and in practice in MD Settings tests), custom elements make it fairly easy to avoid such leaks. The pattern in settings is to empty the <body>, programmatically the element to be tested, attach it to the DOM and start testing it. In other places, like print preview's tests, clearing state between tests is not easy (no custom elements), therefore when I migrated TEST_F to use Mocha, I followed the 1:1 TEST_F to mocha test case approach, see [1]. [1] https://cs.chromium.org/chromium/src/chrome/test/data/webui/print_preview/pri.... scottchen 2017/07/11 18:08:17 Acknowledged. I moved them each to their own TEST_ Show quoted text On 2017/07/11 01:36:56, dpapad wrote: > On 2017/07/10 at 22:13:40, scottchen wrote: > > On 2017/07/10 20:23:53, Devlin wrote: > > > Way back when we first started, Dan was really worried about tests timing > out or > > > interfering with one another, and requested that each test case have a > separate > > > browser test. Has that guidance changed? If so, we should probably update > > > these all in a separate CL, rather than have this deviate. If not, we > should > > > just add a new TEST_F for the new tests. > > > > Hmm, I just noticed the tests are registered a bit differently here than in > settings, so I'll just grep for all 3 tests for now. We can do a sweeping change > in a separate CL to unify with how we do it in settings later on. > > Timing out tests is still a concern. Adding too many mocha test cases behind a > single TEST_F will eventually trigger some timeouts (on some bots). Regarding > leaking state between tests, theoretically (and in practice in MD Settings > tests), custom elements make it fairly easy to avoid such leaks. The pattern in > settings is to empty the <body>, programmatically the element to be tested, > attach it to the DOM and start testing it. > > In other places, like print preview's tests, clearing state between tests is not > easy (no custom elements), therefore when I migrated TEST_F to use Mocha, I > followed the 1:1 TEST_F to mocha test case approach, see [1]. > > [1] > https://cs.chromium.org/chromium/src/chrome/test/data/webui/print_preview/pri.... Acknowledged. I moved them each to their own TEST_F.
195 });	194 });

196	195

197 ////////////////////////////////////////////////////////////////////////////////	196 ////////////////////////////////////////////////////////////////////////////////

198 // Extension Load Error Tests	197 // Extension Load Error Tests

199	198

200 TEST_F('CrExtensionsBrowserTest', 'ExtensionLoadErrorInteractionTest',	199 TEST_F('CrExtensionsBrowserTest', 'ExtensionLoadErrorInteractionTest',

201 function() {	200 function() {

202 extension_load_error_tests.registerTests();	201 extension_load_error_tests.registerTests();

203 mocha.grep(assert(extension_load_error_tests.TestNames.Interaction)).run();	202 mocha.grep(assert(extension_load_error_tests.TestNames.Interaction)).run();

204 });	203 });

(...skipping 166 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
371 assert(extension_navigation_helper_tests.TestNames.Conversions)).run();	370 assert(extension_navigation_helper_tests.TestNames.Conversions)).run();

372 });	371 });

373	372

374 TEST_F('CrExtensionsNavigationHelperBrowserTest',	373 TEST_F('CrExtensionsNavigationHelperBrowserTest',

375 'ExtensionNavigationHelperPushAndReplaceStateTest', function() {	374 'ExtensionNavigationHelperPushAndReplaceStateTest', function() {

376 extension_navigation_helper_tests.registerTests();	375 extension_navigation_helper_tests.registerTests();

377 mocha.grep(	376 mocha.grep(

378 assert(extension_navigation_helper_tests.TestNames.PushAndReplaceState))	377 assert(extension_navigation_helper_tests.TestNames.PushAndReplaceState))

379 .run();	378 .run();

380 });	379 });

OLD	NEW

« chrome/browser/resources/md_extensions/item_list.html ('K') | « chrome/browser/ui/webui/extensions/extensions_ui.cc ('k') | chrome/test/data/webui/extensions/extension_item_list_test.js » ('j') | no next file with comments »

Powered by Google App Engine

This is Rietveld 408576698