Skip to content
This repository was archived by the owner on Apr 17, 2026. It is now read-only.

feat(selenium-grid): run webdriver-manager as a selenium-grid node (merge against master branch) - #386

Open
filipzamorsky wants to merge 13 commits into
angular:masterfrom
filipzamorsky:selenium-grid
Open

feat(selenium-grid): run webdriver-manager as a selenium-grid node (merge against master branch)#386
filipzamorsky wants to merge 13 commits into
angular:masterfrom
filipzamorsky:selenium-grid

Conversation

@filipzamorsky

@filipzamorskyfilipzamorsky commented May 31, 2019

Copy link
Copy Markdown

$webdriver-manager start --gridNode http://localhost:4444/grid/register

@filipzamorskyfilipzamorsky changed the title feat(selenium-grid): Run webdriver-manager as a selenium-grid nodefeat(selenium-grid): Run webdriver-manager as a selenium-grid node (merge against master branch)May 31, 2019
@filipzamorskyfilipzamorsky changed the title feat(selenium-grid): Run webdriver-manager as a selenium-grid node (merge against master branch)feat(selenium-grid): run webdriver-manager as a selenium-grid node (merge against master branch)May 31, 2019
@heathkit
heathkit requested a review from cnishinaJune 5, 2019 18:22

@cnishinacnishina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good so far. I would rebase. There's a few changes that I made

Comment threadREADME.md Outdated
Comment threadlib/cli/index.ts
const gridNodeOption: yargs.Options = {
describe: 'Start the selenium grid with role set to "node".',
type: 'string'
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would rebase. STANDALONE is now SELENIUM and SELENIUM_ALIAS

Could we call this as SELENIUM_GRID_URL?
We should use underscores. So the string would be 'selenium_grid_url'

And have one called SELENIUM_GRID_URL_ALIAS?
And this string would be 'standalone_grid_url'

I am trying to standardize this and add a README for this. I have a few flags to fix that I also need to fix.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok - done

Comment threadlib/cli/index.ts Outdated
.option(SELENIUM_PORT, seleniumPort)
.option(STANDALONE, standaloneOption)
.option(STANDALONE_NODE, standaloneNodeOption)
.option(GRID_NODE, gridNodeOption)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok - done

Comment threadlib/cmds/options.ts Outdated
// Run as role = node option.
runAsNode?: boolean;
// Run as grid node role = hub registration URL path.
gridNode?: string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's call this gridUrl or nodeGridUrl?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok - done

Comment threadlib/cmds/utils.ts Outdated
options.server = {};
options.server.name = 'selenium';
options.server.runAsNode = argv.standalone_node as boolean;
options.server.gridNode = argv.gridNode as string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When grabbing things from the argv, we should use the selenium_grid_url and not the standalone_ alias

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok - done

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@filipzamorsky@cnishina@googlebot