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

Write functional test that checks p2p messages serialization/deserialization #1020

Open
AM5800 opened this issue Apr 25, 2019 · 4 comments
Open
Assignees
Labels
feature New functionality p2p

Comments

@AM5800
Copy link
Member

AM5800 commented Apr 25, 2019

Currently we don't have any dedicated tests for the test framework itself - we only test what we use. One thing that seems important is messages serialization/deserialization.

I propose to write a test that will look like following:

node0                 node1
   \                    /
   mininode0 ------ mininode1

So all network traffic goes through mininodes. And we ensure that it is not altered during serialization/deserialization

@AM5800 AM5800 added feature New functionality p2p labels Apr 25, 2019
@AM5800 AM5800 self-assigned this Apr 25, 2019
@kostyantyn
Copy link
Member

kostyantyn commented Apr 25, 2019

Technically we should have p2p_*.py test for every p2p message we have. But I see our p2p tests check too much. Should we have p2p_<message>_schema.py separately or you plan to list all p2p messages in one test and send them through the proposed setup?

Also, If I understood correctly, you expect that node0 or node1 sends messages, right? For some p2p messages, a particular setup is required. Do you plan to "bypass" this setup or create such an environment for node0/node1 that it will start producing such messages?

@AM5800
Copy link
Member Author

AM5800 commented Apr 25, 2019

Should we have p2p__schema.py separately or you plan to list all p2p messages in one test and send them through the proposed setup?

Initial idea was to list them in one test. However it might get too big and perhaps we will have to split by feature or something else. I would say that this will be better to decide once we start working on this test(s).

There is another option though: we can create a MininodeRelay class that will just connect two nodes. And replace direct nodes connection with this relay in several existing tests which allow this.

Also, If I understood correctly, you expect that node0 or node1 sends messages, right?

Right.

create such an environment for node0/node1 that it will start producing such messages

This is what I would like to test - real messages. Not carefully hand-constructed ones

@cmihai
Copy link
Member

cmihai commented Apr 29, 2019

There is another option though: we can create a MininodeRelay class that will just connect two nodes. And replace direct nodes connection with this relay in several existing tests which allow this.

For an example of this, check out esperanza_expired_vote_conflict.py.

@AM5800
Copy link
Member Author

AM5800 commented Apr 29, 2019

@cmihai yeah, the thing you pointed was actually created by me. But It unfortunately skips serialization/deserialization completely - it relays the whole received buffer.

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

No branches or pull requests

3 participants