Audit all mocha.grep() pattern in tests, because some tests end up running multiple times. |
|||
Issue descriptionExample from MD Extensions: The following two lines seemingly run different tests (see [1]) mocha.grep(assert(extension_item_list_tests.TestNames.Filtering)).run(); mocha.grep(assert(extension_item_list_tests.TestNames.NoSearchResultsMsg)).run() But (see [2]) TestNames.Filtering is defined as 'item list filtering' and NoSearchResultsMsg is defined as 'empty item list filtering results', Which causes the first mocha.grep() statement to actually run both tests, instead of just TestNames.Filtering. [1] https://cs.chromium.org/chromium/src/chrome/test/data/webui/extensions/cr_extensions_browsertest.js?l=223,237 [2] https://cs.chromium.org/chromium/src/chrome/test/data/webui/extensions/extension_item_list_test.js?l=9,11 Need to strengthen the mocha.grep() expression to be more strict, using $ and ^ to ensure that there is a full match, not a partial match)
,
Nov 9 2017
So, found a small complication while trying to fix this issue. My proposal is at [1], but it turns out that Mocha matches the RegExp against the full name of the test at [2], which means that the provided RegExp does not match anything. The full name for the example test mentioned in #1 is "ExtensionItemListTest item list filtering" basically mocha concatenates the suite name and test name (with spaces in between). We would have to modify our RegExp to include the suite name to work. [1] https://chromium-review.googlesource.com/c/chromium/src/+/761981 [2] https://cs.chromium.org/chromium/src/third_party/mocha/mocha.js?l=4643
,
Nov 10 2017
FYI, updated the proposal to address the problem of Mocha matching against the full name, see patchset 4.
,
Nov 10 2017
Re suite name: that's necessary because we could have multiple suites that all have a test named "basic" (as in the examples in the mocha docs). But it's unfortunate that it means yet more boilerplate.
We might also nest suites, like:
suite('mypage tests', function() {
suite('widget tests', function() {
test('click', function() {});
test('keypress', function() {});
});
suite('frobinator tests', function() {
test('click', function() {});
// ...
});
});
in which case we need to concatenate more than two levels of names.
Maybe the registerTests() functions we used to have weren't a terrible idea?
,
Nov 10 2017
I think test name collision can exist even without nested suites? For example:
suite('widget tests', function() {
test('click', function() {});
test('keypress', function() {});
});
suite('frobinator tests', function() {
test('click', function() {});
// ...
});
I don't think nested suites are particularly useful, and don't know if we actually use them, but I'll find out.
Regarding registerTests(): IIRC in settings we rarely used mocha.grep(), see [1], in which case it was just overhead.
[1] https://cs.chromium.org/search/?q=mocha.grep+file:%5Esrc/chrome/test/data/webui/settings/+package:%5Echromium$&type=cs
,
Nov 10 2017
,
Nov 10 2017
FYI, CL in review at https://chromium-review.googlesource.com/c/chromium/src/+/761981.
,
Nov 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a20e3814a2523987ea1bd8277b8ca742e581791 commit 7a20e3814a2523987ea1bd8277b8ca742e581791 Author: dpapad <dpapad@chromium.org> Date: Mon Nov 13 21:43:16 2017 Mocha: Make running single tests more robust. mocha.grep() does substring matching, which causes accidentally running some tests multiple times. Strengthen the RegExp passed to mocha.grep() to ensure that no sub-string matching occurs. This CL deploys the new runMochaTest() method in MD Extensions only. Remaining WebUI tests will be updated in follow up CLs. Before this CL there were actually a couple of tests accidentally run multiple times, fixed now. Bug: 783334 Change-Id: I63fe4fc327f5a2bba4307b087380f1883d337cc1 Reviewed-on: https://chromium-review.googlesource.com/761981 Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Commit-Position: refs/heads/master@{#516062} [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/cr_extensions_browsertest.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_code_section_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_detail_view_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_error_page_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_item_list_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_item_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_keyboard_shortcuts_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_load_error_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_manager_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_manager_unit_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_navigation_helper_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_options_dialog_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_pack_dialog_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_shortcut_input_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_sidebar_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_toolbar_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/extensions/extension_view_manager_test.js [modify] https://crrev.com/7a20e3814a2523987ea1bd8277b8ca742e581791/chrome/test/data/webui/mocha_adapter.js
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9204ed3c61c7f66db40f26ca4466fe67a116560d commit 9204ed3c61c7f66db40f26ca4466fe67a116560d Author: dpapad <dpapad@chromium.org> Date: Wed Nov 15 22:04:02 2017 Print preview tests: Ensure that mocha.grep is used properly. Bug: 783334 Change-Id: Ibb3aa19df112fff00dae02112c956af20484b500 Reviewed-on: https://chromium-review.googlesource.com/772351 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#516846} [modify] https://crrev.com/9204ed3c61c7f66db40f26ca4466fe67a116560d/chrome/test/data/webui/print_preview/print_preview_tests.js [modify] https://crrev.com/9204ed3c61c7f66db40f26ca4466fe67a116560d/chrome/test/data/webui/print_preview/print_preview_ui_browsertest.js |
|||
►
Sign in to add a comment |
|||
Comment 1 by michae...@chromium.org
, Nov 9 2017