Commit 8abaccff authored by Sasha Morrissey's avatar Sasha Morrissey Committed by Commit Bot

Fix FolderShortcuts test to select by label

Change ShortcutItem to set the 'label' attribute of an element, so that
the FolderShortcuts test can select these elements, with a TODO to
remove later. This is the fastest way to fix the integration tests to
work when team drives is enabled (and the IDs move around). This also
makes the tests robust against more directories being added to the top-
level tree.

Other approaches considered:

(1) Add a getIdForTreeElementWithName method to
    background/js/test_util.js. This doesn't work with waitForElement()
    since calling it beforehand fails immediately when the element
    doesn't exist. Adding a waitForElementInTreeWithName() could work,
    but you would need both methods, and adding waitFor*() methods is a
    bit tricky to write.
(2) Use :nth-child selectors. This doesn't work because shortcuts are
    added in alphabetical order, meaning new shortcuts added might
    invalidate previous selectors. So the selectors aren't static.
(3) Pass the expected index of the shortcut into each method call. This
    works for all methods except ones involving Drive, which use a
    slightly different selector. Also, for methods like
    removeShortcut(), it's impossible to tell whether the item has been
    removed (since all the elements shift up by 1, the shortcut at this
    index is now just the next shortcut in the list).
(4) Keep track of the shortcut state in the test, and predict the
    indexes based on that. This is harder to do, and requires a
    nontrivial test rewrite.

Bug: 849497
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Id26c559ea893491cf942b19a874fee29bde79534
Reviewed-on: https://chromium-review.googlesource.com/1103434Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Commit-Queue: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568328}
parent 9dc53e6e
......@@ -933,6 +933,11 @@ function ShortcutItem(modelItem, tree) {
item.setContextMenu_(tree.contextMenuForRootItems);
item.label = modelItem.entry.name;
// Set the 'label' attribute of this element so it can be selected by tests.
// TODO(sashab): Figure out a reliable way to select elements in the directory
// tree by label and remove this.
item.setAttribute('label', item.label);
return item;
}
......
......@@ -49,7 +49,7 @@ var DIRECTORY = {
A: {
contents: [ENTRIES.directoryB.getExpectedRow()],
name: 'A',
navItem: '#tree-item-autogen-id-14',
navItem: '.tree-item[label="A"]',
treeItem: TREEITEM_A
},
B: {
......@@ -60,13 +60,13 @@ var DIRECTORY = {
C: {
contents: [],
name: 'C',
navItem: '#tree-item-autogen-id-14',
navItem: '.tree-item[label="C"]',
treeItem: TREEITEM_C
},
D: {
contents: [ENTRIES.directoryE.getExpectedRow()],
name: 'D',
navItem: '#tree-item-autogen-id-13',
navItem: '.tree-item[label="D"]',
treeItem: TREEITEM_D
},
E: {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment