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

Some autotests failing in Firefox #1075

Closed
msmithNI opened this issue Feb 21, 2023 · 4 comments
Closed

Some autotests failing in Firefox #1075

msmithNI opened this issue Feb 21, 2023 · 4 comments

Comments

@msmithNI
Copy link
Contributor

msmithNI commented Feb 21, 2023

🧹 Tech Debt

We currently have some test failures in Firefox. We should triage these to see if there's any user-visible impact (i.e. if they could be actual bugs in Nimble on Firefox) and fix them if possible.

Note: The drawer/dialog test failures are also present in WebKit. If we find a fix for one browser, we should test it in the other too.

To reproduce: npm run test-firefox -w @ni/nimble-components

NO LONGER FAILING:

  • TableHeader has correct state when sorted descending FAILED
    Expected null to equal 'descending'.
  • TableHeader has correct state when sorted ascending FAILED
    Expected null to equal 'ascending'.

TRACKED BY #1943

  • Drawer with default setup supports opening multiple drawers on top of each other FAILED
    Expected ... to be . Tip: To check for deep equality, use .toEqual() instead of .toBe().
  • Dialog supports opening multiple dialogs on top of each other FAILED
    Expected ... to be . Tip: To check for deep equality, use .toEqual() instead of .toBe().

TRACKED BY #1937

  • Drawer with default setup should resolve promise if drawer completely opens before being closed FAILED
    Error: Timeout - Async function did not complete within 5000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)

TRACKED BY WEBCOMPAT BUG (https://webcompat.com/issues/121098) and #1936 :

  • Drawer with default setup focuses the first button on the drawer when it opens FAILED
    Expected ... to be .... Tip: To check for deep equality, use .toEqual() instead of .toBe().
  • Drawer with default setup focuses the button with autofocus when the drawer opens FAILED
    Expected ... to be .... Tip: To check for deep equality, use .toEqual() instead of .toBe().
  • Dialog focuses the button with autofocus when the dialog opens FAILED
    Expected ... to be .... Tip: To check for deep equality, use .toEqual() instead of .toBe().
  • Dialog focuses the first button on the dialog when it opens FAILED
    Expected ... to be .... Tip: To check for deep equality, use .toEqual() instead of .toBe().

FIXED:

  • AnchorTabs should set aria-selected on active tab FAILED
    Expected undefined to be 'true'.
  • Tooltip should render the error severity when selected and render the corresponding icon when true FAILED
    Expected true to be false.
  • Tooltip should render the default state when selected FAILED
    Expected true to be false.
  • Tooltip should render the information severity when selected FAILED
    Expected true to be false.
  • Tooltip should render the information severity when selected and render the corresponding icon when true FAILED
    Expected true to be false.
  • Tooltip should render the default state when selected and not render an icon when true FAILED
    Expected true to be false.
  • Tooltip should render the error severity when selected FAILED
    Expected true to be false.
  • TableColumnText sets title when cell text is ellipsized FAILED
    Expected '' to be 'a very long value that should get ellipsized due to not fitting within the default cell width'.
@msmithNI msmithNI added tech debt triage New issue that needs to be reviewed labels Feb 21, 2023
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Feb 28, 2023
@m-akinc m-akinc moved this to Defined/Ready to Pickup in Nimble Design System Priorities Feb 28, 2023
m-akinc added a commit that referenced this issue Apr 6, 2023
## 🤨 Rationale

Relates to #1075

I fixed 7 of the 17 tests that were failing on Firefox. We want to start
running the Firefox tests on the CI (and `npm run test`), but skip the
ones known to be failing.

## 👩‍💻 Implementation

Malcolm pointed me to a package called `karma-jasmine-spec-tags` which
they had used on the WebVI team. This package allows running or skipping
tests based on arbitrary tags found in the test names. I've added
"#SkipFirefox" to the 10 remaining tests that are still failing on
Firefox. I then added `--skip-tags SkipFirefox` to the arguments passed
when executing `npm run test-firefox`. I added similar skipping support
for Chrome and Webkit.

I documented our policy for skipping tests in the CONTRIBUTING.

## 🧪 Testing

Tested that the tagged tests are indeed skipped now, and the rest are
being run on the CI.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
rajsite added a commit that referenced this issue Apr 10, 2023
# Pull Request

## 🤨 Rationale

As part of #1156 a change was made to tooltip icon tests to run in
Firefox. The implemented solution added arbitrary additional waits which
are turning out to be intermittent. This PR disables those tests until a
more robust implementation can be found. Additional investigation is
being skipped to get main stable at the moment.

Fixes #1166 (fixes the intermittency on main, and #1075 covers fixing
the actual disabled tests)

I'm also disabling the intermittent test associated with
#1172 as it was causing failues
locally trying to reproduce the intermittency of this test.

## 👩‍💻 Implementation

Revert the test changes and place them into the existing original bucket
of firefox failing tests.

## 🧪 Testing

Relying on CI

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@m-akinc
Copy link
Contributor

m-akinc commented Apr 17, 2023

All of the dialog/drawer tests that deal with the focusing of a certain control are due to a bug/compatibility issue in Firefox. I have filed an issue with webcompat. Basically, the dialog element fails to auto-focus elements if they are slotted. With a plain dialog (no slotting), it focuses just fine.

@jattasNI
Copy link
Contributor

jattasNI commented Apr 18, 2023

@m-akinc once #1188 goes in could you update the description of this issue to categorize the failures into sections like "fixed", "tracked by webcompat bug", "needs investigation", etc?

m-akinc added a commit that referenced this issue Apr 20, 2023
## 🤨 Rationale

See #1075 
Six tooltip tests listed in that bug were failing on Firefox, so we
marked them to be skipped.

## 👩‍💻 Implementation

The tests were checking for the display property of icons before the
anchored region of the tooltip had been loaded. This was resulting in
`getComputedStyle` for the icon to return an empty set of style info.
Waiting for the `loaded` event of the anchored region results in the
tests passing.

## 🧪 Testing

Ran tests locally

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@jattasNI
Copy link
Contributor

jattasNI commented Mar 7, 2024

Next action item is to take another pass at addressing the failures. Anything that's left should be broken into separate issues for each underlying cause. Those issues should be appropriately marked as blocked/upstream with bugs filed to browser vendors.

@jattasNI jattasNI added the discussion Questions, conversations, or announcements label Mar 7, 2024
@rajsite rajsite removed the discussion Questions, conversations, or announcements label Mar 7, 2024
@ni ni deleted a comment from jattasNI Mar 7, 2024
@jattasNI
Copy link
Contributor

Updated the issue description to link to specific issues for each root cause. Closing this one.

@jattasNI jattasNI closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
rajsite added a commit that referenced this issue Mar 18, 2024
# Pull Request

## 🤨 Rationale

One of the tasks in #1747 is to run nimble-components tests in WebKit
and see if there are any new failures. There are several.

We also wanted to start tracking specific issues for each root cause
rather than catch all issues like #1074 and #1075.

## 👩‍💻 Implementation

Marked each failing test with one of these specific issues:
- #1936
- #1938 TODO: ALSO FILE AZDO BUG
- #1939 
- #1940 
- #1942 
- #1943

Also re-enabled a couple of table header tests in Firefox which are now
passing.

## 🧪 Testing

Ran tests in Playwright webkit browser, Firefox, and Safari and and all
the enabled tests now pass.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
@mollykreis mollykreis moved this from Defined/Ready to Pickup to Done in Nimble Design System Priorities Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants