♻️ simplify dead code and test boundaries

clients/ember/src/index.js

@@ -20,10 +20,9 @@
  * // tests/acceptance/my-test.js
  * import { vizzlyScreenshot } from '@vizzly-testing/ember/test-support';
  *
- * test('renders correctly', async function(assert) {
+ * test('renders correctly', async function() {
  *   await visit('/');
  *   await vizzlyScreenshot('homepage');
- *   assert.ok(true);
  * });
  */
 

clients/ember/src/testem-config.js

@@ -25,6 +25,8 @@ let browserMappings = {
   webkit: 'VizzlyWebKit',
 };
 
+export { browserMappings };
+
 /**
  * Remap browser names to Vizzly launcher names
  * @param {string[]} browsers - Array of browser names
@@ -165,5 +167,3 @@ export function configure(userConfig = {}, playwrightOptions = {}) {
 
   return config;
 }
-
-export { browserMappings };

clients/ember/tests/unit/testem-config.test.js

@@ -1,14 +1,10 @@
 import assert from 'node:assert';
 import { existsSync, readFileSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
-import { afterEach, beforeEach, describe, it } from 'node:test';
+import { afterEach, describe, it } from 'node:test';
 import { browserMappings, configure } from '../../src/testem-config.js';
 
 describe('testem-config', () => {
-  beforeEach(() => {
-    // Clean up before each test
-  });
-
   afterEach(() => {
     // Clean up any playwright.json that was written
     let playwrightConfig = join(process.cwd(), '.vizzly', 'playwright.json');
@@ -17,6 +13,19 @@ describe('testem-config', () => {
     }
   });
 
+  it('preserves the public browser mappings', () => {
+    assert.deepStrictEqual(browserMappings, {
+      Chrome: 'VizzlyChrome',
+      chrome: 'VizzlyChrome',
+      Firefox: 'VizzlyFirefox',
+      firefox: 'VizzlyFirefox',
+      Safari: 'VizzlyWebKit',
+      safari: 'VizzlyWebKit',
+      WebKit: 'VizzlyWebKit',
+      webkit: 'VizzlyWebKit',
+    });
+  });
+
   describe('configure()', () => {
     it('returns config with Vizzly launchers when given empty object', () => {
       let result = configure({});
@@ -181,26 +190,4 @@ describe('testem-config', () => {
       assert.deepStrictEqual(input, inputCopy, 'input should not be mutated');
     });
   });
-
-  describe('browserMappings', () => {
-    it('maps Chrome to VizzlyChrome', () => {
-      assert.strictEqual(browserMappings.Chrome, 'VizzlyChrome');
-    });
-
-    it('maps chrome (lowercase) to VizzlyChrome', () => {
-      assert.strictEqual(browserMappings.chrome, 'VizzlyChrome');
-    });
-
-    it('maps Firefox to VizzlyFirefox', () => {
-      assert.strictEqual(browserMappings.Firefox, 'VizzlyFirefox');
-    });
-
-    it('maps Safari to VizzlyWebKit', () => {
-      assert.strictEqual(browserMappings.Safari, 'VizzlyWebKit');
-    });
-
-    it('maps WebKit to VizzlyWebKit', () => {
-      assert.strictEqual(browserMappings.WebKit, 'VizzlyWebKit');
-    });
-  });
 });

clients/static-site/src/screenshot.js

@@ -15,11 +15,6 @@ try {
   vizzlyScreenshot = async () => {};
 }
 
-/** @internal Replace vizzlyScreenshot for testing */
-export function _setVizzlyScreenshot(fn) {
-  vizzlyScreenshot = fn;
-}
-
 /**
  * Generate screenshot name from page path
  * Viewport info goes in properties for grouping
@@ -116,14 +111,18 @@ export async function captureScreenshot(page, options = {}) {
  * @param {Object} pageObj - Page object
  * @param {Object} viewport - Viewport object
  * @param {Object} screenshotOptions - Screenshot options
+ * @param {Object} deps - External dependencies
+ * @param {Function} deps.vizzlyScreenshot - Screenshot transport
  * @returns {Promise<void>}
  */
 export async function captureAndSendScreenshot(
   page,
   pageObj,
   viewport,
-  screenshotOptions = {}
+  screenshotOptions = {},
+  deps = {}
 ) {
+  let sendScreenshot = deps.vizzlyScreenshot || vizzlyScreenshot;
   let name = generateScreenshotName(pageObj);
   let properties = generateScreenshotProperties(viewport, {
     ...screenshotOptions,
@@ -136,5 +135,5 @@ export async function captureAndSendScreenshot(
     vizzlyOptions.requestTimeout = screenshotOptions.requestTimeout;
   }
 
-  await vizzlyScreenshot(name, screenshot, vizzlyOptions);
+  await sendScreenshot(name, screenshot, vizzlyOptions);
 }

clients/static-site/tests/concurrency.test.js

@@ -42,15 +42,6 @@ describe('mapWithConcurrency', () => {
     assert.ok(maxConcurrent <= 2);
   });
 
-  it('handles async function results', async () => {
-    let items = [1, 2, 3];
-
-    await mapWithConcurrency(items, async item => item * 2, 2);
-
-    // Should complete without error
-    assert.ok(true);
-  });
-
   it('handles errors in processing', async () => {
     let items = [1, 2, 3];
 

clients/static-site/tests/pool.test.js

@@ -33,35 +33,20 @@ function createMockContext(id) {
  * Create a mock browser for testing (Playwright style)
  */
 function createMockBrowser() {
-  let contextCount = 0;
-  let newContextCalls = 0;
+  let contexts = [];
 
   return {
     newContext: mock.fn(async () => {
-      contextCount++;
-      newContextCalls++;
-      return createMockContext(contextCount);
+      let context = createMockContext(contexts.length + 1);
+      contexts.push(context);
+      return context;
     }),
-    getPageCount: () => contextCount,
-    getNewPageCalls: () => newContextCalls,
+    getContextCount: () => contexts.length,
+    getContexts: () => contexts,
   };
 }
 
 describe('createTabPool', () => {
-  it('creates a pool with acquire, release, drain, and stats', () => {
-    let browser = createMockBrowser();
-    let pool = createTabPool(browser, 3);
-
-    assert.ok(pool.acquire);
-    assert.ok(pool.release);
-    assert.ok(pool.drain);
-    assert.ok(pool.stats);
-    assert.strictEqual(typeof pool.acquire, 'function');
-    assert.strictEqual(typeof pool.release, 'function');
-    assert.strictEqual(typeof pool.drain, 'function');
-    assert.strictEqual(typeof pool.stats, 'function');
-  });
-
   it('reports correct initial stats', () => {
     let browser = createMockBrowser();
     let pool = createTabPool(browser, 5);
@@ -80,7 +65,7 @@ describe('createTabPool', () => {
 
       let tab = await pool.acquire();
 
-      assert.strictEqual(browser.getNewPageCalls(), 1);
+      assert.strictEqual(browser.getContextCount(), 1);
       assert.strictEqual(tab.id, 1);
       assert.strictEqual(pool.stats().total, 1);
     });
@@ -93,7 +78,7 @@ describe('createTabPool', () => {
       let tab2 = await pool.acquire();
       let tab3 = await pool.acquire();
 
-      assert.strictEqual(browser.getNewPageCalls(), 3);
+      assert.strictEqual(browser.getContextCount(), 3);
       assert.strictEqual(tab1.id, 1);
       assert.strictEqual(tab2.id, 2);
       assert.strictEqual(tab3.id, 3);
@@ -109,7 +94,7 @@ describe('createTabPool', () => {
 
       let tab2 = await pool.acquire();
 
-      assert.strictEqual(browser.getNewPageCalls(), 1);
+      assert.strictEqual(browser.getContextCount(), 1);
       assert.strictEqual(tab2, tab1);
     });
 
@@ -221,9 +206,9 @@ describe('createTabPool', () => {
 
       await pool.drain();
 
-      // Context close is called, not page close
-      assert.strictEqual(page1._poolEntry.context.close.mock.callCount(), 1);
-      assert.strictEqual(page2._poolEntry.context.close.mock.callCount(), 1);
+      let contexts = browser.getContexts();
+      assert.strictEqual(contexts[0].close.mock.callCount(), 1);
+      assert.strictEqual(contexts[1].close.mock.callCount(), 1);
       assert.strictEqual(pool.stats().available, 0);
       assert.strictEqual(pool.stats().total, 0);
     });
@@ -252,7 +237,8 @@ describe('createTabPool', () => {
       let page2 = await pool.acquire();
 
       // Make first context throw on close
-      page1._poolEntry.context.close = mock.fn(async () => {
+      let contexts = browser.getContexts();
+      contexts[0].close = mock.fn(async () => {
         throw new Error('Close failed');
       });
 
@@ -262,8 +248,8 @@ describe('createTabPool', () => {
       // Should not throw
       await pool.drain();
 
-      assert.strictEqual(page1._poolEntry.context.close.mock.callCount(), 1);
-      assert.strictEqual(page2._poolEntry.context.close.mock.callCount(), 1);
+      assert.strictEqual(contexts[0].close.mock.callCount(), 1);
+      assert.strictEqual(contexts[1].close.mock.callCount(), 1);
     });
   });
 
@@ -285,7 +271,7 @@ describe('createTabPool', () => {
 
       assert.strictEqual(results.length, 10);
       // Should have created at most 3 tabs
-      assert.ok(browser.getPageCount() <= 3);
+      assert.ok(browser.getContextCount() <= 3);
     });
   });
 
@@ -297,7 +283,7 @@ describe('createTabPool', () => {
       // First tab created
       let tab1 = await pool.acquire();
       let originalId = tab1.id;
-      assert.strictEqual(browser.getNewPageCalls(), 1);
+      assert.strictEqual(browser.getContextCount(), 1);
 
       // Use 1
       await pool.release(tab1);
@@ -315,7 +301,7 @@ describe('createTabPool', () => {
       // Now acquire should get a fresh tab (recycled)
       let tab4 = await pool.acquire();
       assert.notStrictEqual(tab4.id, originalId);
-      assert.strictEqual(browser.getNewPageCalls(), 2);
+      assert.strictEqual(browser.getContextCount(), 2);
     });
 
     it('tracks recycled count in stats', async () => {
@@ -350,11 +336,12 @@ describe('createTabPool', () => {
       await pool.release(page); // use 1
 
       page = await pool.acquire();
-      assert.strictEqual(page._poolEntry.context.close.mock.callCount(), 0);
+      let [context] = browser.getContexts();
+      assert.strictEqual(context.close.mock.callCount(), 0);
 
       await pool.release(page); // use 2 - triggers recycle
 
-      assert.strictEqual(page._poolEntry.context.close.mock.callCount(), 1);
+      assert.strictEqual(context.close.mock.callCount(), 1);
     });
 
     it('hands off fresh tab to waiting acquirer during recycling', async () => {
@@ -408,70 +395,19 @@ describe('createTabPool', () => {
       let pool = createTabPool(browser, 1, { recycleAfter: 2 });
 
       let page = await pool.acquire();
-      page._poolEntry.context.close = mock.fn(async () => {
+      let [context] = browser.getContexts();
+      context.close = mock.fn(async () => {
         throw new Error('Close failed');
       });
 
       await pool.release(page); // use 1
 
       page = await pool.acquire();
-      page._poolEntry.context.close = mock.fn(async () => {
-        throw new Error('Close failed');
-      });
 
       // Should not throw despite close error
       await pool.release(page); // use 2 - triggers recycle
 
       assert.strictEqual(pool.stats().recycled, 1);
     });
   });
-
-  describe('_poolEntry metadata', () => {
-    it('preserves _poolEntry reference on page', async () => {
-      let browser = createMockBrowser();
-      let pool = createTabPool(browser, 2);
-
-      let page = await pool.acquire();
-
-      assert.ok(page._poolEntry);
-      assert.strictEqual(page._poolEntry.page, page);
-      assert.strictEqual(page._poolEntry.useCount, 1);
-    });
-
-    it('increments useCount on each acquire', async () => {
-      let browser = createMockBrowser();
-      let pool = createTabPool(browser, 1);
-
-      let tab = await pool.acquire();
-      assert.strictEqual(tab._poolEntry.useCount, 1);
-
-      await pool.release(tab);
-
-      tab = await pool.acquire();
-      assert.strictEqual(tab._poolEntry.useCount, 2);
-
-      await pool.release(tab);
-
-      tab = await pool.acquire();
-      assert.strictEqual(tab._poolEntry.useCount, 3);
-    });
-
-    it('increments useCount when handing off to waiter', async () => {
-      let browser = createMockBrowser();
-      let pool = createTabPool(browser, 1);
-
-      let tab = await pool.acquire();
-      assert.strictEqual(tab._poolEntry.useCount, 1);
-
-      // Someone is waiting
-      let acquirePromise = pool.acquire();
-
-      // Release hands off directly to waiter
-      pool.release(tab);
-
-      let sameTab = await acquirePromise;
-      assert.strictEqual(sameTab, tab);
-      assert.strictEqual(tab._poolEntry.useCount, 2);
-    });
-  });
 });