Skip to content

Commit c712e76

Browse files
AtofStrykerAtofStryker
committed
chore: rework electron switches to avoid unknown side effects
how bad does this fail? fix server tests
1 parent c6ddc0c commit c712e76

File tree

9 files changed

+351
-264
lines changed

9 files changed

+351
-264
lines changed

‎packages/server/lib/append_electron_switches.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import os from 'os'
2+
import debugModule from 'debug'
3+
import { DEFAULT_ELECTRON_FLAGS } from './util/chromium_flags'
4+
5+
const debug = debugModule('cypress:server:append_electron_switches')
6+
7+
export const appendElectronSwitches = (app: Electron.App) => {
8+
// NOTE: errors are printed in development mode only
9+
try {
10+
// when running inside the electron process, we need to append the default switches immediately
11+
// before the electron browser is launched. Otherwise, there may be some odd behavior.
12+
debug('appending default switches for electron: %o', DEFAULT_ELECTRON_FLAGS)
13+
DEFAULT_ELECTRON_FLAGS.forEach(({ name, value }) => {
14+
value ? app.commandLine.appendSwitch(name, value) : app.commandLine.appendSwitch(name)
15+
})
16+
17+
if (os.platform() === 'linux') {
18+
app.disableHardwareAcceleration()
19+
}
20+
21+
if (process.env.ELECTRON_EXTRA_LAUNCH_ARGS) {
22+
// regex will be used to convert ELECTRON_EXTRA_LAUNCH_ARGS into an array, for example
23+
// input: 'foo --ipsum=0 --bar=--baz=quux --lorem="--ipsum=dolor --sit=amet"'
24+
// output: ['foo', '--ipsum=0', '--bar=--baz=quux', '--lorem="--ipsum=dolor --sit=amet"']
25+
const regex = /(?:[^\s"']+|"[^"]*"|'[^']*')+/g
26+
const electronLaunchArguments = process.env.ELECTRON_EXTRA_LAUNCH_ARGS.match(regex) || []
27+
28+
electronLaunchArguments.forEach((arg) => {
29+
// arg can be just key --disable-http-cache
30+
// or key value --remote-debugging-port=8315
31+
// or key value with another value --foo=--bar=4196
32+
// or key value with another multiple value --foo='--bar=4196 --baz=quux'
33+
const [key, ...value] = arg.split('=')
34+
35+
// because this is an environment variable, everything is a string
36+
// thus we don't have to worry about casting
37+
// --foo=false for example will be "--foo", "false"
38+
if (value.length) {
39+
let joinedValues = value.join('=')
40+
41+
// check if the arg is wrapped in " or ' (unicode)
42+
const isWrappedInQuotes = !!['\u0022', '\u0027'].find(((charAsUnicode) => joinedValues.startsWith(charAsUnicode) && joinedValues.endsWith(charAsUnicode)))
43+
44+
if (isWrappedInQuotes) {
45+
joinedValues = joinedValues.slice(1, -1)
46+
}
47+
48+
app.commandLine.appendSwitch(key, joinedValues)
49+
} else {
50+
app.commandLine.appendSwitch(key)
51+
}
52+
})
53+
}
54+
} catch (e) {
55+
debug('environment error %s', e.message)
56+
}
57+
}

‎packages/server/lib/cypress.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
require('./environment')
2-
31
// we are not requiring everything up front
42
// to optimize how quickly electron boots while
53
// in dev or linux production. the reasoning is

‎packages/server/lib/environment.js‎

Lines changed: 0 additions & 93 deletions
This file was deleted.

‎packages/server/lib/environment.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import pkg from '@packages/root'
2+
import Bluebird from 'bluebird'
3+
4+
export const configureLongStackTraces = (env: string | undefined) => {
5+
// never cut off stack traces
6+
Error.stackTraceLimit = Infinity
7+
8+
Bluebird.config({
9+
// uses cancellation for automation timeouts
10+
cancellation: true,
11+
// enable long stack traces in dev
12+
longStackTraces: env === 'development',
13+
})
14+
}
15+
16+
export const calculateCypressInternalEnv = () => {
17+
// instead of setting NODE_ENV we will
18+
// use our own separate CYPRESS_INTERNAL_ENV so
19+
// as not to conflict with CI providers
20+
21+
// use env from package first
22+
// or development as default
23+
if (!process.env['CYPRESS_INTERNAL_ENV']) {
24+
// @ts-expect-error
25+
return pkg.env !== null && pkg.env !== undefined ? pkg.env : 'development'
26+
}
27+
28+
return process.env['CYPRESS_INTERNAL_ENV']
29+
}

‎packages/server/start-cypress.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,25 @@ const { telemetry, OTLPTraceExporterCloud } = require('@packages/telemetry')
33
const { apiRoutes } = require('./lib/cloud/routes')
44
const encryption = require('./lib/cloud/encryption')
55

6+
const { calculateCypressInternalEnv, configureLongStackTraces } = require('./lib/environment')
7+
8+
process.env['CYPRESS_INTERNAL_ENV'] = calculateCypressInternalEnv()
9+
configureLongStackTraces(process.env['CYPRESS_INTERNAL_ENV'])
10+
process.env['CYPRESS'] = 'true'
11+
612
// are we in the main node process or the electron process?
713
const isRunningElectron = electronApp.isRunning()
814

915
const pkg = require('@packages/root')
1016

1117
if (isRunningElectron) {
18+
// if we are in the electron process, we need to patch the electron switches before Cypress launches the app
19+
// @see https://www.electronjs.org/docs/latest/api/environment-variables#electron_run_as_node
20+
const { app } = require('electron')
21+
const { appendElectronSwitches } = require('./lib/append_electron_switches')
22+
23+
appendElectronSwitches(app)
24+
1225
// To pass unencrypted telemetry data to an independent open telemetry endpoint,
1326
// disable the encryption header, update the url, and add any other required headers.
1427
// For example:

‎packages/server/test/spec_helper.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/* eslint-disable no-console */
2-
require('../lib/environment')
3-
42
const { enable, mockElectron } = require('./mockery_helper')
53

4+
const { configureLongStackTraces } = require('../lib/environment')
5+
6+
configureLongStackTraces('development')
67
const chai = require('chai')
78

89
chai.use(require('chai-subset'))

‎packages/server/test/unit/append_electron_switches_spec.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import os from 'os'
2+
import sinon from 'sinon'
3+
import mockedEnv from 'mocked-env'
4+
import { appendElectronSwitches } from '../../lib/append_electron_switches'
5+
6+
describe('lib/append_electron_switches', () => {
7+
beforeEach(() => {
8+
sinon.stub(os, 'platform').returns('linux')
9+
})
10+
11+
afterEach(() => {
12+
sinon.restore()
13+
})
14+
15+
// @see https://github.com/electron/electron/issues/46538
16+
// @see https://github.com/cypress-io/cypress/issues/32361
17+
context('sets gtk-version=3 in Electron >= 36', () => {
18+
it('sets launch args', async () => {
19+
const mockApp = {
20+
commandLine: {
21+
appendSwitch: sinon.stub(),
22+
},
23+
} as unknown as Electron.App
24+
25+
appendElectronSwitches(mockApp)
26+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--gtk-version', '3')
27+
})
28+
})
29+
30+
context('disables hardware acceleration on Linux', () => {
31+
it('disables hardware acceleration', async () => {
32+
const mockApp = {
33+
disableHardwareAcceleration: sinon.stub(),
34+
commandLine: {
35+
appendSwitch: sinon.stub(),
36+
},
37+
} as unknown as Electron.App
38+
39+
appendElectronSwitches(mockApp)
40+
expect(mockApp.disableHardwareAcceleration).to.have.been.called
41+
})
42+
})
43+
44+
context('parses ELECTRON_EXTRA_LAUNCH_ARGS', () => {
45+
let restore = null
46+
47+
afterEach(() => {
48+
if (restore) {
49+
return restore()
50+
}
51+
})
52+
53+
it('sets launch args', async () => {
54+
restore = mockedEnv({
55+
ELECTRON_EXTRA_LAUNCH_ARGS: '--foo --bar=baz --quux=true',
56+
})
57+
58+
const mockApp = {
59+
disableHardwareAcceleration: sinon.stub(),
60+
commandLine: {
61+
appendSwitch: sinon.stub(),
62+
},
63+
} as unknown as Electron.App
64+
65+
appendElectronSwitches(mockApp)
66+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--foo')
67+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--bar', 'baz')
68+
69+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--quux', 'true')
70+
})
71+
72+
it('sets launch args with zero', async () => {
73+
restore = mockedEnv({
74+
ELECTRON_EXTRA_LAUNCH_ARGS: '--foo --bar=baz --quux=0',
75+
})
76+
77+
const mockApp = {
78+
disableHardwareAcceleration: sinon.stub(),
79+
commandLine: {
80+
appendSwitch: sinon.stub(),
81+
},
82+
} as unknown as Electron.App
83+
84+
appendElectronSwitches(mockApp)
85+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--foo')
86+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--bar', 'baz')
87+
88+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--quux', '0')
89+
})
90+
91+
it('sets launch args with false', async () => {
92+
restore = mockedEnv({
93+
ELECTRON_EXTRA_LAUNCH_ARGS: '--foo --bar=baz --quux=false',
94+
})
95+
96+
const mockApp = {
97+
disableHardwareAcceleration: sinon.stub(),
98+
commandLine: {
99+
appendSwitch: sinon.stub(),
100+
},
101+
} as Electron.App
102+
103+
appendElectronSwitches(mockApp)
104+
105+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--foo')
106+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--bar', 'baz')
107+
108+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--quux', 'false')
109+
})
110+
111+
it('sets launch args with multiple values inside quotes', async () => {
112+
restore = mockedEnv({
113+
ELECTRON_EXTRA_LAUNCH_ARGS: `--foo --ipsum=0 --bar=--baz=quux --lorem='--ipsum=dolor --sit=amet'`,
114+
})
115+
116+
const mockApp = {
117+
disableHardwareAcceleration: sinon.stub(),
118+
commandLine: {
119+
appendSwitch: sinon.stub(),
120+
},
121+
} as Electron.App
122+
123+
appendElectronSwitches(mockApp)
124+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--foo')
125+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--ipsum', '0')
126+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--bar', '--baz=quux')
127+
expect(mockApp.commandLine.appendSwitch).to.have.been.calledWith('--lorem', '--ipsum=dolor --sit=amet')
128+
})
129+
})
130+
})

0 commit comments

Comments
 (0)