JRuby support #615

Closed
rupurt opened this Issue Jan 28, 2014 · 11 comments

Projects

None yet

4 participants

@rupurt
Contributor
rupurt commented Jan 28, 2014

We're trying to add JRuby support for capybara-webkit. We've opened PR #614 & PR #613 which fix some low hanging fruit. We have also added JRuby to the travis build on the branches jruby-travis & patches but are waiting for the first 2 PR's to get accepted before submitting this.

With the fixes above we are down to 3 failures and need some help solving.

  1. Killing webkit_server only from the main process - JRuby does not support fork. We can't work out the real world use case of this test, and notice that it's skipped in windows already. Do you think it's dangerous to skip this test for jruby, too?
  2. Forwarding stderr - We're really not sure what's causing this. We tried to use a ::IO object instead of a StringIO object with IO.select rather than the sleep, but the IO object responded as ready for reading before anything had been written to it. We also tried a longer sleep to no avail.
  3. Command Logging - Looks to be the same issue as point 2.
@mhoran mhoran added a commit to mhoran/capybara-webkit that referenced this issue Jan 29, 2014
@mhoran mhoran Read the expected number of bytes from StringIO
* Instead of sleep, use blocking #read to determine if the expected
  message is logged.
* Fixes failure 2 from #615.
3f3ad93
@mhoran mhoran added a commit to mhoran/capybara-webkit that referenced this issue Jan 29, 2014
@mhoran mhoran Use a pipe to test command logging
* JRuby doesn't write to StringIO in the same way as cruby/rbx, and the
  #read method used in connection_spec didn't work here.
* Fixes failure 3 from #615.
7816940
@mhoran
Collaborator
mhoran commented Jan 29, 2014

Thanks for the pull requests! I've merged both #613 and #614.

Killing webkit_server only from the main process

There's no reason to test this on JRuby given that JRuby doesn't support fork. I've fixed this in 2b417de.

Forwarding stderr - We're really not sure what's causing this. We tried to use a ::IO object instead of a StringIO object with IO.select rather than the sleep, but the IO object responded as ready for reading before anything had been written to it. We also tried a longer sleep to no avail.

This was a tough one. I ended up using #read with the expected number of bytes to eliminate both the sleep and the failure on JRuby. Fixed in 3f3ad93.

Command Logging - Looks to be the same issue as point 2.

Good call. For some reason the #read fix used for issue 2 didn't work here, but a pipe worked even better. Fixed in 7816940.

@BrentWheeldon

Nice! Thanks, @mhoran!

@rupurt
Contributor
rupurt commented Jan 29, 2014

Fantastic! That's going to make tomorrow nice and easy :)

Cheers mate!

Sent from Mailbox for iPhone

On Tue, Jan 28, 2014 at 9:29 PM, Matthew Horan notifications@github.com
wrote:

Thanks for the pull requests! I've merged both #613 and #614.

Killing webkit_server only from the main process
There's no reason to test this on JRuby given that JRuby doesn't support fork. I've fixed this in 2b417de.
Forwarding stderr - We're really not sure what's causing this. We tried to use a ::IO object instead of a StringIO object with IO.select rather than the sleep, but the IO object responded as ready for reading before anything had been written to it. We also tried a longer sleep to no avail.
This was a tough one. I ended up using #read with the expected number of bytes to eliminate both the sleep and the failure on JRuby. Fixed in 3f3ad93.
Command Logging - Looks to be the same issue as point 2.

Good call. For some reason the #read fix used for issue 2 didn't work here, but a pipe worked even better. Fixed in 7816940.

Reply to this email directly or view it on GitHub:
#615 (comment)

@BrentWheeldon

Hi @mhoran,

We are now down to 2 failures in spec/connection_spec.rb:

  1. Forwarding stderr's fix that you pushed last night doesn't work for us. Travis is green, so it's something locally, but we're just not sure what it is.
  2. Deprecation warning for stdout option only fails when the whole file is run. This problem was introduced overnight but again only locally, so we're really not sure what could be different. Changing the before(:all) to before(:each) solves this. Is this an acceptable thing to change? Is there some way we can reset @rack_server between runs to make the test more like the real world where the server is not rebooted for every test?

We're using jruby 1.7.9 (which seems to be what travis is using, too).

@mhoran
Collaborator
mhoran commented Jan 29, 2014

Forwarding stderr's fix that you pushed last night doesn't work for us. Travis is green, so it's something locally, but we're just not sure what it is.

