Skip to content

Commit

Permalink
[Resolver] Fix flaky test (#80576)
Browse files Browse the repository at this point in the history
Under stress, this test would fail to finish in 5 seconds. With this new
implementation it has better performance.
  • Loading branch information
Robert Austin authored Oct 15, 2020
1 parent 36c15e1 commit 40d6d4d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ export class Simulator {
*/
private readonly sideEffectSimulator: SideEffectSimulator;

/**
* An `enzyme` supported CSS selector for process node elements.
*/
public static nodeElementSelector({
entityID,
selected = false,
}: ProcessNodeElementSelectorOptions = {}): string {
let selector: string = baseNodeElementSelector;
if (entityID !== undefined) {
selector += `[data-test-resolver-node-id="${entityID}"]`;
}
if (selected) {
selector += '[aria-selected="true"]';
}
return selector;
}

constructor({
dataAccessLayer,
resolverComponentInstanceID,
Expand Down Expand Up @@ -193,7 +210,7 @@ export class Simulator {
* returns a `ReactWrapper` even if nothing is found, as that is how `enzyme` does things.
*/
public processNodeElements(options: ProcessNodeElementSelectorOptions = {}): ReactWrapper {
return this.domNodes(processNodeElementSelector(options));
return this.domNodes(Simulator.nodeElementSelector(options));
}

/**
Expand Down Expand Up @@ -230,7 +247,7 @@ export class Simulator {
*/
public unselectedProcessNode(entityID: string): ReactWrapper {
return this.processNodeElements({ entityID }).not(
processNodeElementSelector({ entityID, selected: true })
Simulator.nodeElementSelector({ entityID, selected: true })
);
}

Expand Down Expand Up @@ -265,6 +282,13 @@ export class Simulator {
return this.resolveWrapper(() => this.domNodes(`[data-test-subj="${selector}"]`));
}

/**
* Given a `role`, return DOM nodes that have it. Use this to assert that ARIA roles are present as expected.
*/
public domNodesWithRole(role: string): ReactWrapper {
return this.domNodes(`[role="${role}"]`);
}

/**
* Given a 'data-test-subj' selector, it will return the domNode
*/
Expand Down Expand Up @@ -318,7 +342,7 @@ export class Simulator {
}
}

const baseResolverSelector = '[data-test-subj="resolver:node"]';
const baseNodeElementSelector = '[data-test-subj="resolver:node"]';

interface ProcessNodeElementSelectorOptions {
/**
Expand All @@ -330,20 +354,3 @@ interface ProcessNodeElementSelectorOptions {
*/
selected?: boolean;
}

/**
* An `enzyme` supported CSS selector for process node elements.
*/
function processNodeElementSelector({
entityID,
selected = false,
}: ProcessNodeElementSelectorOptions = {}): string {
let selector: string = baseResolverSelector;
if (entityID !== undefined) {
selector += `[data-test-resolver-node-id="${entityID}"]`;
}
if (selected) {
selector += '[aria-selected="true"]';
}
return selector;
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,28 +137,32 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children',

it('should render 3 elements with "treeitem" roles, each owned by an element with a "tree" role', async () => {
await expect(
simulator.map(() => ({
nodesOwnedByTrees: simulator.testSubject('resolver:node').filterWhere((domNode) => {
/**
* This test verifies corectness w.r.t. the tree/treeitem roles
* From W3C: `Authors MUST ensure elements with role treeitem are contained in, or owned by, an element with the role group or tree.`
*
* https://www.w3.org/TR/wai-aria-1.1/#tree
* https://www.w3.org/TR/wai-aria-1.1/#treeitem
*
* w3c defines two ways for an element to be an "owned element"
* 1. Any DOM descendant
* 2. Any element specified as a child via aria-owns
* (see: https://www.w3.org/TR/wai-aria-1.1/#dfn-owned-element)
*
* In the context of Resolver (as of this writing) nodes/treeitems are children of the tree,
* but they could be moved out of the tree, provided that the tree is given an `aria-owns`
* attribute referring to them (method 2 above).
*/
return domNode.closest('[role="tree"]').length === 1;
}).length,
}))
).toYieldEqualTo({ nodesOwnedByTrees: 3 });
simulator.map(() => {
/**
* This test verifies corectness w.r.t. the tree/treeitem roles
* From W3C: `Authors MUST ensure elements with role treeitem are contained in, or owned by, an element with the role group or tree.`
*
* https://www.w3.org/TR/wai-aria-1.1/#tree
* https://www.w3.org/TR/wai-aria-1.1/#treeitem
*
* w3c defines two ways for an element to be an "owned element"
* 1. Any DOM descendant
* 2. Any element specified as a child via aria-owns
* (see: https://www.w3.org/TR/wai-aria-1.1/#dfn-owned-element)
*
* In the context of Resolver (as of this writing) nodes/treeitems are children of the tree,
* but they could be moved out of the tree, provided that the tree is given an `aria-owns`
* attribute referring to them (method 2 above).
*/
const tree = simulator.domNodesWithRole('tree');
return {
// There should be only one tree.
treeCount: tree.length,
// The tree should have 3 nodes in it.
nodesOwnedByTrees: tree.find(Simulator.nodeElementSelector()).length,
};
})
).toYieldEqualTo({ treeCount: 1, nodesOwnedByTrees: 3 });
});

it(`should show links to the 3 nodes (with icons) in the node list.`, async () => {
Expand Down

0 comments on commit 40d6d4d

Please sign in to comment.