Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tests] Incorrect 2nd argument to window.getComputedStyle #10369

Closed
maxpatiiuk opened this issue Sep 21, 2024 · 2 comments
Closed

[Tests] Incorrect 2nd argument to window.getComputedStyle #10369

maxpatiiuk opened this issue Sep 21, 2024 · 2 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library testing Issues related to automated or manual testing.

Comments

@maxpatiiuk
Copy link
Member

Priority impact

p - low

Test type

E2E tests

Which Component(s)

Button, Chip, ...

Unstable Tests

In several places in the code, Calcite's E2E tests call .getComputedStyle with :hover or :focus as the 2nd argument:

buttonFocusStyle = await buttonEl.getComputedStyle(":focus");
expect(buttonFocusStyle.getPropertyValue("background-color")).toEqual("rgba(0, 0, 0, 0.04)");
await buttonEl.hover();
await page.waitForChanges();
buttonHoverStyle = await buttonEl.getComputedStyle(":hover");
expect(buttonHoverStyle.getPropertyValue("background-color")).toEqual("rgba(0, 0, 0, 0.04)");
});
});
describe("when mode attribute is dark", () => {
it("should render button pseudo classes with value tied to dark mode", async () => {
page = await newE2EPage({
html: `<div class="calcite-mode-dark">${buttonSnippet}</div>`,
});
buttonEl = await page.find("calcite-button >>> button");
await buttonEl.focus();
await page.waitForChanges();
buttonFocusStyle = await buttonEl.getComputedStyle(":focus");
expect(buttonFocusStyle.getPropertyValue("background-color")).toEqual("rgba(255, 255, 255, 0.04)");
await buttonEl.hover();
await page.waitForChanges();
buttonHoverStyle = await buttonEl.getComputedStyle(":hover");
expect(buttonHoverStyle.getPropertyValue("background-color")).toEqual("rgba(255, 255, 255, 0.04)");
});
});
it("should allow the CSS custom property to be overridden", async () => {
const overrideStyle = "rgba(255, 255, 0, 0.9)";
page = await newE2EPage({
html: `
<style>
:root {
--calcite-color-transparent-hover: ${overrideStyle};
}
</style>
<div>${buttonSnippet}</div>`,
});
buttonEl = await page.find("calcite-button >>> button");
await buttonEl.focus();
await page.waitForChanges();
buttonFocusStyle = await buttonEl.getComputedStyle(":focus");
expect(buttonFocusStyle.getPropertyValue("background-color")).toEqual(overrideStyle);
await buttonEl.hover();
await page.waitForChanges();
buttonHoverStyle = await buttonEl.getComputedStyle(":hover");

As far as I am aware this is not valid. Per MDN, only ::after and ::before are allowed.
ChatGPT o1 says the same:

No, ":focus" is not a valid value for the second argument of window.getComputedStyle(). The second argument of window.getComputedStyle() is intended for pseudo-elements, not pseudo-classes. Pseudo-elements are parts of elements (like ::before, ::after), while pseudo-classes represent a state of an element (like :focus, :hover).

Test error, if applicable

While :hover and :focus appears to work fine in Chrome 119 (which is what your Stencil E2E tests are using), it does not appear to work in Chrome 128 (the latest version), causing failing tests.

You should refactor the usages in tests to not pass :hover or :focus

PR skipped, if applicable

No response

Additional Info

No response

@maxpatiiuk maxpatiiuk added testing Issues related to automated or manual testing. 0 - new New issues that need assignment. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Sep 21, 2024
@github-actions github-actions bot added the p - low Issue is non core or affecting less that 10% of people using the library label Sep 21, 2024
@jcfranco jcfranco self-assigned this Sep 23, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. and removed 0 - new New issues that need assignment. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Sep 23, 2024
@jcfranco jcfranco added this to the 2024-10-29 - Oct Milestone milestone Sep 27, 2024
@jcfranco jcfranco added the 3 - installed Issues that have been merged to master branch and are ready for final confirmation. label Sep 28, 2024
@github-actions github-actions bot removed the 2 - in development Issues that are actively being worked on. label Sep 28, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco Sep 28, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Oct 2, 2024

🍠✨ Verified

@DitwanP DitwanP closed this as completed Oct 2, 2024
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library testing Issues related to automated or manual testing.
Projects
None yet
Development

No branches or pull requests

4 participants