It looks like this is a Mac specific issue. I tested last night on Linux (which is what Travis uses) and it passed. I just set up on my Mac at work and it failed. We'll have to poke around the JRuby issues.

Deprecation warning for stdout option only fails when the whole file is run. This problem was introduced overnight but again only locally, so we're really not sure what could be different. Changing the before(:all) to before(:each) solves this. Is this an acceptable thing to change? Is there some way we can reset @rack_server between runs to make the test more like the real world where the server is not rebooted for every test?

I can't get this to fail on a Mac, running 1.7.9 in 1.9 or 2.0 mode.

@samandmoore

Hey all, any update on this issue? I would love to get this running on Mac 👍 Seems like you're really close.

@rupurt
Contributor
rupurt commented May 3, 2014

@samandmoore we got it up and running on JRuby a couple of months ago. Travis is reporting that the JRuby build is working in 1.9 mode. Are you experiencing new problems?

@samandmoore

I'm having issues with 1.9.3 on JRuby 1.7.5 on a Mac. Do you know what
version of JRuby it works with / if it's just not working on Mac?
On May 3, 2014 1:40 PM, "Alex Kwiatkowski" notifications@github.com wrote:

@samandmoore https://github.com/samandmoore we got it up and running on
JRuby a couple of months ago. Travis is reporting that the JRuby build is
working https://travis-ci.org/thoughtbot/capybara-webkit in 1.9 mode.
Are you experiencing new problems?


Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/capybara-webkit/issues/615#issuecomment-42111428
.

@samandmoore

specific error:

Failure/Error: Unable to find matching line from backtrace
TypeError:
    only supports to file or from file copy
# org/jruby/RubyIO.java:4625:in `copy_stream'
# /opt/boxen/rbenv/versions/jruby-1.7.5/lib/ruby/gems/shared/gems/capybara-webkit-1.1.0/lib/capybara/webkit/connection.rb:86:in `forward_output_in_background_thread'

the detailed output says this:

TypeError: no implicit conversion from nil to integer
                    kill at org/jruby/RubyProcess.java:971
                    kill at org/jruby/RubyProcess.java:886
            kill_process at /opt/boxen/rbenv/versions/jruby-1.7.5/lib/ruby/gems/shared/gems/capybara-webkit-1.1.0/lib/capybara/webkit/connection.rb:71
  register_shutdown_hook at /opt/boxen/rbenv/versions/jruby-1.7.5/lib/ruby/gems/shared/gems/capybara-webkit-1.1.0/lib/capybara/webkit/connection.rb:62
@samandmoore

it appears that i missed this line from above:

I can't get this to fail on a Mac, running 1.7.9 in 1.9 or 2.0 mode.

sounds like i just need to use a newer version of JRuby. I will see if that works. Thanks.

@waterlink waterlink added a commit to waterlink/capybara-webkit that referenced this issue May 19, 2014
@mhoran @waterlink mhoran + waterlink Read the expected number of bytes from StringIO
* Instead of sleep, use blocking #read to determine if the expected
  message is logged.
* Fixes failure 2 from #615.
34ca250
@waterlink waterlink added a commit to waterlink/capybara-webkit that referenced this issue May 19, 2014
@mhoran @waterlink mhoran + waterlink Use a pipe to test command logging
* JRuby doesn't write to StringIO in the same way as cruby/rbx, and the
  #read method used in connection_spec didn't work here.
* Fixes failure 3 from #615.
c386d6e
@mhoran
Collaborator
mhoran commented Jul 2, 2014

All outstanding JRuby compatibility patches have been merged. capybara-webkit supports at least JRuby version 1.7.10 in 1.9 mode, and may support earlier versions, though it requires at least version 1.7.6 due to jruby/jruby#1103.

@mhoran mhoran closed this Jul 2, 2014
@youpy youpy added a commit to youpy/capybara-webkit that referenced this issue Jul 22, 2014
@mhoran @youpy mhoran + youpy Read the expected number of bytes from StringIO
* Instead of sleep, use blocking #read to determine if the expected
  message is logged.
* Fixes failure 2 from #615.
106e065
@youpy youpy added a commit to youpy/capybara-webkit that referenced this issue Jul 22, 2014
@mhoran @youpy mhoran + youpy Use a pipe to test command logging
* JRuby doesn't write to StringIO in the same way as cruby/rbx, and the
  #read method used in connection_spec didn't work here.
* Fixes failure 3 from #615.
653983d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment