patch the JRuby TCPSocket#read(0) (originated by @duelinmarkers) #220

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@dnagir

dnagir commented Nov 29, 2011

See #197

@halogenandtoast

This comment has been minimized.

Show comment Hide comment
@halogenandtoast

halogenandtoast Feb 17, 2012

Contributor

Any chance you can add a test for this. With a test case I'd have no problem merging this in.

Contributor

halogenandtoast commented Feb 17, 2012

Any chance you can add a test for this. With a test case I'd have no problem merging this in.

@dnagir

This comment has been minimized.

Show comment Hide comment
@dnagir

dnagir Feb 19, 2012

@halogenandtoast I've added "blind" specs (couldn't run those due to some local installation issues). Please double check those before merging. Sorry for that, hope that's not a big hassle.

dnagir commented Feb 19, 2012

@halogenandtoast I've added "blind" specs (couldn't run those due to some local installation issues). Please double check those before merging. Sorry for that, hope that's not a big hassle.

@halogenandtoast

This comment has been minimized.

Show comment Hide comment
@halogenandtoast

halogenandtoast Mar 16, 2012

Contributor

@dnagir Sorry for taking so long to get back to you. The blind specs you wrote fail because they're calling a private method for subject { browser.read_response }. I tried very briefly to fix this, but wasn't successful. I want to come back to this, but if you can get your tests working before I do, that would help greatly.

Contributor

halogenandtoast commented Mar 16, 2012

@dnagir Sorry for taking so long to get back to you. The blind specs you wrote fail because they're calling a private method for subject { browser.read_response }. I tried very briefly to fix this, but wasn't successful. I want to come back to this, but if you can get your tests working before I do, that would help greatly.

derekprior added a commit to derekprior/capybara-webkit that referenced this pull request Mar 30, 2012

Read from socket only when there's content
This works around an issue with the TCPSocket class in jruby that causes
it to block indefinitely while trying to read a non-existent response.
MRI will immediately return an empty string.

The fix was to a private method and required stubbing an instance
variable not publically accessible so the specs have to get kind of cute
to work around this.

This pull request closes issue #197 and supersedes pull request #220
(which had broken specs). Credit @duelinmarkers.
@halogenandtoast

This comment has been minimized.

Show comment Hide comment
@halogenandtoast

halogenandtoast May 4, 2012

Contributor

@jferris and I refactored the code a bit to make testing this easier and added the fix for this and merged it into master. Closing this pull request.

Contributor

halogenandtoast commented May 4, 2012

@jferris and I refactored the code a bit to make testing this easier and added the fix for this and merged it into master. Closing this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment