MWPW-191476-test cases fixed for firefly-share#71
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
| copyButton.click(); | ||
| // wait a microtask so the promise rejection is observed by the handler | ||
| await Promise.resolve(); | ||
|
|
There was a problem hiding this comment.
The source (firefly-share.js:217) calls writeText(...).then(...) with no .catch(). When this stub rejects, it creates an unhandled promise rejection — the test assertions are correct, but depending on the test runner config (--fail-on-rejection), this could silently swallow the error or break CI. Consider adding .catch(() => {}) to the source chain in a follow-up.
| <div> | ||
| <p><a href="https://www.twitter.com/intent/tweet">twitter</a></p> | ||
| <p><a href="https://www.facebook.com/sharer.php">facebook</a></p> | ||
| <div><a href="https://x.bar.com/share">unknown host</a></div> |
There was a problem hiding this comment.
This fixture exercises two things: (1) an unrecognized platform ("bar" from x.bar.com) that getDetails silently drops, and (2) a non-<p> wrapper so share.closest("p")?.remove() no-ops. Neither behavior is explicitly asserted in the test — the count check (length === 2) covers it only implicitly.
| // assert window.open was called with width parameter | ||
| expect(openStub.firstCall.args[2]).to.include('width=600'); | ||
| } finally { | ||
| openStub.restore(); |
There was a problem hiding this comment.
This assertion proves window.open() is exercised, which the source marks /* c8 ignore next 2 */ at line 167. Similarly, the clipboard tests above cover lines 214-220 (also c8 ignore). Consider removing those stale ignore comments in a follow-up so coverage reflects reality.
| writeTextStub.restore(); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
The source toggles a changeText flag each click, appending (zero-width space) on alternate copies to force screen readers to re-announce the aria-live region. A second click → flush → assert sequence here would verify the toggle: first copy text should lack , second should include it.
|
Nit (nice-to-have): Some tests use fixtures from |
3 participants
Resolves: MWPW-191476
Test URLs:
Dev Validation :
http://127.0.0.1:5502/coverage/lcov-report/blocks/firefly-share/index.html