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: custom reporter and experimentalSessionAndOrigin crashes #24630

Merged
merged 14 commits into from Nov 10, 2022
Merged
10 changes: 5 additions & 5 deletions .circleci/config.yml
Expand Up @@ -27,7 +27,7 @@ mainBuildFilters: &mainBuildFilters
branches:
only:
- develop
- 'zachw/fix-vite-hoisting-issue'
- 'ryanm/fix/improve-binary-cleanup'

# 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
Expand All @@ -36,7 +36,7 @@ macWorkflowFilters: &darwin-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'zachw/fix-vite-hoisting-issue', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/improve-binary-cleanup', << pipeline.git.branch >> ]
- matches:
pattern: "-release$"
value: << pipeline.git.branch >>
Expand All @@ -45,7 +45,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'zachw/fix-vite-hoisting-issue', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/improve-binary-cleanup', << pipeline.git.branch >> ]
- matches:
pattern: "-release$"
value: << pipeline.git.branch >>
Expand All @@ -63,7 +63,7 @@ windowsWorkflowFilters: &windows-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'zachw/fix-vite-hoisting-issue', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/improve-binary-cleanup', << pipeline.git.branch >> ]
- matches:
pattern: "-release$"
value: << pipeline.git.branch >>
Expand Down Expand Up @@ -130,7 +130,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "zachw/fix-vite-hoisting-issue" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "ryanm/fix/improve-binary-cleanup" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
2 changes: 1 addition & 1 deletion npm/webpack-preprocessor/index.ts
Expand Up @@ -249,7 +249,7 @@ const preprocessor: WebpackPreprocessor = (options: PreprocessorOptions = {}): F
webpackOptions.module.rules.unshift({
test: /\.(js|ts|jsx|tsx)$/,
use: [{
loader: path.join(__dirname, 'lib/cross-origin-callback-loader'),
loader: require.resolve('@cypress/webpack-preprocessor/dist/lib/cross-origin-callback-loader.js'),
options: {
commands: callbackReplacementCommands,
},
Expand Down
1 change: 1 addition & 0 deletions npm/webpack-preprocessor/package.json
Expand Up @@ -20,6 +20,7 @@
"watch": "rimraf dist && tsc --watch"
},
"dependencies": {
"@babel/parser": "7.13.0",
"bluebird": "3.7.1",
"debug": "^4.3.2",
"fs-extra": "^10.1.0",
Expand Down
51 changes: 28 additions & 23 deletions scripts/binary/binary-cleanup.js
Expand Up @@ -36,22 +36,23 @@ async function removeEmptyDirectories (directory) {
}
}

const getDependencyPathsToKeep = async () => {
const getDependencyPathsToKeep = async (buildAppDir) => {
const unixBuildAppDir = buildAppDir.split(path.sep).join(path.posix.sep)
const startingEntryPoints = [
'packages/server/index.js',
'packages/server/hook-require.js',
'packages/server/lib/plugins/child/require_async_child.js',
'packages/server/lib/plugins/child/register_ts_node.js',
'packages/rewriter/lib/threads/worker.js',
'node_modules/webpack/lib/webpack.js',
'node_modules/webpack-dev-server/lib/Server.js',
'node_modules/html-webpack-plugin-4/index.js',
'node_modules/html-webpack-plugin-5/index.js',
'node_modules/mocha-7.0.1/index.js',
]

let entryPoints = new Set([
// This is the entry point for the server bundle. It will not have access to the snapshot yet. It needs to be kept in the binary
require.resolve('@packages/server/index.js'),
// This is a dynamic import that is used to load the snapshot require logic. It will not have access to the snapshot yet. It needs to be kept in the binary
require.resolve('@packages/server/hook-require.js'),
// These dependencies are started in a new process or thread and will not have access to the snapshot. They need to be kept in the binary
require.resolve('@packages/server/lib/plugins/child/require_async_child.js'),
require.resolve('@packages/server/lib/plugins/child/register_ts_node.js'),
require.resolve('@packages/rewriter/lib/threads/worker.ts'),
// These dependencies use the `require.resolve(<dependency>, { paths: [<path>] })` pattern where <path> is a path within the cypress monorepo. These will not be
// pulled in by esbuild but still need to be kept in the binary.
require.resolve('webpack'),
require.resolve('webpack-dev-server', { paths: [path.join(__dirname, '..', '..', 'npm', 'webpack-dev-server')] }),
require.resolve('html-webpack-plugin-4', { paths: [path.join(__dirname, '..', '..', 'npm', 'webpack-dev-server')] }),
require.resolve('html-webpack-plugin-5', { paths: [path.join(__dirname, '..', '..', 'npm', 'webpack-dev-server')] }),
...startingEntryPoints.map((entryPoint) => path.join(unixBuildAppDir, entryPoint)),
// These dependencies are completely dynamic using the pattern `require(`./${name}`)` and will not be pulled in by esbuild but still need to be kept in the binary.
...['ibmi',
'sunos',
Expand All @@ -61,7 +62,7 @@ const getDependencyPathsToKeep = async () => {
'linux',
'openbsd',
'sunos',
'win32'].map((platform) => require.resolve(`default-gateway/${platform}`)),
'win32'].map((platform) => path.join(unixBuildAppDir, `node_modules/default-gateway/${platform}.js`)),
])
let esbuildResult
let newEntryPointsFound = true
Expand All @@ -77,8 +78,10 @@ const getDependencyPathsToKeep = async () => {
outdir: workingDir,
platform: 'node',
metafile: true,
absWorkingDir: unixBuildAppDir,
external: [
'./packages/server/server-entry',
'./transpile-ts',
'./server-entry',
'fsevents',
'pnpapi',
'@swc/core',
Expand All @@ -95,9 +98,9 @@ const getDependencyPathsToKeep = async () => {
let entryPoint

if (warningSubject.startsWith('.')) {
entryPoint = path.join(__dirname, '..', '..', path.dirname(warning.location.file), warningSubject)
entryPoint = path.join(unixBuildAppDir, path.dirname(warning.location.file), warningSubject)
} else {
entryPoint = require.resolve(warningSubject)
entryPoint = require.resolve(warningSubject, { paths: [path.join(unixBuildAppDir, path.dirname(warning.location.file))] })
}

if (path.extname(entryPoint) !== '' && !entryPoints.has(entryPoint)) {
Expand All @@ -114,16 +117,18 @@ const getDependencyPathsToKeep = async () => {
const cleanup = async (buildAppDir) => {
// 1. Retrieve all dependencies that still need to be kept in the binary. In theory, we could use the bundles generated here as single files within the binary,
// but for now, we just track on the dependencies that get pulled in
const keptDependencies = [...await getDependencyPathsToKeep(), 'package.json', 'packages/server/server-entry.js']
const keptDependencies = [...await getDependencyPathsToKeep(buildAppDir), 'package.json', 'packages/server/server-entry.js']

// 2. Gather the dependencies that could potentially be removed from the binary due to being in the snapshot
const potentiallyRemovedDependencies = [...snapshotMetadata.healthy, ...snapshotMetadata.deferred, ...snapshotMetadata.norewrite]

// 3. Remove all dependencies that are in the snapshot but not in the list of kept dependencies from the binary
await Promise.all(potentiallyRemovedDependencies.map(async (dependency) => {
// marionette-client requires all of its dependencies in a very non-standard dynamic way. We will keep anything in marionette-client
if (!keptDependencies.includes(dependency.slice(2)) && !dependency.includes('marionette-client')) {
await fs.remove(path.join(buildAppDir, dependency.replace(/.ts$/, '.js')))
const typeScriptlessDependency = dependency.replace(/\.ts$/, '.js')

// marionette-client and babel/runtime require all of their dependencies in a very non-standard dynamic way. We will keep anything in marionette-client and babel/runtime
if (!keptDependencies.includes(typeScriptlessDependency.slice(2)) && !typeScriptlessDependency.includes('marionette-client') && !typeScriptlessDependency.includes('@babel/runtime')) {
await fs.remove(path.join(buildAppDir, typeScriptlessDependency))
}
}))

Expand Down
2 changes: 1 addition & 1 deletion system-tests/scripts/bootstrap-docker-container.sh
Expand Up @@ -39,7 +39,7 @@ export npm_config_package_lock=false
mkdir $npm_config_cache
chown -R 1000:1000 $npm_config_cache

npx npm@latest install --unsafe-perm --allow-root --force file:$CLI_PATH
npx npm@9.1.0 install --unsafe-perm --allow-root --force file:$CLI_PATH

PATH=$PATH:./node_modules/.bin

Expand Down
Expand Up @@ -49,6 +49,8 @@
"./packages/server/lib/modes/record.js",
"./packages/server/lib/modes/run.ts",
"./packages/server/lib/open_project.ts",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/util/suppress_warnings.js",
"./packages/server/node_modules/@benmalka/foxdriver/node_modules/graceful-fs/polyfills.js",
"./packages/server/node_modules/glob/node_modules/minimatch/minimatch.js",
Expand Down Expand Up @@ -844,7 +846,6 @@
"./packages/server/lib/plugins/dev-server.js",
"./packages/server/lib/plugins/preprocessor.js",
"./packages/server/lib/plugins/run_events.js",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/project_utils.ts",
"./packages/server/lib/remote_states.ts",
"./packages/server/lib/reporter.js",
Expand All @@ -857,7 +858,6 @@
"./packages/server/lib/server-ct.ts",
"./packages/server/lib/server-e2e.ts",
"./packages/server/lib/socket-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/socket-e2e.ts",
"./packages/server/lib/unhandled_exceptions.ts",
"./packages/server/lib/util/app_data.js",
Expand Down Expand Up @@ -3929,5 +3929,5 @@
"./tooling/v8-snapshot/cache/prod-darwin/snapshot-entry.js"
],
"deferredHashFile": "yarn.lock",
"deferredHash": "95205f49259fe2d246d45ef15d1499f6e3d1d235d6db892124bbd5423f1ba872"
"deferredHash": "8b71698b89d3804ed712295c20a140cfcd674fa5c3ad9569530282dd6c3e9906"
}
4 changes: 2 additions & 2 deletions tooling/v8-snapshot/cache/prod-linux/snapshot-meta.cache.json
Expand Up @@ -49,6 +49,8 @@
"./packages/server/lib/modes/record.js",
"./packages/server/lib/modes/run.ts",
"./packages/server/lib/open_project.ts",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/util/suppress_warnings.js",
"./packages/server/node_modules/@benmalka/foxdriver/node_modules/graceful-fs/polyfills.js",
"./packages/server/node_modules/glob/node_modules/minimatch/minimatch.js",
Expand Down Expand Up @@ -843,7 +845,6 @@
"./packages/server/lib/plugins/dev-server.js",
"./packages/server/lib/plugins/preprocessor.js",
"./packages/server/lib/plugins/run_events.js",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/project_utils.ts",
"./packages/server/lib/remote_states.ts",
"./packages/server/lib/reporter.js",
Expand All @@ -856,7 +857,6 @@
"./packages/server/lib/server-ct.ts",
"./packages/server/lib/server-e2e.ts",
"./packages/server/lib/socket-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/socket-e2e.ts",
"./packages/server/lib/unhandled_exceptions.ts",
"./packages/server/lib/util/app_data.js",
Expand Down
4 changes: 2 additions & 2 deletions tooling/v8-snapshot/cache/prod-win32/snapshot-meta.cache.json
Expand Up @@ -49,6 +49,8 @@
"./packages/server/lib/modes/record.js",
"./packages/server/lib/modes/run.ts",
"./packages/server/lib/open_project.ts",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/util/suppress_warnings.js",
"./packages/server/node_modules/@benmalka/foxdriver/node_modules/graceful-fs/polyfills.js",
"./packages/server/node_modules/glob/node_modules/minimatch/minimatch.js",
Expand Down Expand Up @@ -846,7 +848,6 @@
"./packages/server/lib/plugins/dev-server.js",
"./packages/server/lib/plugins/preprocessor.js",
"./packages/server/lib/plugins/run_events.js",
"./packages/server/lib/project-base.ts",
"./packages/server/lib/project_utils.ts",
"./packages/server/lib/remote_states.ts",
"./packages/server/lib/reporter.js",
Expand All @@ -859,7 +860,6 @@
"./packages/server/lib/server-ct.ts",
"./packages/server/lib/server-e2e.ts",
"./packages/server/lib/socket-base.ts",
"./packages/server/lib/socket-ct.ts",
"./packages/server/lib/socket-e2e.ts",
"./packages/server/lib/unhandled_exceptions.ts",
"./packages/server/lib/util/app_data.js",
Expand Down
2 changes: 2 additions & 0 deletions tooling/v8-snapshot/src/setup/force-no-rewrite.ts
Expand Up @@ -56,4 +56,6 @@ export default [
'packages/server/node_modules/glob/node_modules/minimatch/minimatch.js',
'node_modules/js-yaml/lib/js-yaml/type/js/function.js',
'packages/server/lib/open_project.ts',
'packages/server/lib/project-base.ts',
'packages/server/lib/socket-ct.ts',
]