Whisper support #117
Whisper support #117
Conversation
|
@pipermerriam How do I configure Travis to run Shh related testcases? |
|
|
||
| def test_shh_filter(web3,skip_if_testrpc): | ||
| skip_if_testrpc(web3) | ||
| topic = "".join(random.choice(string.ascii_uppercase + string.digits + string.ascii_lowercase) for _ in range(10)) |
There was a problem hiding this comment.
Instead of using random here can we use something deterministic or non-random. I have sound that unless the test is intentionally fuzzing the inputs that randomness can lead to infuriating hard to reproduce test failures.
| gevent.sleep(1) | ||
|
|
||
| for message in recieved_messages: | ||
| assert web3.toAscii(message["payload"]) in payloads |
There was a problem hiding this comment.
So this will pass silently if there are zero received messages. I think you need to add another assertion that ensures that at least some messages were received.
There was a problem hiding this comment.
Yea.How did I miss that? Will add one more assert.
There was a problem hiding this comment.
That's what code review is for 😄
| class Shh(object): | ||
| """ | ||
| TODO: flesh this out. | ||
| """ | ||
| def __init__(self, web3): | ||
| self.web3 = web3 | ||
| self.request_manager = web3._requestManager |
There was a problem hiding this comment.
Instead of storing this, can you either make a property that accesses this or access it directly through the web3 object. This pattern will break some of the new managers that I've been working on that distribute requests across multiple providers or managers.
@property
def request_manager(self):
return self.web3._requestManagerThere was a problem hiding this comment.
Sure.I'll go with the property approach.
| @@ -0,0 +1,21 @@ | |||
| import random,string,gevent | |||
|
|
|||
| def test_shh_filter(web3,skip_if_testrpc): | |||
There was a problem hiding this comment.
nitpick
Can you add a space after the , here and in the other test cases as well.
There was a problem hiding this comment.
Forgot the flake check.I will update with correct flake syntax.
|
@pipermerriam Not sure why https://travis-ci.org/pipermerriam/web3.py/jobs/167550202 is failing. |
|
Yeah, that's weird - just checked it out locally and it's worked for me - was hoping I'd be able to contribute. 🎱 |
|
@andylockran Thanks for checking! |
|
This just happens sometimes. Been meaning to increase that timeout. I restarted the run. Should pass and I'll get this merged. |
|
Doh, ok, one more hoop if you're willing to jump through it. Can you add/update the docs @ docs/web3.shh.rst with basic explanations of how to use these methods? Bonus points for functional example code for a common use case 😈 |
|
Sure.Will update soon. |
|
@pipermerriam Am I missing anything? |
|
This looks great. Thank you for doing this. Very well rounded PR. Going to get this merged now but likely not released until early next week. |
|
@pipermerriam Thanks to you! |
What was wrong?
Shh delegations were not implemented.
How was it fixed?
Added missing implementations and basic tests.
Cute Animal Picture