igrigorik / em-http-request

Asynchronous HTTP Client (EventMachine + Ruby)
1.22k stars 220 forks source link

Run test server with WEBrick instead of Mongrel #327

Closed CodeMonkeySteve closed 4 years ago

CodeMonkeySteve commented 6 years ago

(Mongrel is no longer supported with by Rack v2+)

Remaining test failures:

  1) EventMachine::HttpRequest should set content-length to 0 on posts with empty bodies
     Failure/Error: http.response.strip.split(':')[1].should == '0'
       expected: "0"
            got: nil (using ==)

  2) EventMachine::HttpRequest should report error if connection was closed by server on client keepalive requests
     Failure/Error: req.callback { fail }
     RuntimeError:

  3) EventMachine::HttpRequest should fail gracefully on an invalid host in Location header
     Failure/Error: http.error.should match(/unable to resolve (server |)address/)

       expected "connection closed by server" to match /unable to resolve (server |)address/

  4) EventMachine::HttpRequest should keep default http port in redirect url that include it
     Failure/Error: http.last_effective_url.to_s.should == 'http://host:80/'

       expected: "http://host:80/"
            got: "http://host/" (using ==)

  5) EventMachine::HttpRequest should keep default https port in redirect url that include it
     Failure/Error: http.last_effective_url.to_s.should == 'https://host:443/'

       expected: "https://host:443/"
            got: "https://host/" (using ==)
igrigorik commented 6 years ago

👍👍

I wouldn't be surprised if some of these failures are due to Mongrel specific behavior(s), but regardless this looks really promising. Thanks for driving this!

CodeMonkeySteve commented 6 years ago

It looks like WEBrick might not be the best replacement for Mongrel. Aside from the different behavior that breaks the tests, it also doesn't support 100-continue (and breaks if you try to use it). Thin seems to work better, but since it uses EM, every call to EM.stop also stops the Stallion server (even if it's on another thread), and I don't know how to wrap all of RSpec in EM.run.

sodabrew commented 6 years ago

Oh there’s a wiki page in the EM wiki, someone dropped in an example of an EM.run unit test wrapper. (Can’t find at the moment as I am on my phone.)

CodeMonkeySteve commented 6 years ago

It's easy to run each test in an EM block with an around filter, the problem is that any servers or connections (i.e. the Stallion server) need to be restarted before each test, as EM.stop kills their sockets. Alternatively, we could monkey-patch RSpec to run the whole test suite in an EM block, but then we'd need some way for tests to indicate success/failure (each test would only end after some callback fires). I've run into this issue on every project where I use EM, and haven't yet found a good solution.

igrigorik commented 4 years ago

Per https://github.com/igrigorik/em-http-request/pull/327#issuecomment-411124401, closing. Feel free to reopen if you want to resurrect this.