fix(testIsolation): improve the behavior, clarify config options and …

…sync with session command (#24316) Co-authored-by: Matt Henkes <[email protected]> Co-authored-by: Bill Glesias <[email protected]>
cypress-io · Oct 24, 2022 · 75cce81 · 75cce81 · cypress-bot · Oct 24, 2022
1 parent 851552c
commit 75cce81
Show file tree

Hide file tree

Showing 20 changed files with 1,345 additions and 592 deletions.
diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts
@@ -2808,12 +2808,32 @@ declare namespace Cypress {
      */
     supportFile: string | false
     /**
-     * The test isolation level applied to ensure a clean slate between tests.
-     *   - legacy - resets/clears aliases, intercepts, clock, viewport, cookies, and local storage before each test.
-     *   - strict - applies all resets/clears from legacy, plus clears the page by visiting 'about:blank' to ensure clean app state before each test.
-     * @default "legacy", however, when experimentalSessionAndOrigin=true, the default is "strict"
+     * The test isolation ensures a clean browser context between tests. This option is only available when
+     * `experimentalSessionAndOrigin=true`.
+     *
+     * Cypress will always reset/clear aliases, intercepts, clock, and viewport before each test
+     * to ensure a clean test slate; i.e. this configuration only impacts the browser context.
+     *
+     * Note: the [`cy.session()`](https://on.cypress.io/session) command will inherent this value to determine whether
+     * or not the page is cleared when the command executes. This command is only available in end-to-end testing.
+     *
+     *  - on - The page is cleared before each test. Cookies, local storage and session storage in all domains are cleared
+     *         before each test. The `cy.session()` command will also clear the page and current browser context when creating
+     *         or restoring the browser session.
+     *  - off - The current browser state will persist between tests. The page does not clear before the test and cookies, local
+     *          storage and session storage will be available in the next test. The `cy.session()` command will only clear the
+     *          current browser context when creating or restoring the browser session - the current page will not clear.
+     *
+     * Tradeoffs:
+     *      Turning test isolation off may improve performance of end-to-end tests, however, previous tests could impact the
+     *      browser state of the next test and cause inconsistency when using .only(). Be mindful to write isolated tests when
+     *      test isolation is off. If a test in the suite impacts the state of other tests and it were to fail, you could see
+     *      misleading errors in later tests which makes debugging clunky. See the [documentation](https://on.cypress.io/test-isolation)
+     *      for more information.
+     *
+     * @default null, when experimentalSessionAndOrigin=false. The default is 'on' when experimentalSessionAndOrigin=true.
      */
-    testIsolation: 'legacy' | 'strict'
+    testIsolation: null | 'on' | 'off'
     /**
      * Path to folder where videos will be saved after a headless or CI run
      * @default "cypress/videos"

diff --git a/packages/config/__snapshots__/index.spec.ts.js b/packages/config/__snapshots__/index.spec.ts.js
@@ -70,7 +70,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys 1
   "supportFile": "cypress/support/e2e.{js,jsx,ts,tsx}",
   "supportFolder": false,
   "taskTimeout": 60000,
-  "testIsolation": "legacy",
+  "testIsolation": null,
   "trashAssetsBeforeRuns": true,
   "userAgent": null,
   "video": true,
@@ -155,7 +155,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys f
   "supportFile": "cypress/support/e2e.{js,jsx,ts,tsx}",
   "supportFolder": false,
   "taskTimeout": 60000,
-  "testIsolation": "legacy",
+  "testIsolation": null,
   "trashAssetsBeforeRuns": true,
   "userAgent": null,
   "video": true,

diff --git a/packages/config/src/browser.ts b/packages/config/src/browser.ts
@@ -155,15 +155,19 @@ export const matchesConfigKey = (key: string) => {
   return
 }
 
-export const validate = (cfg: any, onErr: (property: ErrResult | string) => void) => {
+export const validate = (cfg: any, onErr: (property: ErrResult | string) => void, testingType: TestingType | null) => {
   debug('validating configuration')
 
   return _.each(cfg, (value, key) => {
     const validationFn = validationRules[key]
 
     // key has a validation rule & value different from the default
     if (validationFn && value !== defaultValues[key]) {
-      const result = validationFn(key, value)
+      const result = validationFn(key, value, {
+        testingType,
+        // TODO: remove with experimentalSessionAndOrigin. Fixed with: https://github.com/cypress-io/cypress/issues/21471
+        experimentalSessionAndOrigin: cfg.experimentalSessionAndOrigin,
+      })
 
       if (result !== true) {
         return onErr(result)
@@ -199,6 +203,9 @@ export const validateOverridableAtRunTime = (config: any, isSuiteLevelOverride:
       return
     }
 
+    // TODO: add a hook to ensure valid testing-type configuration is being set at runtime for all configuration values.
+    // https://github.com/cypress-io/cypress/issues/24365
+
     if (overrideLevel === 'never' || (overrideLevel === 'suite' && !isSuiteLevelOverride)) {
       onErr({
         invalidConfigKey: configKey,

diff --git a/packages/config/src/options.ts b/packages/config/src/options.ts
@@ -31,6 +31,11 @@ const BREAKING_OPTION_ERROR_KEY: Readonly<AllCypressErrorNames[]> = [
   'TEST_FILES_RENAMED',
 ] as const
 
+type ValidationOptions = {
+  testingType: TestingType | null
+  experimentalSessionAndOrigin: boolean
+}
+
 export type BreakingOptionErrorKey = typeof BREAKING_OPTION_ERROR_KEY[number]
 
 export type OverrideLevel = 'any' | 'suite' | 'never'
@@ -90,7 +95,7 @@ export interface BreakingOption {
   showInLaunchpad?: boolean
 }
 
-const isValidConfig = (testingType: string, config: any) => {
+const isValidConfig = (testingType: string, config: any, opts: ValidationOptions) => {
   const status = validate.isPlainObject(testingType, config)
 
   if (status !== true) {
@@ -99,7 +104,7 @@ const isValidConfig = (testingType: string, config: any) => {
 
   for (const rule of options) {
     if (rule.name in config && rule.validation) {
-      const status = rule.validation(`${testingType}.${rule.name}`, config[rule.name])
+      const status = rule.validation(`${testingType}.${rule.name}`, config[rule.name], opts)
 
       if (status !== true) {
         return status
@@ -200,6 +205,7 @@ const driverConfigOptions: Array<DriverConfigOption> = [
     isExperimental: true,
     requireRestartOnChange: 'server',
   }, {
+    // TODO: remove with experimentalSessionAndOrigin. Fixed with: https://github.com/cypress-io/cypress/issues/21471
     name: 'experimentalSessionAndOrigin',
     defaultValue: false,
     validation: validate.isBoolean,
@@ -373,11 +379,37 @@ const driverConfigOptions: Array<DriverConfigOption> = [
     name: 'testIsolation',
     // TODO: https://github.com/cypress-io/cypress/issues/23093
     // When experimentalSessionAndOrigin is removed and released as GA,
-    // update the defaultValue from 'legacy' to 'strict' and
+    // update the defaultValue from undefined to 'on' and
     // update this code to remove the check/override specific to enable
-    // strict by default when experimentalSessionAndOrigin=true
-    defaultValue: 'legacy',
-    validation: validate.isOneOf('legacy', 'strict'),
+    // 'on' by default when experimentalSessionAndOrigin=true
+    defaultValue: (options: Record<string, any> = {}) => {
+      if (options.testingType === 'component') {
+        return null
+      }
+
+      return options?.experimentalSessionAndOrigin || options?.config?.e2e?.experimentalSessionAndOrigin ? 'on' : null
+    },
+    validation: (key: string, value: any, opts: ValidationOptions) => {
+      const { testingType, experimentalSessionAndOrigin } = opts
+
+      if (testingType == null || testingType === 'component') {
+        return true
+      }
+
+      if (experimentalSessionAndOrigin && testingType === 'e2e') {
+        return validate.isOneOf('on', 'off')(key, value)
+      }
+
+      if (value == null) {
+        return true
+      }
+
+      return {
+        key,
+        value,
+        type: 'not set unless the experimentalSessionAndOrigin flag is turned on',
+      }
+    },
     overrideLevel: 'suite',
   }, {
     name: 'trashAssetsBeforeRuns',

diff --git a/packages/config/src/project/index.ts b/packages/config/src/project/index.ts
@@ -60,7 +60,7 @@ export function updateWithPluginValues (cfg: FullConfig, modifiedConfig: any, te
     }
 
     return errors.throwErr('CONFIG_VALIDATION_ERROR', 'configFile', configFile, validationResult)
-  })
+  }, testingType)
 
   debug('validate that there is no breaking config options added by setupNodeEvents')
 

diff --git a/packages/config/src/project/utils.ts b/packages/config/src/project/utils.ts
@@ -401,7 +401,11 @@ export function mergeDefaults (
     config.baseUrl = url.replace(/\/\/+$/, '/')
   }
 
-  const defaultsForRuntime = getDefaultValues(options)
+  const defaultsForRuntime = getDefaultValues({
+    ...options,
+    // TODO: clean this up. Fixed with: https://github.com/cypress-io/cypress/issues/21471
+    experimentalSessionAndOrigin: config.experimentalSessionAndOrigin,
+  })
 
   _.defaultsDeep(config, defaultsForRuntime)
 
@@ -454,7 +458,7 @@ export function mergeDefaults (
     }
 
     return errors.throwErr('CONFIG_VALIDATION_ERROR', null, null, validationResult)
-  })
+  }, testingType)
 
   config = setAbsolutePaths(config)
 
@@ -477,15 +481,15 @@ export function mergeDefaults (
   }, testingType)
 
   // TODO: https://github.com/cypress-io/cypress/issues/23093
-  // testIsolation should equal 'strict' by default when experimentalSessionAndOrigin=true
+  // testIsolation should equal 'on' by default when experimentalSessionAndOrigin=true
   // Once experimentalSessionAndOrigin is made GA, remove this logic and update the defaultValue
-  // to be be 'strict'
+  // to be be 'on'
   if (testingType === 'e2e' && config.experimentalSessionAndOrigin) {
     if (config.rawJson.testIsolation) {
       config.resolved.testIsolation.from = 'config'
     } else {
-      config.testIsolation = 'strict'
-      config.resolved.testIsolation.value = 'strict'
+      config.testIsolation = 'on'
+      config.resolved.testIsolation.value = 'on'
       config.resolved.testIsolation.from === 'default'
     }
   }

diff --git a/packages/config/test/index.spec.ts b/packages/config/test/index.spec.ts
@@ -118,7 +118,7 @@ describe('config/src/index', () => {
 
       configUtil.validate({
         'baseUrl': 'https://',
-      }, errorFn)
+      }, errorFn, 'e2e')
 
       expect(errorFn).to.have.callCount(0)
     })
@@ -128,7 +128,7 @@ describe('config/src/index', () => {
 
       configUtil.validate({
         'baseUrl': ' ',
-      }, errorFn)
+      }, errorFn, 'e2e')
 
       expect(errorFn).to.have.been.calledWithMatch({ key: 'baseUrl' })
       expect(errorFn).to.have.been.calledWithMatch({ type: 'a fully qualified URL (starting with `http://` or `https://`)' })
@@ -195,7 +195,7 @@ describe('config/src/index', () => {
 
         const isSuiteOverride = true
 
-        configUtil.validateOverridableAtRunTime({ testIsolation: 'strict' }, isSuiteOverride, errorFn)
+        configUtil.validateOverridableAtRunTime({ testIsolation: 'on' }, isSuiteOverride, errorFn)
 
         expect(errorFn).to.have.callCount(0)
       })

diff --git a/packages/config/test/project/utils.spec.ts b/packages/config/test/project/utils.spec.ts
@@ -1074,7 +1074,7 @@ describe('config/src/project/utils', () => {
             supportFile: { value: false, from: 'config' },
             supportFolder: { value: false, from: 'default' },
             taskTimeout: { value: 60000, from: 'default' },
-            testIsolation: { value: 'legacy', from: 'default' },
+            testIsolation: { value: null, from: 'default' },
             trashAssetsBeforeRuns: { value: true, from: 'default' },
             userAgent: { value: null, from: 'default' },
             video: { value: true, from: 'default' },
@@ -1190,7 +1190,7 @@ describe('config/src/project/utils', () => {
             supportFile: { value: false, from: 'config' },
             supportFolder: { value: false, from: 'default' },
             taskTimeout: { value: 60000, from: 'default' },
-            testIsolation: { value: 'legacy', from: 'default' },
+            testIsolation: { value: null, from: 'default' },
             trashAssetsBeforeRuns: { value: true, from: 'default' },
             userAgent: { value: null, from: 'default' },
             video: { value: true, from: 'default' },
@@ -1206,7 +1206,7 @@ describe('config/src/project/utils', () => {
         })
       })
 
-      it('sets testIsolation=strict by default when experimentalSessionAndOrigin=true and e2e testing', () => {
+      it('sets testIsolation=on by default when experimentalSessionAndOrigin=true and e2e testing', () => {
         sinon.stub(utils, 'getProcessEnvVars').returns({})
 
         const obj = {
@@ -1227,7 +1227,7 @@ describe('config/src/project/utils', () => {
           expect(cfg.resolved).to.have.property('experimentalSessionAndOrigin')
           expect(cfg.resolved.experimentalSessionAndOrigin).to.deep.eq({ value: true, from: 'config' })
           expect(cfg.resolved).to.have.property('testIsolation')
-          expect(cfg.resolved.testIsolation).to.deep.eq({ value: 'strict', from: 'default' })
+          expect(cfg.resolved.testIsolation).to.deep.eq({ value: 'on', from: 'default' })
         })
       })
 
@@ -1239,7 +1239,7 @@ describe('config/src/project/utils', () => {
           supportFile: false,
           baseUrl: 'http://localhost:8080',
           experimentalSessionAndOrigin: true,
-          testIsolation: 'legacy',
+          testIsolation: 'on',
         }
 
         const options = {
@@ -1253,7 +1253,7 @@ describe('config/src/project/utils', () => {
           expect(cfg.resolved).to.have.property('experimentalSessionAndOrigin')
           expect(cfg.resolved.experimentalSessionAndOrigin).to.deep.eq({ value: true, from: 'config' })
           expect(cfg.resolved).to.have.property('testIsolation')
-          expect(cfg.resolved.testIsolation).to.deep.eq({ value: 'legacy', from: 'config' })
+          expect(cfg.resolved.testIsolation).to.deep.eq({ value: 'on', from: 'config' })
         })
       })
     })

diff --git a/packages/data-context/src/data/ProjectConfigManager.ts b/packages/data-context/src/data/ProjectConfigManager.ts
@@ -367,7 +367,7 @@ export class ProjectConfigManager {
       }
 
       throw getError('CONFIG_VALIDATION_ERROR', 'configFile', file || null, errMsg)
-    })
+    }, this._testingType)
 
     return validateNoBreakingConfigLaunchpad(
       config,
-Original file line number
+Diff line change
@@ Expand Up @@
         }
         return errors.throwErr('CONFIG_VALIDATION_ERROR', 'configFile', configFile, validationResult)
-      })
+      }, testingType)
       debug('validate that there is no breaking config options added by setupNodeEvents')
@@ Expand Down @@