refactor: prepare tests for migration #10509
Conversation
github-actions
bot
added
the
refactor
| // initialize page with calcite-color-picker to make it available in the evaluate callback below | ||
| const page = await newE2EPage({ | ||
| html: "<calcite-color-picker></calcite-color-picker>", | ||
| html: "", | ||
| }); | ||
| await page.setContent(""); |
There was a problem hiding this comment.
This seems wrong.
The html option of newE2EPage just calls setContent behind the scenes.
Each time setContent is called, the page is re-created from scratch - there is no information carried over in between.
|
|
||
| await page.evaluate(async (color) => { | ||
| const picker = document.createElement("calcite-color-picker"); | ||
| picker.value = color; | ||
| document.body.append(picker); | ||
|
|
||
| await new Promise<void>((resolve) => requestAnimationFrame(() => resolve())); | ||
| await picker.componentOnReady(); |
There was a problem hiding this comment.
componentOnReady is more suited for this
| @@ -104,7 +104,6 @@ export class Loader implements LocalizedComponent { | |||
| [CSS.loaderPart]: true, | |||
| [CSS.loaderPartId(index)]: true, | |||
| }} | |||
| key={index} | |||
There was a problem hiding this comment.
key={index} seems useless?
Lumina has a warning about using index as a key (it suggests not using the key at all in those cases)
| @@ -22,7 +22,7 @@ describe("calcite-navigation-user", () => { | |||
| }, | |||
| { | |||
| propertyName: "textDisabled", | |||
| value: "", | |||
| value: true, | |||
There was a problem hiding this comment.
In Stencil, casting is done both to property and attribute (thus setting boolean property to "" will cast to true). In Lit, casting is only done when converting attribute to property or property to attribute.
| @@ -15,7 +15,7 @@ describe("calcite-navigation", () => { | |||
| reflects("calcite-navigation", [ | |||
| { | |||
| propertyName: "navigationAction", | |||
| value: "", | |||
| value: true, | |||
| @@ -9,7 +9,7 @@ describe("calcite-pick-list-item", () => { | |||
| }); | |||
|
|
|||
| describe("honors hidden attribute", () => { | |||
| hidden("calcite-list-item"); | |||
| hidden("calcite-pick-list-item"); | |||
| @@ -399,7 +399,7 @@ describe("calcite-rating", () => { | |||
| expect(labels[4]).not.toHaveClass("partial"); | |||
| }); | |||
|
|
|||
| it("should update the ui of the rating when a hover event triggers on a rating label", async () => { | |||
| it("should update the UI of the rating when a hover event triggers on a rating label", async () => { | |||
There was a problem hiding this comment.
consistency with usages in previous test titles
| @@ -118,7 +118,7 @@ describe("calcite-shell-panel", () => { | |||
| "calcite-shell-panel", | |||
| (panel: HTMLCalciteShellPanelElement, containerClass: string, contentClass: string) => { | |||
| const container = panel.shadowRoot.querySelector(containerClass); | |||
| return container.firstElementChild.className == contentClass; | |||
| return container.firstElementChild.classList.contains(contentClass); | |||
There was a problem hiding this comment.
In Lumina, when class={} prop is an object, the attribute is set to not just "value" but to " value " (notice the spaces.
Adjusting this to use classList.contains to have the code work both pre and post migration
| @@ -290,7 +290,7 @@ function invalidHandler(event: Event) { | |||
| formComponent.status = "idle"; | |||
| } | |||
|
|
|||
| if ("validationIcon" in formComponent && !formComponent.validationIcon) { | |||
| if ("validationIcon" in formComponent) { | |||
There was a problem hiding this comment.
Unless I am wrong, it seems like this condition should not have been here and it worked in Stencil accidentally:
- Since validationIcon prop is of mixed type (string | boolean), setting it to empty string does not automatically cast to
true - You set validationIcon prop to true here on form validation error:
- SInce this prop is reflected, stencil reflects it to empty string attribute:
- Reflection of the attribute triggers attributeChangedCallback. Stencil sees that the attribute value
""does not match the property valuetrue, and sets the property to"". - When it comes to clear the validation message, this place does the
&& !formComponent.validationIconcheck. That worked because""is falsy.- However, this condition does not work in Lit because Lit ignores attributeChangedCallback when it does the reflection of a property to an attribute
Make minor adjustments to Calcite code to have post-migration tests pass.
Changes that need to be done post-migration are encoded in the following codemod files in the arcgis-web-components repository:
At this point, I am finishing up the failing tests in the S components and will be pretty much done with all A-S component (with exception of #10394 and #10495)