-
Notifications
You must be signed in to change notification settings - Fork 669
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
Start browsers with clean profile by default #1792
Start browsers with clean profile by default #1792
Conversation
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.
allow to set a custom profile via --user-data-dir
or -profile
flags (testcafe "firefox -profile MyProfile" test.js
). We've discussed it in private
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.
nice job! a few minor remarks
if (config.path) | ||
firefoxInfo = await browserTools.getBrowserInfo(config.path); | ||
else | ||
firefoxInfo = await browserTools.getBrowserInfo(browserName); |
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.
firefoxInfo = await browserTools.getBrowserInfo(config.path || browserName);
return match && match !== '0' && match !== 'false'; | ||
} | ||
|
||
export function useSystemProfile (array) { |
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.
it seems this is redundant
|
||
|
||
const HEADLESS_DEFAULT_WIDTH = 1280; | ||
const HEADLESS_DEFAULT_HEIGHT = 800; | ||
|
||
const CONFIG_TERMINATOR_RE = /(\s+|^)-/; | ||
const MODES_LIST = ['userProfile', 'headless', 'emulation']; |
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.
I feel MODE_LIST
is more correctly.
But maybe it's more appropriate to use AVAILABLE_MODES
here
return result; | ||
} | ||
|
||
export function getPathFromParsedModes (modesList, possibleModes = []) { |
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.
I'd like to use modes, availableModes = []
here
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.
modes, availableModes = []
|
||
|
||
const CONFIG_TERMINATOR_RE = /(\s+|^)-/; | ||
const SYSTEM_PROFILE_OPTION_RE = /^useSystemProfile=(.*)$/; |
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.
We've changed it to userProfile? Am I right?
❌ Tests for the commit 7ca7b33 have failed. See details: |
❌ Tests for the commit 031e55a have failed. See details: |
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.
Rebase to the upsteram to fix the client tests
return result; | ||
} | ||
|
||
export function getPathFromParsedModes (modesList, possibleModes = []) { |
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.
modes, availableModes = []
return path; | ||
} | ||
|
||
export function getModes (modesList, availableModes = []) { |
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.
modes, availableModes = []
❌ Tests for the commit 031e55a have failed. See details: |
031e55a
to
8c6d128
Compare
\cc @AlexanderMoskovkin @miherlosev
closes #1623