Skip to content

Commit

Permalink
test: fix empty findAll results (#11401)
Browse files Browse the repository at this point in the history 
**Related Issue:** #11398 

## Summary

This introduces a replacement to using `page.findAll`/`element.findAll`
to avoid false positives from asserting on empty results (e.g.,
assertions in a loop).

Also, this will be enforced by the
[`no-restricted-properties`](https://eslint.org/docs/latest/rules/no-restricted-properties)
ESLint rule.
  • Loading branch information
@jcfranco
jcfranco authored Jan 29, 2025
1 parent a8b4e7a commit f208961
Show file tree
Hide file tree
Showing 40 changed files with 544 additions and 430 deletions.
7 changes: 7 additions & 0 deletions packages/calcite-components/eslint.config.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ export default tseslint.config(
],
},
],
"no-restricted-properties": [
"error",
{
property: "findAll",
message: "Use custom findAll test util for more predictable (non-empty) result usage.",
},
],

"@esri/calcite-components/no-dynamic-createelement": "warn",
"@esri/calcite-components/strict-boolean-attributes": "error",
Expand Down
30 changes: 18 additions & 12 deletions packages/calcite-components/src/components/accordion/accordion.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
import { accessible, defaults, hidden, reflects, renders, themed } from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { CSS as ACCORDION_ITEM_CSS } from "../accordion-item/resources";
import { findAll } from "../../tests/utils";
import { CSS } from "./resources";

describe("calcite-accordion", () => {
Expand Down Expand Up @@ -86,13 +87,13 @@ describe("calcite-accordion", () => {
]);
});

it("inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` modified on the parent get passed into items", async () => {
it.skip("inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` modified on the parent get passed into items", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-accordion icon-position="start" icon-type="plus-minus" selection-mode="single-persist" scale="l">
${accordionContentInheritablePropsNonDefault}
</calcite-accordion>`);
const accordionItems = await page.findAll("calcite-accordion-items");
const accordionItems = await findAll(page, "calcite-accordion-items");

for (const item of accordionItems) {
expect(await item.getProperty("iconPosition")).toBe("start");
Expand Down Expand Up @@ -142,8 +143,9 @@ describe("calcite-accordion", () => {
${accordionContent}
</calcite-accordion>`);
const element = await page.find("calcite-accordion");
const [item1, item2, item3] = await element.findAll("calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await element.findAll(
const [item1, item2, item3] = await findAll(element, "calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await findAll(
element,
`calcite-accordion-item >>> .${ACCORDION_ITEM_CSS.content}`,
);

Expand All @@ -166,8 +168,9 @@ describe("calcite-accordion", () => {
</calcite-accordion>`);
const element = await page.find("calcite-accordion");
expect(element).toEqualAttribute("selection-mode", "multiple");
const [item1, item2, item3] = await element.findAll("calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await element.findAll(
const [item1, item2, item3] = await findAll(element, "calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await findAll(
element,
`calcite-accordion-item >>> .${ACCORDION_ITEM_CSS.content}`,
);
await item1.click();
Expand All @@ -191,8 +194,9 @@ describe("calcite-accordion", () => {
</calcite-accordion>`);
const element = await page.find("calcite-accordion");
expect(element).toEqualAttribute("selection-mode", "single");
const [item1, item2, item3] = await element.findAll("calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await element.findAll(
const [item1, item2, item3] = await findAll(element, "calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await findAll(
element,
`calcite-accordion-item >>> .${ACCORDION_ITEM_CSS.content}`,
);
await item1.click();
Expand Down Expand Up @@ -240,8 +244,9 @@ describe("calcite-accordion", () => {

const element = await page.find("calcite-accordion");
expect(element).toEqualAttribute("selection-mode", "single-persist");
const [item1, item2, item3] = await element.findAll("calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await element.findAll(
const [item1, item2, item3] = await findAll(element, "calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await findAll(
element,
`calcite-accordion-item >>> .${ACCORDION_ITEM_CSS.content}`,
);
await item2.click();
Expand All @@ -267,8 +272,9 @@ describe("calcite-accordion", () => {
expect(element).toEqualAttribute("selection-mode", "single");
element.setAttribute("selection-mode", "multiple");
await page.waitForChanges();
const [item1, item2, item3] = await element.findAll("calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await element.findAll(
const [item1, item2, item3] = await findAll(element, "calcite-accordion-item");
const [item1Content, item2Content, item3Content] = await findAll(
element,
`calcite-accordion-item >>> .${ACCORDION_ITEM_CSS.content}`,
);
await item1.click();
Expand Down
30 changes: 15 additions & 15 deletions packages/calcite-components/src/components/action-bar/action-bar.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
t9n,
themed,
} from "../../tests/commonTests";
import { getFocusedElementProp } from "../../tests/utils";
import { findAll, getFocusedElementProp } from "../../tests/utils";
import { DEBOUNCE } from "../../utils/resources";
import type { ActionGroup } from "../action-group/action-group";
import { CSS, SLOTS } from "./resources";
Expand Down Expand Up @@ -358,7 +358,7 @@ describe("calcite-action-bar", () => {
</calcite-action-bar>`,
);

const groups = await page.findAll("calcite-action-group");
const groups = await findAll(page, "calcite-action-group");

expect(groups).toHaveLength(2);
expect(await groups[0].getProperty("menuOpen")).toBe(false);
Expand Down Expand Up @@ -413,8 +413,8 @@ describe("calcite-action-bar", () => {
});
await page.waitForTimeout(DEBOUNCE.resize);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(2);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(0);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(2);
expect(await findAll(page, slottedActionsSelector, { allowEmpty: true })).toHaveLength(0);

await page.$eval("calcite-action-bar", (element: ActionBar["el"]) => {
element.ownerDocument.getElementById("second-action").insertAdjacentHTML(
Expand All @@ -431,8 +431,8 @@ describe("calcite-action-bar", () => {
await page.waitForTimeout(DEBOUNCE.resize + 10);
await page.waitForChanges();

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(7);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(8);
expect(await findAll(page, slottedActionsSelector)).toHaveLength(7);
});

it("should slot 'menu-actions' on sublist changes after being enabled", async () => {
Expand Down Expand Up @@ -461,16 +461,16 @@ describe("calcite-action-bar", () => {
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(0);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(8);
expect(await findAll(page, slottedActionsSelector, { allowEmpty: true })).toHaveLength(0);

const actionBar = await page.find("calcite-action-bar");
actionBar.setProperty("overflowActionsDisabled", false);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(7);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(8);
expect(await findAll(page, slottedActionsSelector)).toHaveLength(7);
});

it.skip("should slot 'menu-actions' on resize of component", async () => {
Expand All @@ -497,8 +497,8 @@ describe("calcite-action-bar", () => {
});
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(7);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(8);
expect(await findAll(page, slottedActionsSelector)).toHaveLength(7);

await page.$eval("calcite-action-bar", (element: ActionBar["el"]) => {
element.style.height = "550px";
Expand All @@ -507,8 +507,8 @@ describe("calcite-action-bar", () => {
await page.waitForTimeout(DEBOUNCE.resize + 10);
await page.waitForChanges();

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(2);
expect(await findAll(page, dynamicGroupActionsSelector)).toHaveLength(8);
expect(await findAll(page, slottedActionsSelector)).toHaveLength(2);
});
});

Expand Down Expand Up @@ -543,7 +543,7 @@ describe("calcite-action-bar", () => {
`;
await page.waitForChanges();

const groups = await page.findAll("calcite-action-group");
const groups = await findAll(page, "calcite-action-group");

for (const childGroup of groups) {
expect(await childGroup.getProperty("layout")).toBe("vertical");
Expand Down
21 changes: 10 additions & 11 deletions packages/calcite-components/src/components/action-menu/action-menu.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ import {
slots,
themed,
} from "../../tests/commonTests";
import { TOOLTIP_OPEN_DELAY_MS } from "../tooltip/resources";
import { CSS as TooltipCSS } from "../tooltip/resources";
import { isElementFocused, skipAnimations } from "../../tests/utils";
import { CSS as TooltipCSS, TOOLTIP_OPEN_DELAY_MS } from "../tooltip/resources";
import { findAll, isElementFocused, skipAnimations } from "../../tests/utils";
import type { Action } from "../action/action";
import { CSS, SLOTS, activeAttr } from "./resources";
import { activeAttr, CSS, SLOTS } from "./resources";

describe("calcite-action-menu", () => {
describe("renders", () => {
Expand Down Expand Up @@ -301,7 +300,7 @@ describe("calcite-action-menu", () => {
await page.waitForChanges();

const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");
const trigger = await page.find(`calcite-action-menu >>> .${CSS.defaultTrigger}`);

expect(await actionMenu.getProperty("open")).toBe(false);
Expand Down Expand Up @@ -341,7 +340,7 @@ describe("calcite-action-menu", () => {
await page.waitForChanges();

const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");
const trigger = await page.find(`calcite-action-menu >>> .${CSS.defaultTrigger}`);

expect(await actionMenu.getProperty("open")).toBe(false);
Expand Down Expand Up @@ -380,7 +379,7 @@ describe("calcite-action-menu", () => {
await page.waitForChanges();

const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");
const trigger = await page.find(`calcite-action-menu >>> .${CSS.defaultTrigger}`);

expect(await actionMenu.getProperty("open")).toBe(false);
Expand Down Expand Up @@ -420,7 +419,7 @@ describe("calcite-action-menu", () => {
await page.waitForChanges();

const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");
const trigger = await page.find(`calcite-action-menu >>> .${CSS.defaultTrigger}`);

expect(await actionMenu.getProperty("open")).toBe(false);
Expand Down Expand Up @@ -482,7 +481,7 @@ describe("calcite-action-menu", () => {
await skipAnimations(page);
await page.waitForChanges();
const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");

expect(await actionMenu.getProperty("open")).toBe(false);

Expand Down Expand Up @@ -514,7 +513,7 @@ describe("calcite-action-menu", () => {
await skipAnimations(page);
await page.waitForChanges();
const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");

expect(await actionMenu.getProperty("open")).toBe(false);

Expand Down Expand Up @@ -549,7 +548,7 @@ describe("calcite-action-menu", () => {
await skipAnimations(page);
await page.waitForChanges();
const actionMenu = await page.find("calcite-action-menu");
const actions = await page.findAll("calcite-action");
const actions = await findAll(page, "calcite-action");

expect(await actionMenu.getProperty("open")).toBe(false);

Expand Down
7 changes: 4 additions & 3 deletions packages/calcite-components/src/components/action-pad/action-pad.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
themed,
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { findAll } from "../../tests/utils";
import { CSS, SLOTS } from "./resources";

describe("calcite-action-pad", () => {
Expand Down Expand Up @@ -296,7 +297,7 @@ describe("calcite-action-pad", () => {

await page.waitForChanges();

let groups = await page.findAll("calcite-action-group");
let groups = await findAll(page, "calcite-action-group");

expect(await groups[0].getProperty("menuOpen")).toBe(false);
expect(await groups[1].getProperty("menuOpen")).toBe(true);
Expand All @@ -308,7 +309,7 @@ describe("calcite-action-pad", () => {

expect(eventSpy).toHaveReceivedEventTimes(2);

groups = await page.findAll("calcite-action-group");
groups = await findAll(page, "calcite-action-group");

expect(await groups[0].getProperty("menuOpen")).toBe(true);
expect(await groups[1].getProperty("menuOpen")).toBe(false);
Expand Down Expand Up @@ -355,7 +356,7 @@ describe("calcite-action-pad", () => {
`;
await page.waitForChanges();

const groups = await page.findAll("calcite-action-group");
const groups = await findAll(page, "calcite-action-group");

for (const childGroup of groups) {
expect(await childGroup.getProperty("layout")).toBe("vertical");
Expand Down
24 changes: 12 additions & 12 deletions packages/calcite-components/src/components/autocomplete/autocomplete.e2e.ts
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
import { describe, it, beforeEach, expect } from "vitest";
import { beforeEach, describe, expect, it } from "vitest";
import { E2EPage, newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
import {
accessible,
defaults,
disabled,
hidden,
floatingUIOwner,
focusable,
formAssociated,
hidden,
labelable,
openClose,
reflects,
renders,
slots,
t9n,
themed,
focusable,
slots,
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { defaultMenuPlacement } from "../../utils/floating-ui";
import { Input } from "../input/input";
import { isElementFocused, skipAnimations } from "../../tests/utils";
import { findAll, isElementFocused, skipAnimations } from "../../tests/utils";
import { CSS, SLOTS } from "./resources";
import { Autocomplete } from "./autocomplete";

Expand Down Expand Up @@ -517,7 +517,7 @@ describe("calcite-autocomplete", () => {

expect(await autocomplete.getProperty("open")).toBe(true);

const items = await page.findAll("calcite-autocomplete-item");
const items = await findAll(page, "calcite-autocomplete-item");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("active")).toBe(key === "ArrowUp" ? items.length - 2 === i : i === 0);
Expand All @@ -534,7 +534,7 @@ describe("calcite-autocomplete", () => {

expect(await autocomplete.getProperty("open")).toBe(true);

const items = await page.findAll("calcite-autocomplete-item");
const items = await findAll(page, "calcite-autocomplete-item");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("active")).toBe(false);
Expand Down Expand Up @@ -572,7 +572,7 @@ describe("calcite-autocomplete", () => {

expect(await autocomplete.getProperty("open")).toBe(true);

const items = await page.findAll("calcite-autocomplete-item");
const items = await findAll(page, "calcite-autocomplete-item");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("active")).toBe(false);
Expand Down Expand Up @@ -610,7 +610,7 @@ describe("calcite-autocomplete", () => {

expect(await autocomplete.getProperty("open")).toBe(true);

const items = await page.findAll("calcite-autocomplete-item");
const items = await findAll(page, "calcite-autocomplete-item");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("active")).toBe(false);
Expand Down Expand Up @@ -641,7 +641,7 @@ describe("calcite-autocomplete", () => {

expect(await autocomplete.getProperty("open")).toBe(true);

const items = await page.findAll("calcite-autocomplete-item");
const items = await findAll(page, "calcite-autocomplete-item");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("active")).toBe(false);
Expand Down Expand Up @@ -767,8 +767,8 @@ describe("calcite-autocomplete", () => {
const page = await newE2EPage();
await page.setContent(simpleGroupHTML);

const items = await page.findAll("calcite-autocomplete-item");
const groups = await page.findAll("calcite-autocomplete-item-group");
const items = await findAll(page, "calcite-autocomplete-item");
const groups = await findAll(page, "calcite-autocomplete-item-group");

for (let i = 0; i < items.length; i++) {
expect(await items[i].getProperty("scale")).toBe("m");
Expand Down
Loading

0 comments on commit f208961

Please sign in to comment.