fixed the flaky test for logWebVitalsUtils.test.js #1
Conversation
test/utils/logWebVitalsUtils.test.js
Outdated
| window.performance.getEntriesByType = () => [ | ||
| { | ||
| startTime: 0, | ||
| name: '/test/utils/mocks/media_.png', | ||
| type: 'paint', | ||
| duration: 10, | ||
| }, | ||
| ]; | ||
|
|
||
| const parentElement = document.createElement('div'); | ||
| parentElement.id = 'parent'; | ||
|
|
||
| const lcpElement = document.createElement('div'); | ||
| lcpElement.id = 'lcpElement'; | ||
| lcpElement.textContent = 'LCP Element'; | ||
|
|
||
| parentElement.appendChild(lcpElement); | ||
| document.body.appendChild(parentElement); |
There was a problem hiding this comment.
I would assume document.body.innerHTML = await readFile({ path: './mocks/body.html' }); already handles adding the image.
If you mock the window.performance.getEntriesByType, I'd do that in a beforeEach block and "reset/restore" it in an afterEach block - otherwise you might affect other tests by building up test state and connected tests.
Since it seems you are not trying to go with this approach anymore, you can also delete those lines again.
There was a problem hiding this comment.
Noted 📓 and added the beforeEach and afterEach
test/utils/logWebVitalsUtils.test.js
Outdated
| expect(vitals).to.have.property('lcpEl'); | ||
| expect(vitals.lcpEl).to.be.a('string').that.is.not.empty; | ||
| expect(vitals).to.have.property('lcpElType'); | ||
| expect(vitals.lcpElType).to.be.oneOf(['img', 'div', 'p', 'span', '']); |
There was a problem hiding this comment.
I'd probably also just verify it has an lcpType and not neccesarily test if it's oneOf.
Reason being, if chrome or whatever browser decides it changes it algorithm or the mock on L12 is changed, this will break, despite the functionality still being correct
|
Just FYI for repo-sanity, usually u'd just call your fork |
that makes sense, i'll change it to milo 👍 |
…obecom#3549) * fixed the flaky test for logWebVitalsUtils.test.js (#1) * 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 * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * removed dependency on gist.github.com by mocking script request * replaced setTimeout to prevent flakiness * Replaced Promise.resolve().then() with setTimeout() * earlier return for if statement * Improve callback matching in script mock * improve event loop handling * Replaced IIFE with async
* fixed the flaky test for logWebVitalsUtils.test.js (#1) * 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 * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * Update fstab.yaml * enhanced tool tips Unit tests * added assertions and isolated DOM state * changed unnecessary async to sync * added loadIcons for duplicate test * tooltip should be visible on hover * added a test case for visibility of tooltip * testcase for tooltip visibility * Changes visibility test case & gave appropriate name to tests * Update test/features/icons/icons.test.js Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com> --------- Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Fixes
Specific Changes