Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(testIsolation): improve the behavior, clarify config options and sync with session command #24316

Merged
merged 17 commits into from
Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions cli/types/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2808,12 +2808,40 @@ 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"
*/
testIsolation: 'legacy' | 'strict'
* The test isolation ensures a clean browser context between tests.
*
* Cypress will always resets/clears aliases, intercepts, clock, and viewport before each test
AtofStryker marked this conversation as resolved.
Show resolved Hide resolved
emilyrohrbough marked this conversation as resolved.
Show resolved Hide resolved
* 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.
*
* Options:
* When experimentalSessionAndOrigin=false
* - legacy - "Pass through" to document the default Cypress test isolation behavior. The page is not cleared
* before each end-to-end test. Cookies, local storage and session storage in the current domains are
* cleared before each test. The `cy.session()` command is not available for end-to-end without this experiment.
* NOTE: this behavior will be changed in the next major release and the new default will be 'on'.
*
* When experimentalSessionAndOrigin=true
* - 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.
*
AtofStryker marked this conversation as resolved.
Show resolved Hide resolved
* @default 'legacy', when running end-to-end tests. When running component tests or running end-to-end tests with
* experimentalSessionAndOrigin=true, the default is 'on'.
*/
testIsolation: 'legacy' | 'on' | 'off'
/**
* Path to folder where videos will be saved after a headless or CI run
* @default "cypress/videos"
Expand Down
11 changes: 9 additions & 2 deletions packages/config/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
emilyrohrbough marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/cypress-io/cypress/issues/24365

if (overrideLevel === 'never' || (overrideLevel === 'suite' && !isSuiteLevelOverride)) {
onErr({
invalidConfigKey: configKey,
Expand Down
25 changes: 21 additions & 4 deletions packages/config/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,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,
Expand Down Expand Up @@ -373,11 +374,27 @@ 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 'legacy' 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 'on'
}

return options?.experimentalSessionAndOrigin || options?.config?.e2e?.experimentalSessionAndOrigin ? 'on' : 'legacy'
},
validation: (key: string, value: any, { testingType, experimentalSessionAndOrigin }: { testingType: TestingType, experimentalSessionAndOrigin: boolean }) => {
if (testingType === 'component') {
return validate.isOneOf('on')(key, value)
}

if (experimentalSessionAndOrigin) {
return validate.isOneOf('on', 'off')(key, value)
}

return validate.isOneOf('legacy')(key, value)
},
overrideLevel: 'suite',
}, {
name: 'trashAssetsBeforeRuns',
Expand Down
2 changes: 1 addition & 1 deletion packages/config/src/project/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
16 changes: 10 additions & 6 deletions packages/config/src/project/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -454,7 +458,7 @@ export function mergeDefaults (
}

return errors.throwErr('CONFIG_VALIDATION_ERROR', null, null, validationResult)
})
}, testingType)

config = setAbsolutePaths(config)

Expand All @@ -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'
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/config/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('config/src/index', () => {

configUtil.validate({
'baseUrl': 'https://',
}, errorFn)
}, errorFn, 'e2e')

expect(errorFn).to.have.callCount(0)
})
Expand All @@ -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://`)' })
Expand Down Expand Up @@ -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)
})
Expand Down
8 changes: 4 additions & 4 deletions packages/config/test/project/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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' })
})
})

Expand All @@ -1239,7 +1239,7 @@ describe('config/src/project/utils', () => {
supportFile: false,
baseUrl: 'http://localhost:8080',
experimentalSessionAndOrigin: true,
testIsolation: 'legacy',
testIsolation: 'on',
}

const options = {
Expand All @@ -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' })
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/data-context/src/data/ProjectConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export class ProjectConfigManager {
}

throw getError('CONFIG_VALIDATION_ERROR', 'configFile', file || null, errMsg)
})
}, this._testingType)

return validateNoBreakingConfigLaunchpad(
config,
Expand Down
Loading