Mwpw 162709 tooltip unit tests #3612
Conversation
* fixed the flaky test * move performance mock to beforeEach/afterEach for isolation, simplify lcpElType check * removed unused code * fixed the same issue in logWebVitals.test.js and reverted unnecessary changes
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3612 +/- ##
==========================================
+ Coverage 96.53% 98.39% +1.86%
==========================================
Files 274 79 -195
Lines 61849 10149 -51700
==========================================
- Hits 59705 9986 -49719
+ Misses 2144 163 -1981 ☔ View full report in Codecov by Sentry. |
|
Oh lucky you, seems like you get to also investigate a flaky test 😁
|
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
skholkhojaev
force-pushed
the
MWPW-162709-tooltip-UnitTests
branch
from
b64b25c to
ad3f9d4
Compare
| icons = document.querySelectorAll('span.icon'); | ||
| await loadIcons(icons, config); | ||
| await loadIcons(icons, config); // Test duplicate icon not created if run twice | ||
| }); | ||
|
|
||
| it('Fetches successfully with cache control enabled', async () => { |
There was a problem hiding this comment.
Is the test name appropiate? How does cache control play in here
There was a problem hiding this comment.
i dont think so but i wasnt sure about that becasue i wasnt sure what the dev that originally wrote that test meant with that, but i will change it
test/features/icons/icons.test.js
Outdated
| it('Replaces span.icon', () => { | ||
| const selector = icons[0].querySelector(':scope svg'); | ||
| expect(selector).to.exist; | ||
| }); |
There was a problem hiding this comment.
Let's rename this to Renders an SVG after loading the icons
test/features/icons/icons.test.js
Outdated
| expect(tooltip.getAttribute('role')).to.equal('button'); | ||
| expect(tooltip.className).to.contain('top'); | ||
| tooltip.focus(); | ||
| expect(document.activeElement).to.equal(tooltip); |
There was a problem hiding this comment.
I know we had this on Slack, but is there no ward to check if the if the tooltip is visible or at least something more like this? https://stackoverflow.com/questions/29084253/how-to-unit-test-pseudo-elements
Verifying the existance of the pseudo class is likely a bit better, since the element would still be active even if someone removed the CSS class and the tooltip would not show anymore.
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Discription
This PR enhances the unit tests for the tooltip component
Changes
icons.test.jsResolves: MWPW-162709
Test URLs: