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

Fix last contacted #3286

Merged
merged 3 commits into from
May 25, 2021
Merged

Conversation

dsiganos
Copy link
Contributor

Fix unit test network.last_contacted (#3285)

The unit test network.last_contacted has the following problems:
* regular keepalive messages sent automatically can interfere with the test
* the test should wait for the clock to move at least one tick to eliminate the possibility of the clock not having the time to move forward
* the test waits for a the keepalive counter to be incremented and then expects the last_packet_received timestamp to be updated but the timestamp is updated after the counter is incremented, which is a race condition

The solution:
* send a second keepalive to handle the race condition between setting the timestamp and incrementing the counter
* send a third keepalive to handle the case where an automatic keepalive message is already in flight
* wait for the clock to move after reading it and before continuing

Assumptions:
We assume that there will be no more than 1 keelalive in flight.
Of course, in theory, there could be multiple keepalives queued up but
for the purposes of this test, we assume max 1 keepalive in flight.
It is not an unreasonable assumption in this test case.

This is done so that node numbering numbering matches the channel
numbering, which makes easy to determine which channel belongs to which
node.
The unit test network.last_contacted has the following problems:

* regular keepalive messages sent automatically can interfere with the test
* the test should wait for the clock to move at least one tick to eliminate the possibility of the clock not having the time to move forward
* the test waits for a the keepalive counter to be incremented and then expects the last_packet_received timestamp to be updated but the timestamp is updated after the counter is incremented, which is a race condition

The solution:
* send a second keepalive to handle the race condition between setting the timestamp and incrementing the counter
* send a third keepalive to handle the case where an automatic keepalive message is already in flight
* wait for the clock to move after reading it and before continuing

Assumptions:
We assume that there will be no more than 1 keelalive in flight.
Of course, in theory, there could be multiple keepalives queued up but
for the purposes of this test, we assume max 1 keepalive in flight.
It is not an unreasonable assumption in this test case.
@dsiganos dsiganos merged commit 64b710d into nanocurrency:develop May 25, 2021
@zhyatt zhyatt modified the milestones: V22.1, V23.0 Jun 22, 2021
@zhyatt zhyatt added the unit test Related to a new, changed or fixed unit test label Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unit test Related to a new, changed or fixed unit test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants