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

test: Fix rpc_net.py "pong" race condition #15069

Merged
merged 1 commit into from Jan 2, 2019
Merged

Conversation

@Empact
Copy link
Member

@Empact Empact commented Dec 31, 2018

Prior to this change, the test fails with KeyError if pong has
a zero value at the time this is called, as getpeerinfo's
bytesrecv_per_msg result excludes zero-values.

Combined these to a single wait_until as well, which will be a bit more
forgiving re the timeout while still enforcing the same 2 seconds
overall.

https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

@Empact
Copy link
Member Author

@Empact Empact commented Dec 31, 2018

/cc #12804

@fanquake fanquake added the Tests label Dec 31, 2018
@fanquake fanquake requested a review from jnewbery Jan 1, 2019
Copy link
Member

@MarcoFalke MarcoFalke left a comment

It seems weird that a key error is thrown unless the value is not 0. With the principle of least surprise, it should probably always return the full result with all values (even if they are 0)

test/functional/rpc_net.py Outdated Show resolved Hide resolved
@Empact
Copy link
Member Author

@Empact Empact commented Jan 1, 2019

@MarcoFalke makes sense to me to show them, given its an rpc command / programmatic interface as well as a human one. zero-values have been excluded since introduction in ca188c6. /cc @jonasschnelli

Prior to this change, the test fails with KeyError if pong has
a zero value at the time this is called, as getpeerinfo's
bytesrecv_per_msg result excludes zero-values.

https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62
@Empact
Copy link
Member Author

@Empact Empact commented Jan 1, 2019

On reflection, seems the before case is the more likely culprit, which calls for using get rather than regular key access, to guard against the 0 case.

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 2, 2019

utACK de23739, I prefer this solution to #15077

@MarcoFalke MarcoFalke merged commit de23739 into bitcoin:master Jan 2, 2019
2 checks passed
MarcoFalke added a commit that referenced this issue Jan 2, 2019
de23739 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0
@Empact Empact deleted the rpc_net-race branch Jan 2, 2019
@jnewbery
Copy link
Member

@jnewbery jnewbery commented Jan 7, 2019

utACK de23739. Thanks @Empact!

codablock added a commit to codablock/dash that referenced this issue Apr 7, 2020
de23739 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0
codablock added a commit to codablock/dash that referenced this issue Apr 8, 2020
de23739 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0
codablock added a commit to codablock/dash that referenced this issue Apr 8, 2020
de23739 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Apr 24, 2020
Summary:
de23739b22 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0

Backport Core [[bitcoin/bitcoin#15069 | PR15069]]

Test Plan:
```
test_runner.py rpc_net  # run a few times
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5829
ftrader added a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this issue Aug 17, 2020
Summary:
de23739b22 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0

Backport Core [[bitcoin/bitcoin#15069 | PR15069]]

Test Plan:
```
test_runner.py rpc_net  # run a few times
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5829
ckti added a commit to ckti-ioncore-current/ion that referenced this issue Mar 28, 2021
de23739 test: Fix rpc_net.py "pong" race condition (Ben Woosley)

Pull request description:

  Prior to this change, the test fails with KeyError if pong has
  a zero value at the time this is called, as getpeerinfo's
  bytesrecv_per_msg result excludes zero-values.

  Combined these to a single wait_until as well, which will be a bit more
  forgiving re the timeout while still enforcing the same 2 seconds
  overall.

  https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/21310881#L62

Tree-SHA512: dc60f95a0e139c104fd81c8a7e0c9b3c25907de26c9d4e5976ae490e8ed5db0f0c492cd0e996ef6b5eb02cae82a62d4551ed36f95601871b19472050b3247bc0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants