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.

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

Enhancements to the PhantomJSDriverService to support passing command-line arguments to PhantomJS.exe #13

Conversation

gmcelhanon
Copy link

Changed driver service properties to use non-nullable types, added JSON.NET serialization properties to embed knowledge of PhantomJS command-line argument default values, fleshed out driver service unit tests, and made processing of command-line arguments dynamic based on PhantomJSDriverService property reflection, easing addition of new arguments in the future.

@jimevans
Copy link
Member

jimevans commented Mar 5, 2013

There are a number of things here that I would change. My changes to PhantomJSDriver.cs would not be nearly so extensive. I'd add a new attribute that allows you to specify what the argument name will be on the PhantomJS command line so as to decouple it from the JSON property name (there's a recognizable pattern to the names now, but there's no guarantee it will remain that way). I'd reorganize the code to correctly pass FxCop and StyleCop analysis. I have a version of this pull request that is reorganized in this way which I'm happy to commit, but it wouldn't specifically be under your name. Would you like to take another crack at it, or do you want me to commit my version (giving you attribution in the commit message)?

@gmcelhanon
Copy link
Author

If you haven’t already committed it, I’d say go ahead. I really don’t have any more time to allocate to this. If you want to give me a shout out in the commit message, that would be nice.

@jimevans
Copy link
Member

jimevans commented Apr 5, 2013

Applied modified pull request in c8bc1dd

@jimevans jimevans closed this Apr 5, 2013
IlyaNaumovich added a commit to IlyaNaumovich/selenium that referenced this pull request Feb 11, 2018
yiming-tang-cs pushed a commit to ponder-lab/selenium that referenced this pull request Jan 2, 2020
If an exception is raised during the handshake we close the session
so the client can retry the handshake later with the other credentials.

Also Fix SeleniumHQ#13 : closed sessions are not removed from the maps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants