-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: use cloud validations for packages/server api request/response types #32632
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
base: develop
Are you sure you want to change the base?
Changes from all commits
f61e780
b35c26e
f7a85d5
b4fe99b
f0e7345
753cee9
c9a4bcf
36318c2
b14d51f
681153e
331519c
a720aef
d7e2dd8
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
lib/validations |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -532,7 +532,7 @@ async function createInstance (options: InstanceOptions) { | |
} | ||
} | ||
|
||
const _postInstanceTests = ({ | ||
async function _postInstanceTests ({ | ||
runId, | ||
instanceId, | ||
config, | ||
|
@@ -541,17 +541,18 @@ const _postInstanceTests = ({ | |
parallel, | ||
ciBuildId, | ||
group, | ||
}) => { | ||
return api.postInstanceTests({ | ||
runId, | ||
instanceId, | ||
config, | ||
tests, | ||
hooks, | ||
}) | ||
.catch((err: any) => { | ||
throwCloudCannotProceed({ parallel, ciBuildId, group, err }) | ||
}) | ||
}) { | ||
try { | ||
return await api.postInstanceTests({ | ||
runId, | ||
instanceId, | ||
config, | ||
tests, | ||
hooks, | ||
}) | ||
} catch (err: unknown) { | ||
throw cloudCannotProceedErr({ parallel, ciBuildId, group, err }) | ||
} | ||
} | ||
|
||
const createRunAndRecordSpecs = (options: any = {}) => { | ||
|
@@ -778,42 +779,34 @@ const createRunAndRecordSpecs = (options: any = {}) => { | |
}) | ||
.value() | ||
|
||
const responseDidFail = {} | ||
const response = await _postInstanceTests({ | ||
runId, | ||
instanceId, | ||
config: resolvedRuntimeConfig, | ||
tests, | ||
hooks, | ||
parallel, | ||
ciBuildId, | ||
group, | ||
}) | ||
.catch((err: any) => { | ||
onError(err) | ||
|
||
return responseDidFail | ||
}) | ||
|
||
if (response === responseDidFail) { | ||
debug('`responseDidFail` equals `response`, allowing browser to hang until it is killed: Response %o', { responseDidFail }) | ||
try { | ||
const response = await _postInstanceTests({ | ||
runId, | ||
instanceId, | ||
config: resolvedRuntimeConfig, | ||
tests, | ||
hooks, | ||
parallel, | ||
ciBuildId, | ||
group, | ||
}) | ||
|
||
// dont call the cb, let the browser hang until it's killed | ||
return | ||
} | ||
if (_.some(response.actions, { type: 'SPEC', action: 'SKIP' })) { | ||
errorsWarning('CLOUD_CANCEL_SKIPPED_SPEC') | ||
|
||
if (_.some(response.actions, { type: 'SPEC', action: 'SKIP' })) { | ||
errorsWarning('CLOUD_CANCEL_SKIPPED_SPEC') | ||
// set a property on the response so the browser runner | ||
// knows not to start executing tests | ||
project.emit('end', { skippedSpec: true, stats: {} }) | ||
|
||
// set a property on the response so the browser runner | ||
// knows not to start executing tests | ||
project.emit('end', { skippedSpec: true, stats: {} }) | ||
// dont call the cb, let the browser hang until it's killed | ||
return | ||
} | ||
|
||
// dont call the cb, let the browser hang until it's killed | ||
return | ||
return cb(response) | ||
} catch (err: unknown) { | ||
onError(err) | ||
debug('postInstanceTests failed, allowing browser to hang until it is killed: Error %o', { err }) | ||
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 marked this conversation as resolved.
Show resolved
Hide resolved
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. Bug: Error Handling Fails to Return Early After API FailureThe refactored error handling for |
||
} | ||
|
||
return cb(response) | ||
}) | ||
|
||
return runAllSpecs({ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,13 @@ | |
"private": true, | ||
"main": "index.js", | ||
"scripts": { | ||
"build-prod": "tsc || echo 'built, with type errors'", | ||
"check-ts": "tsc --noEmit", | ||
"build-prod": "yarn ensure-cloud-validations && tsc || echo 'built, with type errors'", | ||
"check-ts": "yarn ensure-cloud-validations && tsc --noEmit", | ||
"clean-deps": "rimraf node_modules", | ||
"codecov": "codecov", | ||
"dev": "node index.js", | ||
"docker": "cd ../.. && WORKING_DIR=/packages/server ./scripts/run-docker-local.sh", | ||
"postinstall": "patch-package", | ||
"postinstall": "patch-package && yarn sync-cloud-validations", | ||
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. if we are doing this at the package level, do we still need to call it in CI after the build? Would be nice to get rid of this https://github.com/cypress-io/cypress/blob/develop/.circleci/src/pipeline/@pipeline.yml#L201 |
||
"lint": "eslint", | ||
"rebuild-better-sqlite3": "electron-rebuild -f -o better-sqlite3", | ||
"repl": "node repl.js", | ||
|
@@ -19,7 +19,9 @@ | |
"test-integration": "node ./test/scripts/run.js --glob-in-dir=test/integration", | ||
"test-performance": "node ./test/scripts/run.js --glob-in-dir=test/performance", | ||
"test-unit": "node ./test/scripts/run.js --glob-in-dir=test/unit", | ||
"test-watch": "./test/scripts/watch test" | ||
"test-watch": "./test/scripts/watch test", | ||
"sync-cloud-validations": "./scripts/sync-cloud-validations.sh sync", | ||
"ensure-cloud-validations": "./scripts/sync-cloud-validations.sh" | ||
}, | ||
"dependencies": { | ||
"@babel/parser": "7.28.0", | ||
|
@@ -211,7 +213,8 @@ | |
"tsconfig-paths": "3.10.1", | ||
"webpack": "^5.88.2", | ||
"ws": "5.2.4", | ||
"xvfb-maybe": "0.2.1" | ||
"xvfb-maybe": "0.2.1", | ||
"zod": "^3.23.8" | ||
}, | ||
"files": [ | ||
"config", | ||
|
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.
do we no longer need this logic?