Merge branch 'steven/cypress-one-watcher-per-test' into steven/cypres…

…s-network-idle

.gitignore

@@ -22,4 +22,5 @@ test-archives
 **/playwright/.cache/
 packages/playwright/test-results/
 packages/cypress/tests/cypress/downloads/
+packages/cypress/tests/cypress/test-downloads/
 packages/*/coverage/

package.json

@@ -12,8 +12,8 @@
   "scripts": {
     "archive-storybook:playwright": "CHROMATIC_ARCHIVE_LOCATION=packages/playwright/test-results packages/playwright/dist/bin/archive-storybook.js",
     "build-archive-storybook:playwright": "CHROMATIC_ARCHIVE_LOCATION=packages/playwright/test-results packages/playwright/dist/bin/build-archive-storybook.js",
-    "archive-storybook:cypress": "CHROMATIC_ARCHIVE_LOCATION=packages/cypress/tests/cypress/downloads packages/cypress/dist/bin/archive-storybook.js",
-    "build-archive-storybook:cypress": "CHROMATIC_ARCHIVE_LOCATION=packages/cypress/tests/cypress/downloads packages/cypress/dist/bin/build-archive-storybook.js",
+    "archive-storybook:cypress": "CHROMATIC_ARCHIVE_LOCATION=packages/cypress/tests/cypress/test-downloads packages/cypress/dist/bin/archive-storybook.js",
+    "build-archive-storybook:cypress": "CHROMATIC_ARCHIVE_LOCATION=packages/cypress/tests/cypress/test-downloads packages/cypress/dist/bin/build-archive-storybook.js",
     "clean": "yarn workspaces foreach --all --parallel run clean",
     "prebuild": "yarn clean",
     "build": "yarn workspaces foreach --all --topological-dev --parallel run build",

packages/cypress/CHANGELOG.md

@@ -1,5 +1,12 @@
 # chromatic-cypress
 
+## 0.6.15
+
+### Patch Changes
+
+- 6144340: - Use the configured `downloadsFolder` in Cypress as the output dir for archives
+  - Move Playwright-related path logic out of the shared package into the Playwright package
+
 ## 0.6.14
 
 ### Patch Changes

packages/cypress/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@chromatic-com/cypress",
-  "version": "0.6.14",
+  "version": "0.6.15",
   "description": "Chromatic Visual Regression Testing for Cypress",
   "repository": {
     "type": "git",

packages/cypress/src/index.ts

@@ -21,6 +21,7 @@ interface WriteParams {
   chromaticStorybookParams: ChromaticStorybookParameters;
   pageUrl: string;
   viewport: Viewport;
+  outputDir: string;
 }
 
 interface WriteArchivesParams extends WriteParams {
@@ -34,6 +35,7 @@ const writeArchives = async ({
   chromaticStorybookParams,
   pageUrl,
   viewport,
+  outputDir,
 }: WriteArchivesParams) => {
   const allSnapshots = Object.fromEntries(
     // manual snapshots can be given a name; otherwise, just use the snapshot's place in line as the name
@@ -46,9 +48,7 @@ const writeArchives = async ({
   await writeTestResult(
     {
       titlePath: testTitlePath,
-      // this will store it at ./cypress/downloads (the last directory doesn't matter)
-      // TODO: change so we don't have to do this trickery
-      outputDir: './cypress/downloads/some',
+      outputDir,
       pageUrl,
       viewport,
     },

packages/cypress/src/network-idle-watcher.ts

@@ -1,5 +1,8 @@
 const TOTAL_TIMEOUT_DURATION = 3000;
 
+// A Cypress equivalent of Playwright's `page.waitForLoadState()` (https://playwright.dev/docs/api/class-page#page-wait-for-load-state).
+// Intentionally simplistic since in Cypress this is just used to make sure there aren't any pending requests hanging around
+// after the test has finished.
 export class NetworkIdleWatcher {
   private numInFlightRequests = 0;
 
@@ -16,7 +19,7 @@ export class NetworkIdleWatcher {
           reject(new Error('some responses have not returned'));
         }, TOTAL_TIMEOUT_DURATION);
 
-        // assign a function that resolves... and it can be used...
+        // assign a function that'll be called as soon as responses are all back
         this.exitIdleOnResponse = () => {
           resolve(true);
         };
@@ -30,7 +33,7 @@ export class NetworkIdleWatcher {
 
   onResponse() {
     this.numInFlightRequests -= 1;
-    // resolve if the in-flight request amount is now zero
+    // resolve immediately if the in-flight request amount is now zero
     if (this.numInFlightRequests === 0) {
       clearTimeout(this.idleTimer);
       this.exitIdleOnResponse?.();

packages/cypress/src/support.ts

@@ -5,10 +5,6 @@ const generateTestId = () => {
   return Cypress.currentTest.title;
 };
 
-const shouldTakeSnapshot = () => {
-  return !Cypress.env('disableAutoSnapshot') && !Cypress.config('isInteractive');
-};
-
 // these client-side lifecycle hooks will be added to the user's Cypress suite
 beforeEach(() => {
   // don't take snapshots when running `cypress open`
@@ -69,6 +65,7 @@ afterEach(() => {
               height: Cypress.config('viewportHeight'),
               width: Cypress.config('viewportWidth'),
             },
+            outputDir: Cypress.config('downloadsFolder'),
           },
         });
       });

packages/cypress/tests/cypress.config.ts

@@ -1,14 +1,23 @@
 import { defineConfig } from 'cypress';
 import { installPlugin } from '../dist';
+import { existsSync } from 'node:fs';
 
 export default defineConfig({
+  // `downloadsFolder` cannot be overridden in tests, so we're setting
+  // this to a non-default value for asserting in the tests
+  downloadsFolder: 'cypress/test-downloads',
   // needed since we use common mock images between Cypress and Playwright
   fixturesFolder: '../../../test-server/fixtures',
   screenshotOnRunFailure: false,
   e2e: {
     baseUrl: 'http://localhost:3000',
     setupNodeEvents(on, config) {
       installPlugin(on, config);
+      on('task', {
+        directoryExists(directoryName) {
+          return existsSync(directoryName);
+        },
+      });
     },
   },
 });

packages/cypress/tests/cypress/e2e/custom-downloads-directory.cy.ts

@@ -0,0 +1,13 @@
+// Snapshots are disabled because we're asserting on file system concerns,
+// not testing anything visual
+it(
+  'downloads archives to the user-specified folder',
+  { env: { disableAutoSnapshot: true } },
+  () => {
+    cy.visit('/asset-paths/query-params');
+    const chromaticArchivesDir = `${Cypress.config('downloadsFolder')}/chromatic-archives`;
+    cy.task('directoryExists', chromaticArchivesDir).then((dirExists) => {
+      expect(dirExists).to.be.true;
+    });
+  }
+);

packages/playwright/CHANGELOG.md

@@ -1,5 +1,12 @@
 # chromatic-playwright
 
+## 0.6.16
+
+### Patch Changes
+
+- 6144340: - Use the configured `downloadsFolder` in Cypress as the output dir for archives
+  - Move Playwright-related path logic out of the shared package into the Playwright package
+
 ## 0.6.15
 
 ### Patch Changes

packages/playwright/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@chromatic-com/playwright",
-  "version": "0.6.15",
+  "version": "0.6.16",
   "description": "Chromatic Visual Regression Testing for Playwright",
   "repository": {
     "type": "git",

packages/playwright/src/makeTest.ts

@@ -14,6 +14,7 @@ import {
   trackRun,
   DEFAULT_GLOBAL_RESOURCE_ARCHIVE_TIMEOUT_MS,
 } from '@chromatic-com/shared-e2e';
+import { join } from 'node:path';
 import { chromaticSnapshots, takeSnapshot } from './takeSnapshot';
 import { createResourceArchive } from './createResourceArchive';
 
@@ -72,8 +73,11 @@ export const performChromaticSnapshot = async (
       ...(cropToViewport && { cropToViewport }),
     };
 
+    // TestInfo.outputDir gives us the test-specific subfolder (https://playwright.dev/docs/api/class-testconfig#test-config-output-dir);
+    // we want to write one level above that
+    const outputDir = join(testInfo.outputDir, '..');
     await writeTestResult(
-      { ...testInfo, pageUrl: page.url(), viewport: page.viewportSize() },
+      { ...testInfo, outputDir, pageUrl: page.url(), viewport: page.viewportSize() },
       Object.fromEntries(snapshots),
       resourceArchive,
       chromaticStorybookParams

packages/shared/src/resource-archiver/index.ts

@@ -56,6 +56,7 @@ export class ResourceArchiver {
   }
 
   async close() {
+    // Playwright's client uses detach(), Cypress' uses close()
     if (this.client.close) {
       await this.client.close();
     } else if (this.client.detach) {

packages/shared/src/write-archive/index.test.ts

@@ -34,7 +34,7 @@ describe('writeTestResult', () => {
       // the default output directory in playwright
       {
         titlePath: ['file.spec.ts', 'Test Story'],
-        outputDir: resolve('test-results/test-story-chromium'),
+        outputDir: resolve('test-results'),
         pageUrl: 'http://localhost:3000/',
         viewport: { height: 800, width: 800 },
       },
@@ -90,7 +90,7 @@ describe('writeTestResult', () => {
       // the default output directory in playwright
       {
         titlePath: ['file.spec.ts', 'Toy Story'],
-        outputDir: resolve('test-results/toy-story-chromium'),
+        outputDir: resolve('test-results'),
         pageUrl: 'http://localhost:3000/',
         viewport: { height: 800, width: 800 },
       },
@@ -127,7 +127,7 @@ describe('writeTestResult', () => {
       {
         titlePath: ['file.spec.ts', 'Test Story'],
         // simulates setting a custom output directory in Playwright
-        outputDir: resolve('some-custom-directory/directory/test-story-chromium'),
+        outputDir: resolve('some-custom-directory/directory'),
         pageUrl: 'http://localhost:3000/',
         viewport: { height: 800, width: 800 },
       },
@@ -151,7 +151,7 @@ describe('writeTestResult', () => {
       await writeTestResult(
         {
           titlePath: ['file.spec.ts', 'A grouping', 'Test Story'],
-          outputDir: resolve('test-results/test-story-chromium'),
+          outputDir: resolve('test-results'),
           pageUrl: 'http://localhost:3000/',
           viewport: { height: 800, width: 800 },
         },
@@ -177,7 +177,7 @@ describe('writeTestResult', () => {
             '.someFunction',
             '.someFunction() calls something',
           ],
-          outputDir: resolve('test-results/test-story-chromium'),
+          outputDir: resolve('test-results'),
           pageUrl: 'http://localhost:3000/',
           viewport: { height: 800, width: 800 },
         },
@@ -199,7 +199,7 @@ describe('writeTestResult', () => {
       await writeTestResult(
         {
           titlePath: ['some.file.spec.ts', 'Test Story'],
-          outputDir: resolve('test-results/test-story-chromium'),
+          outputDir: resolve('test-results'),
           pageUrl: 'http://localhost:3000/',
           viewport: { height: 800, width: 800 },
         },
@@ -221,7 +221,7 @@ describe('writeTestResult', () => {
       await writeTestResult(
         {
           titlePath: ['some.file.cy.ts', 'Test Story'],
-          outputDir: resolve('test-results/test-story-chromium'),
+          outputDir: resolve('test-results'),
           pageUrl: 'http://localhost:3000/',
           viewport: { height: 800, width: 800 },
         },
@@ -243,7 +243,7 @@ describe('writeTestResult', () => {
       await writeTestResult(
         {
           titlePath: ['file.ts', 'Test Story'],
-          outputDir: resolve('test-results/test-story-chromium'),
+          outputDir: resolve('test-results'),
           pageUrl: 'http://localhost:3000/',
           viewport: { height: 800, width: 800 },
         },

packages/shared/src/write-archive/index.ts

@@ -1,4 +1,4 @@
-import { join } from 'path';
+import { join } from 'node:path';
 import { outputFile, ensureDir, outputJSONFile } from '../utils/filePaths';
 import { logger } from '../utils/logger';
 import { ArchiveFile } from './archive-file';
@@ -38,9 +38,7 @@ export async function writeTestResult(
   );
   // in Storybook, `/` splits the title out into hierarchies (folders)
   const title = titlePathWithoutFileExtensions.join('/');
-  // outputDir gives us the test-specific subfolder (https://playwright.dev/docs/api/class-testconfig#test-config-output-dir);
-  // we want to write one level above that
-  const finalOutputDir = join(outputDir, '..', 'chromatic-archives');
+  const finalOutputDir = join(outputDir, 'chromatic-archives');
 
   const archiveDir = join(finalOutputDir, 'archive');