-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
breaking: no longer inject document.domain by default #30770
base: release/14.0.0
Are you sure you want to change the base?
Changes from 72 commits
5b42419
151952d
dc4fb0b
330adc4
2000405
0f822b5
8275e4d
4a52817
b3b60a1
a8f6406
c54224b
110996b
4895219
33d33cd
1d348c9
8b943d0
1166858
25c3ac7
b4c0be3
3e613f0
b6090f7
4da149d
9bcc1f3
e8ccb4d
db047b7
55fa5b1
018816c
5622cc5
b01df45
03f34bb
c3ac388
376627b
52156bf
18ae100
e66d194
19ce24e
3d2a0c1
edbdb36
dd9a50d
70c0a8f
29110af
e1e0829
ec669b8
e448022
e19fb59
9a1c30e
e97d548
941701f
e651ef6
0ec1b12
5de96f0
0d6b14d
5afda0f
6cde8b9
69b7f6b
ddce890
a6b486f
a857a38
ec653df
81cacf8
125b41e
1dcc5ab
7378f8e
af8f2a7
40ea65a
e4cdd85
361b066
3de7b83
b560617
3966f54
b8cfc70
0300676
4e090de
58db915
728e4bf
bed8fb3
a0a151a
6c52c93
34e0573
c03e62b
0539a72
a8f0663
fb1219c
ea57654
b86bd00
a20a34e
1bece96
8ce3f5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ mainBuildFilters: &mainBuildFilters | |
- 'update-v8-snapshot-cache-on-develop' | ||
- 'chore/update_vue_test_utils' | ||
- 'publish-binary' | ||
- 'cacie/29590/document-domain-subdomains' | ||
|
||
# usually we don't build Mac app - it takes a long time | ||
# but sometimes we want to really confirm we are doing the right thing | ||
|
@@ -573,6 +574,11 @@ commands: | |
description: chrome channel to install | ||
type: string | ||
default: '' | ||
inject-document-domain: | ||
description: run subset of tests with injectDocumentDomain config enabled | ||
type: boolean | ||
default: false | ||
|
||
steps: | ||
- restore_cached_workspace | ||
- when: | ||
|
@@ -594,11 +600,20 @@ commands: | |
echo Current working directory is $PWD | ||
echo Total containers $CIRCLE_NODE_TOTAL | ||
|
||
|
||
if << parameters.inject-document-domain >> ; then | ||
YARN_CMD="cypress:run:inject-document-domain" | ||
PARALLEL="" | ||
else | ||
YARN_CMD="cypress:run" | ||
PARALLEL="--parallel --group 5x-driver-<<parameters.browser>>" | ||
fi | ||
|
||
if [[ -v MAIN_RECORD_KEY ]]; then | ||
# internal PR | ||
CYPRESS_RECORD_KEY=$MAIN_RECORD_KEY \ | ||
CYPRESS_INTERNAL_ENABLE_TELEMETRY="true" \ | ||
yarn cypress:run --record --parallel --group 5x-driver-<<parameters.browser>> --browser <<parameters.browser>> --runner-ui | ||
yarn $YARN_CMD --record $PARALLEL --browser <<parameters.browser>> --runner-ui | ||
else | ||
# external PR | ||
TESTFILES=$(circleci tests glob "cypress/e2e/**/*.cy.*" | circleci tests split --total=$CIRCLE_NODE_TOTAL) | ||
|
@@ -607,7 +622,7 @@ commands: | |
if [[ -z "$TESTFILES" ]]; then | ||
echo "Empty list of test files" | ||
fi | ||
yarn cypress:run --browser <<parameters.browser>> --spec $TESTFILES --runner-ui | ||
yarn $YARN_CMD --browser <<parameters.browser>> --spec $TESTFILES --runner-ui | ||
fi | ||
working_directory: packages/driver | ||
- verify-mocha-results | ||
|
@@ -2002,6 +2017,16 @@ jobs: | |
- run-driver-integration-tests: | ||
browser: chrome | ||
install-chrome-channel: stable | ||
|
||
driver-integration-tests-chrome-inject-document-domain: | ||
<<: *defaults | ||
parallelism: 5 | ||
resource_class: medium+ | ||
steps: | ||
- run-driver-integration-tests: | ||
browser: chrome | ||
install-chrome-channel: stable | ||
inject-document-domain: true | ||
|
||
driver-integration-tests-chrome-beta: | ||
<<: *defaults | ||
|
@@ -2012,6 +2037,16 @@ jobs: | |
browser: chrome:beta | ||
install-chrome-channel: beta | ||
|
||
driver-integration-tests-chrome-beta-inject-document-domain: | ||
<<: *defaults | ||
resource_class: medium+ | ||
parallelism: 5 | ||
steps: | ||
- run-driver-integration-tests: | ||
browser: chrome:beta | ||
install-chrome-channel: beta | ||
inject-document-domain: true | ||
|
||
driver-integration-tests-firefox: | ||
<<: *defaults | ||
resource_class: medium+ | ||
|
@@ -2034,6 +2069,8 @@ jobs: | |
steps: | ||
- run-driver-integration-tests: | ||
browser: webkit | ||
# inject document domain must be true for webkit, as cy.origin is not supported | ||
inject-document-domain: true | ||
|
||
run-reporter-component-tests-chrome: | ||
<<: *defaults | ||
|
@@ -2816,7 +2853,8 @@ linux-x64-workflow: &linux-x64-workflow | |
- run-vite-dev-server-integration-tests | ||
- driver-integration-tests-firefox | ||
- driver-integration-tests-chrome | ||
- driver-integration-tests-chrome-beta | ||
- driver-integration-tests-chrome-inject-document-domain | ||
- driver-integration-tests-chrome-beta-inject-document-domain | ||
- driver-integration-tests-electron | ||
- driver-integration-tests-webkit | ||
- driver-integration-memory-tests | ||
|
@@ -2873,10 +2911,18 @@ linux-x64-workflow: &linux-x64-workflow | |
context: test-runner:cypress-record-key | ||
requires: | ||
- build | ||
- driver-integration-tests-chrome-inject-document-domain: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- build | ||
- driver-integration-tests-chrome-beta: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- build | ||
- driver-integration-tests-chrome-beta-inject-document-domain: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- build | ||
- driver-integration-tests-firefox: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
|
@@ -3000,6 +3046,8 @@ linux-x64-workflow: &linux-x64-workflow | |
- driver-integration-tests-firefox | ||
- driver-integration-tests-chrome | ||
- driver-integration-tests-chrome-beta | ||
- driver-integration-tests-chrome-inject-document-domain | ||
- driver-integration-tests-chrome-beta-inject-document-domain | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adding firefox and electron to this mix was going to be pretty cumbersome - I'd like to refactor workflows.yml to use matrix parameters, which would make all of this so much easier. |
||
- driver-integration-tests-electron | ||
- driver-integration-tests-webkit | ||
- driver-integration-memory-tests | ||
|
@@ -3234,10 +3282,18 @@ linux-x64-contributor-workflow: &linux-x64-contributor-workflow | |
context: test-runner:cypress-record-key | ||
requires: | ||
- contributor-pr | ||
- driver-integration-tests-chrome-inject-document-domain: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- contributor-pr | ||
- driver-integration-tests-chrome-beta: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- contributor-pr | ||
- driver-integration-tests-chrome-beta-inject-document-domain: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
- contributor-pr | ||
- driver-integration-tests-firefox: | ||
context: test-runner:cypress-record-key | ||
requires: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,8 +70,7 @@ declare namespace Cypress { | |
strategy: 'file' | 'http' | ||
origin: string | ||
fileServer: string | null | ||
props: Record<string, any> | ||
visiting: string | ||
props: Record<string, any> | null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
interface Backend { | ||
|
@@ -3113,6 +3112,8 @@ declare namespace Cypress { | |
* @default null | ||
*/ | ||
experimentalSkipDomainInjection: string[] | null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cacieprins This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks to need some adjustments in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think full removal of |
||
// TODO: document | ||
injectDocumentDomain: boolean | ||
/** | ||
* Enables AST-based JS/HTML rewriting. This may fix issues caused by the existing regex-based JS/HTML replacement algorithm. | ||
* @default false | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,10 @@ | ||
import { describe, it } from 'vitest' | ||
import { describe, it, beforeEach, afterEach } from 'vitest' | ||
import Fixtures, { ProjectFixtureDir } from '@tooling/system-tests' | ||
import * as FixturesScaffold from '@tooling/system-tests/lib/dep-installer' | ||
import execa from 'execa' | ||
import path from 'path' | ||
import * as fs from 'fs-extra' | ||
|
||
const scaffoldAngularProject = async (project: string) => { | ||
const projectPath = Fixtures.projectPath(project) | ||
|
||
Fixtures.removeProject(project) | ||
await Fixtures.scaffoldProject(project) | ||
await FixturesScaffold.scaffoldProjectNodeModules({ project }) | ||
await fs.remove(path.join(projectPath, 'cypress.config.ts')) | ||
await fs.remove(path.join(projectPath, 'cypress')) | ||
|
||
return projectPath | ||
} | ||
|
||
const runCommandInProject = (command: string, projectPath: string) => { | ||
const [ex, ...args] = command.split(' ') | ||
|
||
|
@@ -38,25 +26,38 @@ const cypressSchematicPackagePath = path.join(__dirname, '..') | |
|
||
const ANGULAR_PROJECTS: ProjectFixtureDir[] = ['angular-18', 'angular-19'] | ||
|
||
describe('ng add @cypress/schematic / e2e and ct', { timeout: 1000 * 60 * 5 }, function () { | ||
for (const project of ANGULAR_PROJECTS) { | ||
it('should install ct files with option and no component specs', async () => { | ||
const projectPath = await scaffoldAngularProject(project) | ||
const timeout = 1000 * 60 * 5 | ||
|
||
await runCommandInProject(`yarn add @cypress/schematic@file:${cypressSchematicPackagePath}`, projectPath) | ||
await runCommandInProject('yarn ng add @cypress/schematic --e2e --component', projectPath) | ||
await copyAngularMount(projectPath) | ||
await runCommandInProject('yarn ng run angular:ct --watch false --spec src/app/app.component.cy.ts', projectPath) | ||
}) | ||
|
||
it('should generate component alongside component spec', async () => { | ||
const projectPath = await scaffoldAngularProject(project) | ||
|
||
await runCommandInProject(`yarn add @cypress/schematic@file:${cypressSchematicPackagePath}`, projectPath) | ||
await runCommandInProject('yarn ng add @cypress/schematic --e2e --component', projectPath) | ||
await copyAngularMount(projectPath) | ||
await runCommandInProject('yarn ng generate c foo', projectPath) | ||
await runCommandInProject('yarn ng run angular:ct --watch false --spec src/app/foo/foo.component.cy.ts', projectPath) | ||
describe('ng add @cypress/schematic / e2e and ct', function () { | ||
for (const project of ANGULAR_PROJECTS) { | ||
describe(project, () => { | ||
const projectPath: string = Fixtures.projectPath(project) | ||
|
||
beforeEach(async () => { | ||
await Fixtures.scaffoldProject(project) | ||
await FixturesScaffold.scaffoldProjectNodeModules({ project }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moving the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we consider moving these changes into its own PR for a quick review and to reduce changeset here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so - This refactor was done to help understand why the tests were failing, and don't actually change the nature of the tests other than clarification of stdout. |
||
await fs.remove(path.join(projectPath, 'cypress.config.ts')) | ||
await fs.remove(path.join(projectPath, 'cypress')) | ||
|
||
await runCommandInProject(`yarn add @cypress/schematic@file:${cypressSchematicPackagePath}`, projectPath) | ||
}, timeout) | ||
|
||
afterEach(() => { | ||
Fixtures.removeProject(project) | ||
}, timeout) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving the cleanup to an afterEach ensures that the project is removed after the last |
||
|
||
it('should install ct files with option and no component specs', async () => { | ||
await runCommandInProject('yarn ng add @cypress/schematic --e2e --component', projectPath) | ||
await copyAngularMount(projectPath) | ||
await runCommandInProject('yarn ng run angular:ct --watch false --spec src/app/app.component.cy.ts', projectPath) | ||
}, timeout) | ||
|
||
it('should generate component alongside component spec', async () => { | ||
await runCommandInProject('yarn ng add @cypress/schematic --e2e --component', projectPath) | ||
await copyAngularMount(projectPath) | ||
await runCommandInProject('yarn ng generate c foo', projectPath) | ||
await runCommandInProject('yarn ng run angular:ct --watch false --spec src/app/foo/foo.component.cy.ts', projectPath) | ||
}, timeout) | ||
}) | ||
} | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,12 +100,6 @@ export const Cypress = ( | |
configureServer: async (server: ViteDevServer) => { | ||
server.middlewares.use(`${base}index.html`, async (req, res) => { | ||
let transformedIndexHtml = await server.transformIndexHtml(base, '') | ||
const viteImport = `<script type="module" src="${options.cypressConfig.devServerPublicPathRoute}/@vite/client"></script>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because |
||
|
||
// If we're doing cy-in-cy, we need to be able to access the Cypress instance from the parent frame. | ||
if (process.env.CYPRESS_INTERNAL_VITE_OPEN_MODE_TESTING) { | ||
transformedIndexHtml = transformedIndexHtml.replace(viteImport, `<script>document.domain = 'localhost';</script>${viteImport}`) | ||
} | ||
|
||
return res.end(transformedIndexHtml) | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -278,5 +278,6 @@ | |
"devtools-protocol": "0.0.1346313", | ||
"sharp": "0.29.3", | ||
"vue-template-compiler": "2.6.12" | ||
} | ||
}, | ||
"packageManager": "[email protected]+sha512.ff4579ab459bb25aa7c0ff75b62acebe576f6084b36aa842971cf250a5d8c6cd3bc9420b22ce63c7f93a0857bc6ef29291db39c3e7a23aab5adfd5a4dd6c5d71" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. using yarn in conjunction with corepack added this line There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think develop is on |
||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,6 +40,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys 1 | |||||
'experimentalRunAllSpecs': false, | ||||||
'experimentalMemoryManagement': false, | ||||||
'experimentalModifyObstructiveThirdPartyCode': false, | ||||||
'injectDocumentDomain': false, | ||||||
'experimentalSkipDomainInjection': null, | ||||||
'experimentalOriginDependencies': false, | ||||||
'experimentalSourceRewriting': false, | ||||||
|
@@ -131,6 +132,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys f | |||||
'experimentalRunAllSpecs': false, | ||||||
'experimentalMemoryManagement': false, | ||||||
'experimentalModifyObstructiveThirdPartyCode': false, | ||||||
'injectDocumentDomain': false, | ||||||
'experimentalSkipDomainInjection': null, | ||||||
'experimentalOriginDependencies': false, | ||||||
'experimentalSourceRewriting': false, | ||||||
|
@@ -218,6 +220,7 @@ exports['config/src/index .getPublicConfigKeys returns list of public config key | |||||
'experimentalRunAllSpecs', | ||||||
'experimentalMemoryManagement', | ||||||
'experimentalModifyObstructiveThirdPartyCode', | ||||||
'injectDocumentDomain', | ||||||
'experimentalSkipDomainInjection', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be removed from the snapshots
Suggested change
|
||||||
'experimentalOriginDependencies', | ||||||
'experimentalSourceRewriting', | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,6 +230,12 @@ const driverConfigOptions: Array<DriverConfigOption> = [ | |
isExperimental: true, | ||
requireRestartOnChange: 'server', | ||
}, { | ||
name: 'injectDocumentDomain', | ||
defaultValue: false, | ||
validation: validate.isBoolean, | ||
requireRestartOnChange: 'server', | ||
}, | ||
{ | ||
name: 'experimentalSkipDomainInjection', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't need this anymore as well |
||
defaultValue: null, | ||
validation: validate.isNullOrArrayOfStrings, | ||
|
@@ -768,6 +774,16 @@ export const testingTypeBreakingOptions: { e2e: Array<BreakingOption>, component | |
errorKey: 'JIT_COMPONENT_TESTING', | ||
isWarning: false, | ||
}, | ||
{ | ||
name: 'experimentalSkipDomainInjection', | ||
errorKey: 'EXPERIMENTAL_SKIP_DOMAIN_INJECTION', | ||
isWarning: false, | ||
}, | ||
{ | ||
name: 'injectDocumentDomain', | ||
errorKey: 'INJECT_DOCUMENT_DOMAIN_DEPRECATION', | ||
isWarning: true, | ||
}, | ||
], | ||
component: [ | ||
{ | ||
|
@@ -800,5 +816,10 @@ export const testingTypeBreakingOptions: { e2e: Array<BreakingOption>, component | |
errorKey: 'EXPERIMENTAL_USE_DEFAULT_DOCUMENT_DOMAIN_E2E_ONLY', | ||
isWarning: false, | ||
}, | ||
{ | ||
name: 'injectDocumentDomain', | ||
errorKey: 'INJECT_DOCUMENT_DOMAIN_E2E_ONLY', | ||
isWarning: false, | ||
}, | ||
], | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1085,6 +1085,7 @@ describe('config/src/project/utils', () => { | |
fixturesFolder: { value: 'cypress/fixtures', from: 'default' }, | ||
hosts: { value: null, from: 'default' }, | ||
includeShadowDom: { value: false, from: 'default' }, | ||
injectDocumentDomain: { value: false, from: 'default' }, | ||
justInTimeCompile: { value: true, from: 'default' }, | ||
isInteractive: { value: true, from: 'default' }, | ||
keystrokeDelay: { value: 0, from: 'default' }, | ||
|
@@ -1204,6 +1205,7 @@ describe('config/src/project/utils', () => { | |
fixturesFolder: { value: 'cypress/fixtures', from: 'default' }, | ||
hosts: { value: null, from: 'default' }, | ||
includeShadowDom: { value: false, from: 'default' }, | ||
injectDocumentDomain: { value: false, from: 'default' }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to separate out the full removal of |
||
justInTimeCompile: { value: true, from: 'default' }, | ||
isInteractive: { value: true, from: 'default' }, | ||
keystrokeDelay: { value: 0, from: 'default' }, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be parallel because it ends up with the same build id but a different set of specs from the run without
injectDocumentDomain