MWPW-191474-Firefly-caraousal test cases by hkhatana26 · Pull Request #70 · adobecom/da-cc

hkhatana26 · 2026-05-18T11:59:55Z

Resolves: MWPW-191474

Test URLs:

Dev Validation-
http://127.0.0.1:5502/coverage/lcov-report/blocks/firefly-carousel/index.html

aem-code-sync · 2026-05-18T11:59:58Z

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

Re-run all PSI checks
Re-run failed PSI checks
Re-sync branch

Commits

446d14a 🔄 (latest)
2499b7a ✅
2f4a7a3 ✅

aem-code-sync · 2026-05-18T12:29:07Z

	Page	Scores	Audits	Google
📱	/		Lighthouse returned error: NOT_HTML. The page provided is not HTML (served as MIME type text/plain).
🖥️	/		Lighthouse returned error: NOT_HTML. The page provided is not HTML (served as MIME type text/plain).

nkthakur48 · 2026-06-17T03:52:27Z

+    });
+
+    it('handles carousel with exactly 2 items', async () => {
+      const block = body.querySelector('#carousel-basic');


This uses #carousel-basic which has 4 items, not 2. The test name says "exactly 2 items" but never exercises the 2-item boundary. Use #carousel-mixed-media (which has 2 items) or add a dedicated #carousel-two-items fixture. This matters because createMoveHandler gates on itemCount <= 1 — the 2-item case is the minimum for navigation and circular ordering with only 2 cards is a known edge case.

nkthakur48 · 2026-06-17T03:55:19Z

+        const handler = sinon.spy();
+        viewport.addEventListener('mouseenter', handler);
+        viewport.dispatchEvent(new Event('mouseenter'));
+        expect(handler.called).to.be.true;


This test only proves dispatchEvent triggers a listener you added yourself — it does not test carousel behavior. The actual auto-scroll stop handler is registered by setupAutoScroll with { once: true }. To test it properly: mock IntersectionObserver to start auto-scroll, verify the interval is active, then dispatch mouseenter and assert no further slides advance.

nkthakur48 · 2026-06-17T03:55:50Z

+      expect(cards.length).to.be.greaterThan(0);
+    });
+
+    it('first card exists after initialization', async () => {


5 duplicate tests that inflate the count without adding coverage:

Duplicate Identical to

"first card exists after initialization" (L47) "converts carousel items to firefly-carousel-cards" (L38)

"gracefully handles carousel with no items" (L400) "handles empty carousel gracefully" (L56)

"initializes all carousel features without errors" (L674) "carousel initializes without errors" (L65)

"applies aria-label to navigation buttons" (L267) "navigation buttons are properly labeled" (L169)

"renders without errors" (L390) "carousel initializes without errors" (L65)

~41 lines of pure duplication. Remove these and invest the effort in the uncovered branches (touch swipe, IntersectionObserver auto-scroll, prompt pill click → window.open, parseSlideItem null paths).

nkthakur48 · 2026-06-17T03:56:46Z

+
+      const videoContainer = block.querySelector('.video-container');
+      if (videoContainer) {
+        expect(videoContainer).to.exist;


if (x) expect(x).to.exist is a no-op — the assertion only runs when it's guaranteed to pass. This pattern appears in 3 tests:

Here (L669): if (videoContainer) { expect(videoContainer).to.exist; }

L278: if (nextBtn) { expect(nextBtn.title).to.exist; }

L289: if (pills.length > 0) { expect(firstPill.getAttribute('aria-label')).to.exist; }

If the element is expected, assert unconditionally — let the test fail when it's missing. If it genuinely might not exist, the test verifies nothing and should be removed.

nkthakur48 · 2026-06-17T03:58:31Z

+      clock.tick(100);
+
+      const cards = block.querySelectorAll('.firefly-carousel-card');
+      expect(cards.length).to.be.greaterThan(0);


greaterThan(0) is too weak — #carousel-basic has exactly 4 items, so use .to.equal(4). This same pattern appears 14+ times across the file. With greaterThan(0), a regression that silently drops 3 of 4 cards still passes. Use exact counts wherever the fixture is known.

nkthakur48 · 2026-06-17T03:59:25Z

+    });
+
+    it('handles video source attributes in video slides', async () => {
+      const block = body.querySelector('#carousel-with-videos');


Silent early return — if the fixture is missing, this test passes without running any assertions. Since you authored the mock HTML and #carousel-with-videos exists in it, replace with expect(block).to.exist; so a broken fixture fails loudly. Same pattern at L104, L159, L318, L328, L663.

nkthakur48 · 2026-06-17T04:00:46Z

+      await init(block);
+      clock.tick(100);
+
+      const dir = block.getAttribute('dir') || document.documentElement.getAttribute('dir');


This doesn't test RTL behavior. The source computes RTL via document.dir === 'rtl' (not a dir attribute on the block element) — dir shouldn't be checked on element DOM, only page DOM. This test just asserts the mock's own attribute exists, which always passes.

Fix: Set document.dir = 'rtl' before init, then verify the track transform uses a negative axis multiplier (i.e. translate3d offset is positive instead of negative). Reset document.dir in cleanup.

nkthakur48 · 2026-06-17T04:01:35Z

+      expect(viewport).to.exist;
+      expect(track).to.exist;
+
+      window.dispatchEvent(new Event('resize'));


The source uses ResizeObserver on the viewport (see observeResize), not a window resize listener — this event doesn't trigger the actual responsive code path. The assertion just re-checks that the viewport still exists, which it always will.

Fix: Mock ResizeObserver and trigger the callback manually, then verify controls.reposition() recalculates the track transform. Or resize the viewport element's dimensions and flush the observer.

nkthakur48 · 2026-06-17T04:02:00Z

+    });
+  });
+
+  describe('Branch Coverage - Enhanced Tests', () => {


Despite the name, several important source branches remain uncovered:

Source path Gap

setupSwipe (touch events) No touch tests at all

setupAutoScroll (IntersectionObserver) No IO mock — only verified setInterval was called

parseSlideItem null returns Not tested for < 3 children, missing media, or missing deeplink

createPromptPill click → window.open Not tested

ensureVideoSource setting src from dataset Not tested

waitForTrackTransition fallback timer vs transitionend Not tested

createMoveHandler returning false (itemCount ≤ 1) Single-slide test early-returns before asserting

setupArrowKeyNavigation focus guard (!el.contains(activeElement)) Not tested

setMediaControlTabOrder with .pause-play-wrapper Mock has no pause-play-wrapper elements

These are the branches that actually catch regressions. Consider replacing the duplicate/tautological tests with coverage for these paths.

nkthakur48 · 2026-06-17T04:02:21Z

+
+    it('carousel initializes without errors', async () => {
+      const block = body.querySelector('#carousel-basic');
+      expect(() => {


init is async — wrapping it in a sync callback means any rejection becomes an unhandled promise rejection, not a synchronous throw. This assertion would pass even if init rejected. Happens to work here because init doesn't actually await anything internally, but it's conceptually wrong.

Fix: Just await init(block) directly — if it throws, Mocha catches it and fails the test. No expect().not.to.throw() wrapper needed.

nkthakur48 · 2026-06-17T04:04:03Z

+      await init(block);
+
+      const track = block.querySelector('.firefly-carousel-track');
+      let errorThrown = false;


Manual try/catch reimplements what expect(...).not.to.throw() already does, but loses the error message on failure. And since init is async, a rejected promise wouldn't be caught here anyway — it'd become an unhandled rejection and the test would still pass.

Fix: Remove the try/catch scaffolding. Just await init(block) and run the assertions directly — WTR/Mocha will fail the test if anything throws.

nkthakur48 · 2026-06-17T04:07:58Z

@hkhatana26 Please check why the UT github action is failing?

aem-code-sync Bot temporarily deployed to mwpw-191474-ffcaraousal-testcases May 21, 2026 18:54 Inactive

aem-code-sync Bot temporarily deployed to mwpw-191474-ffcaraousal-testcases June 1, 2026 09:02 Inactive

hkhatana26 requested review from hkuraware, nkthakur48 and suhjainadobe June 4, 2026 11:51

aem-code-sync Bot temporarily deployed to mwpw-191474-ffcaraousal-testcases June 8, 2026 05:41 Inactive