chore: add better-sqlite3 dependency #26168
Conversation
29 flaky tests on run #44926 ↗︎
Details:
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| network stubbing > intercepting request > can delay and throttle a StaticResponse |
Output
|
|
cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox
| Test | Artifacts | |
|---|---|---|
| ... > correctly returns currentRetry |
Output
|
|
| ... > correctly returns currentRetry |
Output
|
|
| ... > correctly returns currentRetry |
Output
|
|
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron
| Test | Artifacts | |
|---|---|---|
| ... > stops waiting when an fetch request is canceled |
Output
Video
|
|
cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron
| Test | Artifacts | |
|---|---|---|
| ... > correctly returns currentRetry |
Output
Video
|
|
| ... > correctly returns currentRetry |
Output
Video
|
|
| ... > correctly returns currentRetry |
Output
Video
|
|
create-from-component.cy.ts • 1 flaky test • app-e2e
| Test | Artifacts | |
|---|---|---|
| ... > runs generated spec |
Output
Screenshots
Video
|
|
The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud.
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
| @@ -19,9 +20,12 @@ const setupProtocol = async (url?: string): Promise<AppCaptureProtocolInterface | |||
| } | |||
|
|
|||
| if (script) { | |||
| const cypressProtocolDirectory = path.join(os.tmpdir(), 'cypress', 'protocol') | |||
|
|
|||
| await fs.ensureDir(cypressProtocolDirectory) | |||
There was a problem hiding this comment.
I feel like I've seen file permissions issues before (maybe just docker? worth adding a try-catch to safely bail if we fail here?
There was a problem hiding this comment.
I think this path will have several errors associated with it that we will be handling later (it's being caught one level higher right now). I'll add a TODO here to make sure this gets addressed when we dig into error handling for initializing the protocol.
There was a problem hiding this comment.
Addressed here: 74784cd (#26168)
| @@ -37,6 +38,7 @@ | |||
| "ansi_up": "5.0.0", | |||
| "ast-types": "0.13.3", | |||
| "base64url": "^3.0.1", | |||
| "better-sqlite3": "8.2.0", | |||
There was a problem hiding this comment.
typically we document dependency bumps. Should this have a changelog entry to mention we added this?
There was a problem hiding this comment.
Addressed here: 74784cd (#26168)
packages/server/test/support/fixtures/cloud/protocol/test-protocol.js
Outdated
Show resolved
Hide resolved
| @@ -4,6 +4,7 @@ | |||
| "private": true, | |||
| "main": "index.js", | |||
| "scripts": { | |||
| "build": "electron-rebuild -o better-sqlite3", | |||
There was a problem hiding this comment.
How come you didn't update the build-prod script to call yarn build? It seems this would remove the need to update the scripts/binary/build.ts script. Is there a reason to do it as a post-install when building the production binary? If there is a reason, can we add a comment in the scripts/binary/build.ts file?
There was a problem hiding this comment.
I tried what you suggested first and unfortunately that didn't work. In the binary build we actually do a fresh yarn install with a very minimal postinstall step that does not run build or build-prod so anything we would have done in build-prod earlier would be overridden at that point.
There was a problem hiding this comment.
It seems like the rule might be: anything we do to the node_modules dependencies themselves needs to be done here. This includes patching packages and rebuilding native bindings.
There was a problem hiding this comment.
👍🏻 Makes sense. I assume you've found that via trial and error 😅
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
scripts/binary/build.ts
Outdated
| fs.writeJsonSync(meta.distDir('package.json'), { | ||
| ...packageJsonContents, | ||
| scripts: { | ||
| postinstall: 'patch-package', | ||
| postinstall: 'patch-package && yarn workspace @packages/server build', | ||
| }, | ||
| }, { spaces: 2 }) |
There was a problem hiding this comment.
add comment that this ensure a correct post-install and the native bindings are re-built for the machine per https://github.com/cypress-io/cypress/pull/26168/files#r1144897619
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
This PR coincides with the services PR here: https://github.com/cypress-io/cypress-services/pull/5467
This PR adds
better-sqlite3as a dependency.The dependency needs to be built for the proper version of electron that Cypress uses, so we set up the rebuild process on post install and binary build.
Steps to test
cypress-servicesrepo and checkoutryanm/chore/better-sqlite3yarnyarn watchinpackages/app-capture-protocolCYPRESS_LOCAL_PROTOCOL_PATHto the path tocypress-services/packages/app-capture-protocol/dist/index.jscypressrepo and checkoutryanm/chore/better-sqlite3yarncypress runon a project in record mode withDEBUGset tocypress:*protocol*and ensure you can see the lifecycle eventsHow has the user experience changed?
n/a
PR Tasks
cypress-documentation?type definitions?