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.

Sign up for GitHub

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

feat: Add support for optional env key to browser launch options #23624

Merged
merged 12 commits into from
Sep 22, 2022
+208 −72
Merged

feat: Add support for optional env key to browser launch options #23624

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7d6e0da
feat: Add support for optional env key to browser launch options
ryanjduffy Aug 30, 2022
38a63d6
combine env option and add warning
ryanjduffy Sep 10, 2022
ecbdfe1
add default env to webkit launch options
ryanjduffy Sep 20, 2022
62ad6d3
Update electron.ts
lmiller1990 Sep 21, 2022
1789849
revert
lmiller1990 Sep 21, 2022
778b2a1
fix tests
lmiller1990 Sep 21, 2022
788154b
fix lint
lmiller1990 Sep 21, 2022
25338aa
update snapshots
ryanjduffy Sep 21, 2022
8300db1
update tests
lmiller1990 Sep 22, 2022
743a7f3
snaps
lmiller1990 Sep 22, 2022
8ce893d
one more snapshot update
ryanjduffy Sep 22, 2022
47149dd
Merge branch 'develop' into add-browser-env
ryanjduffy Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/types/cypress.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5492,6 +5492,7 @@ declare namespace Cypress {
extensions: string[]
preferences: { [key: string]: any }
args: string[]
env: { [key: string]: any }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is essentially a no-op in Electron, should we add an error or warning if someone sets .env with Electron?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I noticed that args seems to be the same so I've added a warning for both of those. Let me know if I've misread and args is used or if there's a different way that is preferred for warning.

}

interface Dimensions {
Expand Down
1 change: 1 addition & 0 deletions cli/types/tests/plugins-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const pluginConfig2: Cypress.PluginConfig = (on, config) => {
browser.displayName // $ExpectType string
options.extensions // $ExpectType string[]
options.args // $ExpectType string[]
options.env // $ExpectType { [key: string]: any }

console.log('launching browser', browser.displayName)
return options
Expand Down
6 changes: 6 additions & 0 deletions packages/errors/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ export const AllCypressErrors = {

This option will not have an effect in ${fmt.off(_.capitalize(browser))}. Tests that rely on web security being disabled will not run as expected.`
},
BROWSER_UNSUPPORTED_LAUNCH_OPTION: (browser: string, options: string[]) => {
return errTemplate`\
Warning: The following browser launch options were provided but are not supported by ${fmt.highlightSecondary(browser)}

${fmt.listItems(options)}`
},
BROWSER_NOT_FOUND_BY_NAME: (browser: string, foundBrowsersStr: string[]) => {
let canarySuffix: PartialErr | null = null

Expand Down
6 changes: 4 additions & 2 deletions packages/launcher/lib/browsers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export function launch (
url: string,
debuggingPort: number,
args: string[] = [],
defaultBrowserEnv = {},
browserEnv = {},
) {
debug('launching browser %o', { browser, url })

Expand All @@ -181,7 +181,9 @@ export function launch (

// allow setting default env vars such as MOZ_HEADLESS_WIDTH
// but only if it's not already set by the environment
const env = Object.assign({}, defaultBrowserEnv, process.env)
const env = Object.assign({}, browserEnv, process.env)

debug('spawning browser with environment %o', { env })

const proc = cp.spawn(browser.path, args, { stdio: ['ignore', 'pipe', 'pipe'], env })

Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/browsers/chrome.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export = {
// first allows us to connect the remote interface,
// start video recording and then
// we will load the actual page
const launchedBrowser = await launch(browser, 'about:blank', port, args) as unknown as BrowserInstance & { browserCriClient: BrowserCriClient}
const launchedBrowser = await launch(browser, 'about:blank', port, args, launchOptions.env) as unknown as BrowserInstance & { browserCriClient: BrowserCriClient}

la(launchedBrowser, 'did not get launched browser instance')

Expand Down
14 changes: 14 additions & 0 deletions packages/server/lib/browsers/electron.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,18 @@ export = {
throw new Error('Attempting to connect to existing browser for Cypress in Cypress which is not yet implemented for electron')
},

validateLaunchOptions (launchOptions: typeof utils.defaultLaunchOptions) {
const options: string[] = []

if (Object.keys(launchOptions.env).length > 0) options.push('env')

if (launchOptions.args.length > 0) options.push('args')

if (options.length > 0) {
errors.warning('BROWSER_UNSUPPORTED_LAUNCH_OPTION', 'electron', options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanjduffy sorry last comment here before approval... how can I actually pass something to this? I want to verify this error actually shows up, but I couldn't figure out how.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this should do it!

      on("before:browser:launch", (browser, launchOptions) => {
        launchOptions.env = {
          DEBUG: "whatever",
        };

        return launchOptions;
      });

image

}
},

async open (browser: Browser, url: string, options: BrowserLaunchOpts, automation: Automation) {
debug('open %o', { browser, url })

Expand All @@ -486,6 +498,8 @@ export = {

const launchOptions = await utils.executeBeforeBrowserLaunch(browser, defaultLaunchOptions, electronOptions)

this.validateLaunchOptions(launchOptions)

const { preferences } = launchOptions

debug('launching browser window to url: %s', url)
Expand Down
1 change: 1 addition & 0 deletions packages/server/lib/browsers/firefox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
// user can overwrite this default with these env vars or --height, --width arguments
MOZ_HEADLESS_WIDTH: '1280',
MOZ_HEADLESS_HEIGHT: '721',
...launchOptions.env,
})

try {
Expand Down
2 changes: 2 additions & 0 deletions packages/server/lib/browsers/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ const defaultLaunchOptions: {
preferences: {[key: string]: any}
extensions: string[]
args: string[]
env: {[key: string]: any}
} = {
preferences: {},
extensions: [],
args: [],
env: {},
}

const KNOWN_LAUNCH_OPTION_PROPERTIES = _.keys(defaultLaunchOptions)
Expand Down
1 change: 1 addition & 0 deletions packages/server/lib/browsers/webkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
},
extensions: [],
args: [],
env: {},
}

const launchOptions = await utils.executeBeforeBrowserLaunch(browser, defaultLaunchOptions, options)
Expand Down