andrewvc / dripdrop

Experimental Swiss Army Knife of Network Concurrency, ZeroMQ, EventMachine, WebSockets, HTTP, and More
MIT License
144 stars 10 forks source link

Broken specs for 00fea4e #14

Closed oruen closed 13 years ago

oruen commented 13 years ago

After making rspec run on my 1.9.2 ruby I've realized some spec are broken.

Some of them are fixed using DripDrop::Node.error_handler instead of DripDrop::Node#error_handler. Patch is here: oruen/dripdrop@717e8870a574c1114dba5945e5d5c26f78a416a0

Others looks a little bit strange:

[oruen@192 dripdrop (master %=)]$ rspec spec/node/zmq_xrepxreq_spec.rb

zmq xreq/xrep
  basic sending and receiving
Expected message in at least 3 parts, got ["\x00\v\x97s^\x18 I\xCD\x81\xF8t\xBFd]\xA0a", "{\"name\":\"test-0\",\"head\":{\"message_class\":\"DripDrop::Message\",\"_dd/xctr\":1},\"body\":null}"]/Users/oruen/apps/my/dripdrop/lib/dripdrop/handlers/zeromq.rb:200:in `on_readable' 

I'm using Ruby 1.9.2 and zmq from head at zeromq/zeromq2@29e0e7dbadfcd0bab70feee119bd7c5e623b38d4

andrewvc commented 13 years ago

First off, thanks for the patches. Much appreciated :)

I just fixed that 10 minutes ago here: https://github.com/andrewvc/dripdrop/commit/95349e272a1a58bbefb19199c5508ed41ddddbd6.

Can you try pulling? Basically, a bug crept in where the message delimiter wasn't being handled properly.

oruen commented 13 years ago

zmq_xrepxreq_spec is ok now, thanks!

Looks like I'm having with spec/node/zmq_pushpull_spec.rb When I run rake spec one of it's spec seems to be broken:

Failures:

  1) zmq push/pull basic sending and receiving should receive messages on both pull sockets
     Failure/Error: @responses.map {|r| r.head['recv_sock']}.uniq.sort.should == [1,2]
       expected: [1, 2]
            got: [1] (using ==)
       Diff:
       @@ -1,2 +1,2 @@
       -[1, 2]
       +[1]
     # ./spec/node/zmq_pushpull_spec.rb:49:in `block (3 levels) in '

But when I run exactly that spec file, it seems to be ok:

$ rspec spec/node/zmq_pushpull_spec.rb 

zmq push/pull
  basic sending and receiving
    should receive all sent messages
    should receive messages on both pull sockets

Finished in 0.31276 seconds
2 examples, 0 failures
Broken pipe
nbytes != -1 (mailbox.cpp:189)
Abort trap
andrewvc commented 13 years ago

Works for me but, here's an idea, can you try it with this version of em-zeromq?

https://github.com/andrewvc/em-zeromq/tree/stable

I haven't released it as a gem yet, because it's a hack. Basically, it seems on weird occasions eventmachine doesn't get notified that the sock is readable, and can miss a message. The patch attempts to read from all readable sockets once every 0.5 seconds. A terrible idea if you have a lot of sockets of course (kind of ruins the idea of eventmachine) but it might make this spec work.

oruen commented 13 years ago

Looks like specs fails from time to time. Even with that em-zeromq stable branch.

Do you have that

Broken pipe
nbytes != -1 (mailbox.cpp:189)
Abort trap

part on specs run? Refs to https://github.com/zeromq/zeromq2/blob/master/src/mailbox.cpp#L189

andrewvc commented 13 years ago

Yeah, I've got that as well. that nbytes issue is from not terminating ZMQ properly, so, it's only something you'll see on shutdown. It's definitely worth fixing. I never call terminate on the zmq context, and I definitely should. Additionally, dripdrop should really use LINGER and clean up messages before terminating.

As far as that spec goes, I'm not sure what the bug is, it could be that the run_reactor part of the spec isn't being given enough time. In practice I haven't seen this surface as an actual bug fortunately.

wishdev commented 13 years ago

I was hitting this quite a bit and I've push out a patch to the spec which solved it for me. Basically it runs the reactor for 2 seconds and adds a sleep between setting up the sockets and firing off the messages. My guess is that the spec is just running too fast sometimes and it's not getting things right.

This is always one of the problems with testing things like this in that it's not a real setup but rather a tight little "loop" that fires off the messages so timing is much harder to deal with.

This should solve things....

https://github.com/andrewvc/dripdrop/commit/d2154aa172953ea6ffb58165dd815038c2b68c34

andrewvc commented 13 years ago

Great catch, I know that those timing issues are hard to debug. In fact, with em specs I generally just start throwing sleeps in there if I have weird issues myself.

BTW, we're gearing up for a release in a couple days. Hopefully we can tie it in with a new em-zeromq, and even more hopefully, with a new eventmachine gem (the bug aman fixed last week made em-zeromq unsuitable for production).

andrewvc commented 13 years ago

I'll let you guys resolve this by the way, as I've never been able to repro this (my laptop just has the right touch I guess).