Skip to content

Commit

Permalink
refactor: rename the flag, fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
AndreyBelym committed Sep 27, 2022
1 parent d265db6 commit c56213f
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const IN_DOCKER_FLAGS = ['--no-sandbox', '--disable-dev-shm-usage'];
export const CONTAINERIZED_CHROME_FLAGS = ['--no-sandbox', '--disable-dev-shm-usage'];

export function buildChromeArgs ({ config, cdpPort, platformArgs, tempProfileDir, inContainer }) {
export function buildChromeArgs ({ config, cdpPort, platformArgs, tempProfileDir, isContainerized }) {
let chromeArgs = []
.concat(
cdpPort ? [`--remote-debugging-port=${cdpPort}`] : [],
Expand All @@ -11,10 +11,10 @@ export function buildChromeArgs ({ config, cdpPort, platformArgs, tempProfileDir
)
.join(' ');

if (inContainer) {
IN_DOCKER_FLAGS.forEach(inDockerFlag => {
if (!chromeArgs.includes(inDockerFlag))
chromeArgs = chromeArgs.concat(' ', inDockerFlag);
if (isContainerized) {
CONTAINERIZED_CHROME_FLAGS.forEach(flag => {
if (!chromeArgs.includes(flag))
chromeArgs = chromeArgs.concat(' ', flag);
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/browser/provider/built-in/dedicated/chrome/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default {
};

//NOTE: A not-working tab is opened when the browser start in the docker so we should create a new tab.
if (runtimeInfo.inContainer)
if (runtimeInfo.isContainerized)
await startLocalChromeOnDocker(pageUrl, runtimeInfo);
else
await startLocalChrome(pageUrl, runtimeInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ const browserStarter = new BrowserStarter();
const LIST_TABS_TIMEOUT = 10000;
const LIST_TABS_DELAY = 500;

export async function start (pageUrl, { browserName, config, cdpPort, tempProfileDir, inDocker }) {
export async function start (pageUrl, { browserName, config, cdpPort, tempProfileDir, isContainerized }) {
const chromeInfo = await browserTools.getBrowserInfo(config.path || browserName);
const chromeOpenParameters = Object.assign({}, chromeInfo);

chromeOpenParameters.cmd = buildChromeArgs({ config, cdpPort, platformArgs: chromeOpenParameters.cmd, tempProfileDir, inDocker });
chromeOpenParameters.cmd = buildChromeArgs({ config, cdpPort, platformArgs: chromeOpenParameters.cmd, tempProfileDir, isContainerized });

await browserStarter.startBrowser(chromeOpenParameters, pageUrl);
}
Expand All @@ -29,8 +29,8 @@ async function tryListTabs (cdpPort) {
}
}

export async function startOnDocker (pageUrl, { browserName, config, cdpPort, tempProfileDir, inDocker }) {
await start('', { browserName, config, cdpPort, tempProfileDir, inDocker });
export async function startOnDocker (pageUrl, { browserName, config, cdpPort, tempProfileDir, isContainerized }) {
await start('', { browserName, config, cdpPort, tempProfileDir, isContainerized });

let { tabs, error } = await tryListTabs(cdpPort);
const timer = new Timer(LIST_TABS_TIMEOUT);
Expand Down
10 changes: 5 additions & 5 deletions src/browser/provider/built-in/dedicated/chrome/runtime-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ export default class ChromeRuntimeInfo {
public config: Config;
public tempProfileDir: null | TempDirectory;
public cdpPort: number;
public inContainer: boolean;
public isContainerized: boolean;
public browserName?: string;

protected constructor (config: Config) {
this.config = config;
this.tempProfileDir = null;
this.cdpPort = this.config.cdpPort;
this.inContainer = isDocker() || isPodman();
this.config = config;
this.tempProfileDir = null;
this.cdpPort = this.config.cdpPort;
this.isContainerized = isDocker() || isPodman();
}

protected async createTempProfile (proxyHostName: string, disableMultipleWindows: boolean): Promise<TempDirectory> {
Expand Down
32 changes: 19 additions & 13 deletions test/server/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -589,23 +589,29 @@ describe('Utils', () => {
const IN_DOCKER_FLAGS_RE = new RegExp(IN_DOCKER_FLAGS.join(' '));
const SANDBOX_FLAG_RE = new RegExp('--no-sandbox');
const DISABLE_DEV_SHM_USAGE_RE = new RegExp('--disable-dev-shm-usage');
let inDockerFlagMatch = null;

chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, inDocker: false });
inDockerFlagMatch = chromeArgs.match(IN_DOCKER_FLAGS_RE);
expect(inDockerFlagMatch).eql(null);
let containerizedChromeFlags = null;

chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, inDocker: true });
inDockerFlagMatch = chromeArgs.match(IN_DOCKER_FLAGS_RE);
expect(inDockerFlagMatch.length).eql(1);
chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, isContainerized: false });
containerizedChromeFlags = chromeArgs.match(IN_DOCKER_FLAGS_RE);

expect(containerizedChromeFlags).eql(null);

chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, isContainerized: true });
containerizedChromeFlags = chromeArgs.match(IN_DOCKER_FLAGS_RE);

expect(containerizedChromeFlags.length).eql(1);

// NOTE: Flag should not be duplicated
config.userArgs = '--no-sandbox --disable-dev-shm-usage';
chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, inDocker: true });
inDockerFlagMatch = chromeArgs.match(SANDBOX_FLAG_RE);
expect(inDockerFlagMatch.length).eql(1);
inDockerFlagMatch = chromeArgs.match(DISABLE_DEV_SHM_USAGE_RE);
expect(inDockerFlagMatch.length).eql(1);
config.userArgs = '--no-sandbox --disable-dev-shm-usage';
chromeArgs = buildChromeArgs({ config, cdpPort, platformArgs, tempProfileDir, isContainerized: true });
containerizedChromeFlags = chromeArgs.match(SANDBOX_FLAG_RE);

expect(containerizedChromeFlags.length).eql(1);

containerizedChromeFlags = chromeArgs.match(DISABLE_DEV_SHM_USAGE_RE);

expect(containerizedChromeFlags.length).eql(1);
});

describe('Create temporary profile for the Google Chrome browser', () => {
Expand Down

0 comments on commit c56213f

Please sign in to comment.