Allow Java options when starting a standalone selenium server in Ruby#3444
Allow Java options when starting a standalone selenium server in Ruby#3444aliking wants to merge 3 commits intoSeleniumHQ:masterfrom animoto:rb_server_java_options
Conversation
The existing server allows additional args to be passed in, but they are only placed at the end of the constructed command line. For any options to the `java` command, they need to be placed before the `-jar` switch. This pulls out any additional args that start with `-D` and put them in the right place in the command.
Changes a check in gemspec that the __FILE__ location is the same as the pwd. This will probably be unnecessary for most people, but was failing on my jenkins server, due to mounted directories. `File.realpath` always returns the full, actual path for comparison.
|
Just out of curiosity, why are you starting standalone with ruby? |
titusfortner
left a comment
titusfortner
left a comment
There was a problem hiding this comment.
I'm good with adding these in, but I'm curious the use case. This should only be necessary when running on a local machine, and at that point you're better off using the Ruby bindings directly.
|
|
||
| root = File.expand_path(File.dirname(__FILE__)) | ||
| raise "cwd must be #{root} when reading gemspec" if root != Dir.pwd | ||
| root = File.realpath(File.dirname(__FILE__)) |
There was a problem hiding this comment.
What is this giving you that #expand_path does not? Do you have some kind of symlink set up?
rb/selenium-webdriver.gemspec
Outdated
| Gem::Specification.new do |s| | ||
| s.name = 'selenium-webdriver' | ||
| s.version = '3.0.5' | ||
| s.version = '3.0.6' |
There was a problem hiding this comment.
Don't include this in the PR, please. We'll bump it when we're ready to do another release.
rb/lib/selenium/server.rb
Outdated
| cp = ChildProcess.build('java', '-jar', @jar, '-port', @port.to_s, *@additional_args) | ||
| # extract any additional_args that start with -D as options | ||
| options = if @additional_args | ||
| @additional_args.dup - @additional_args.delete_if{|arg| arg.match(/^-D.*$/)} |
There was a problem hiding this comment.
@additional_args is always going to return true since it defaults to []
let's name it properties or system_properties, and we should avoid using #match where possible. Otherwise this is good. :)
I think my preference would be:
properties = @additional_args.dup - @additional_args.delete_if { |arg| arg[/^-D/] }
|
Addressed comments. Thanks for being so responsive. These are both a bit unique to our setup. Our jenkins box has two levels of symlinking on the jenkins home directory - with The reason that we're starting Selenium server in this way is down to a framework that we're using which connects to the selenium server as though it is remote regardless of where it is. I've considered changing it, but it keeps other code consistent. |
|
Merged into master, will be available on next release. @aliking - is this an open source framework, or something custom for your company? Thanks for this contribution. |
|
It's https://github.com/chemistrykit/chemistrykit and https://github.com/chemistrykit/selenium-connect. At this point our fork has diverged a lot from that, but I'm less concerned since it's not being updated. I'm trying to unpick some of the dependencies on small customizations on old forks of other things, so I'm super glad to not have to do that with selenium. Thanks again! |
|
Ah Dave's framework. The different chemistry terms confused me too much, and it looked like it replicated a lot of test runner things that I didn't see use for. So your fork is not open source? |
|
@aliking - Bad news on the @p0deje - I think I need to revert this before the next release. We could theoretically work around the JRuby build process for this, but that way lies madness. I'm not sure how close you were to figuring out the updated JRuby, and now that we have things working in Buck with what is there it isn't any kind of a priority any longer. Any other thoughts? You can see the error with: |
|
@titusfortner I've spent a couple of hours trying to upgrade JRuby once again, but it's much more complicated than it was before because it now uses ChildProcess inside :( I can definitely figure it out but I don't want to do this again if it's not going to be merged like with #2210. Need your advice on this :) |
|
Thanks @p0deje, I went ahead and reverted this change for now. I was the one who pointed @shs96c to use ChildProcess. The issue had to do with something he needed in a standard library that wasn't working with our legacy JRuby (I'd have to dig through conversations from last summer to refresh my memory). Let's come up with a plan for this when we're all face to face at the Selenium Conference in April. |
Sounds reasonable. |
4 participants
Existing server allows additional args to be passed in, but they are only placed at the end of the constructed command line. For any options to the
javacommand, they need to be placed before the-jarswitch.This pulls out any additional args that start with
-Dand put them in the right place.Also includes a change to the gemspec checking that the FILE location is the same as the pwd. This will probably be unnecessary form most people, but was failing on my jenkins server, due to mounted directories.
Xin the preceding checkbox, I verify that I have signed the Contributor License Agreement