Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JeroMQ 0.5.1 #704

Closed
daveyarwood opened this issue Apr 2, 2019 · 16 comments
Closed

JeroMQ 0.5.1 #704

daveyarwood opened this issue Apr 2, 2019 · 16 comments

Comments

@daveyarwood
Copy link
Contributor

I'd like to release JeroMQ 0.5.1, but am currently unable to do so because 5 test cases are failing on my machine:

[INFO] [ERROR] Failures:
[INFO] [ERROR]   ZBeaconTest.testReceiveOwnBeacons:64
[INFO] Expected: is <0L>
[INFO]      but: was <1L>
[INFO] [ERROR]   ZBeaconTest.testReceiveOwnBeaconsBlocking:123
[INFO] Expected: is <0L>
[INFO]      but: was <1L>
[INFO] [ERROR]   ZBeaconTest.testUseBuilder:40
[INFO] Expected: is <0L>
[INFO]      but: was <1L>
[INFO] [ERROR]   TestAddress.testInvalid Expected exception: org.zeromq.ZMQException
[INFO] [ERROR]   TcpAddressTest.testBad:106
[INFO] [INFO]
[INFO] [ERROR] Tests run: 550, Failures: 5, Errors: 0, Skipped: 16

The ZBeacon tests have been noted before in an open issue, #620. I haven't seen these other 2 test failures before.

Help getting these tests to pass would be much appreciated!

The changelog also needs to be updated, which I'm happy to take care of unless somebody else beats me to it.

@fbacchella
Copy link
Contributor

My own build and tests succeeded with Java 8 and 11:
https://circleci.com/gh/fbacchella/jeromq/167
https://circleci.com/gh/fbacchella/jeromq/166.

Tests are very brittle and can fails for no reason.

Perhaps this commit will help: 8c3f999

@fredoboulo
Copy link
Contributor

Could you give a try to modify the tests by stopping the beacon before asserting?

@daveyarwood
Copy link
Contributor Author

@fredoboulo I just tried that, and the same 3 tests still failed.

I did notice that the tests take a long time, which makes me think that the latch.await(20, TimeUnit.SECONDS); call is timing out instead of counting down. (Which makes sense anyway, since the test failure is that the latch count is 1 instead of 0.) Could this actually be a bug in ZBeacon?

@daveyarwood
Copy link
Contributor Author

Aha! I found the issue. We're creating the ZBeacon for those tests on port 255.255.255.255, which is supposed to be the "global" broadcast address, not all routers support it and its use is discouraged.

I changed it to 127.0.0.1 and the tests pass.

I can make a PR with that change, unless there is a reason not to?

@daveyarwood
Copy link
Contributor Author

daveyarwood commented Apr 3, 2019

Hmm, it would mean that the broadcast feature would not be tested, though...

Context: f9107df, #686

@fbacchella
Copy link
Contributor

255.255.255.255 is a local link broadcast address (not a port), so no router at all should forward it. The answer that you linked to is plain wrong.

@fredoboulo
Copy link
Contributor

I see not point of making an integration test for broadcast feature, when there is only one local machine having the beacon.
I guess we will have to rely on manual testing for that part.

@daveyarwood
Copy link
Contributor Author

It does seem kind of pointless, come to think of it.

I wonder if the integration tests idea we've been discussing (#631) could be set up in some way to test using multiple machines? I doubt there is a way to do it for free, though, since this project doesn't have funding. We might be stuck with manual testing, at the end of the day.

@daveyarwood
Copy link
Contributor Author

I'm looking into the TestAddress.testInvalid failure now. The test case asserts that ggglocalhostxxx:90 does not resolve, but when I run it on my machine, it resolves to 198.105.254.228:90!

I played around with dig on ggglocalhostxxx as well as a few gibberish strings, and they all resolve to that same IP address, so I think this might be my ISP providing a fallback IP address in the case that an address cannot be resolved.

I'm not really sure how to address this. Maybe this means that we can't reliably determine that an address is invalid, and we should just remove the testInvalid test case altogether. I'll make a PR proposing that.

@daveyarwood
Copy link
Contributor Author

TcpAddressTest.testBad looks to be exactly the same issue. Will also propose removing that test case in the same PR.

@fbacchella
Copy link
Contributor

Did you try to use an invalid FQDN like ggglocalhostxxx.google.com ? It might help.

@daveyarwood
Copy link
Contributor Author

@fbacchella Good call! I tried that and both tests passed. So if I'm understanding correctly, as long as we use a known domain like google.com and a known invalid subdomain, the tests should pass.

I'll make another PR to re-add those tests.

@daveyarwood
Copy link
Contributor Author

I think I'm un-blocked at this point. I'll take a stab at releasing JeroMQ 0.5.1 soon!

@daveyarwood
Copy link
Contributor Author

JeroMQ 0.5.1 is now available in Maven Central.

TODO at this point:

  • Update the changelog.
  • Announce the release on the ZMQ mailing list.

@trevorbernard It looks like I would need to subscribe to the ZMQ mailing list in order to announce the release, and I'm not really interested in subscribing, particularly because this page describes it as high-traffic and I'm an Inbox Zero person. Would you be OK with continuing to announce releases on the mailing list?

@trevorbernard
Copy link
Member

No worries. I can do the announcements.

@daveyarwood
Copy link
Contributor Author

Awesome, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants