Skip to content

Conversation

darosior
Copy link
Member

As per #16787 (comment), fixes #16844.

This adds a test for both commands in the first commit and renames the test for getnetworkinfo in the second commit.

@darosior darosior changed the title Functionnal tests for servicesnames field in getpeerinfo and getnetworkinfo Functional tests for servicesnames field in getpeerinfo and getnetworkinfo Sep 10, 2019
@fanquake fanquake added the Tests label Sep 10, 2019
@darosior
Copy link
Member Author

darosior commented Sep 10, 2019

Could someone restart the stopped job (583339836) please ?
EDIT: Thanks !

@maflcko maflcko changed the title Functional tests for servicesnames field in getpeerinfo and getnetworkinfo test: servicesnames field in getpeerinfo and getnetworkinfo Sep 11, 2019
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Copy link
Contributor

@jamesob jamesob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, thanks for writing tests.

@darosior
Copy link
Member Author

Rebased to regroup into a function as suggested by @MarcoFalke.

@laanwj
Copy link
Member

laanwj commented Sep 12, 2019

ACK 1d524c6

laanwj added a commit that referenced this pull request Sep 12, 2019
…tworkinfo`

1d524c6 tests: rename 'test_getnetworkinginfo' in 'test_getnetworkinfo' (darosior)
07a8f65 tests: add a test for the 'servicesnames' RPC field (darosior)

Pull request description:

  As per #16787 (comment), fixes #16844.

  This adds a test for both commands in the first commit and renames the test for `getnetworkinfo` in the second commit.

ACKs for top commit:
  laanwj:
    ACK 1d524c6

Tree-SHA512: 8267dce4d54356debab75014e6f9ba885b892da605ed32f26a5446c232992fcae761861bb678adbdb942815d4706f3768c70deee6afec68f219b23605475be01
@laanwj laanwj merged commit 1d524c6 into bitcoin:master Sep 12, 2019
luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Sep 21, 2019
In getpeerinfo and getnetworkinfo

Github-Pull: bitcoin#16850
Rebased-From: 07a8f65
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 16, 2020
Summary:
This adds a test for both commands in the first commit and renames the test for getnetworkinfo.
Depends on D7948

This is a backport of Core [[bitcoin/bitcoin#16850 | PR16850]]

Test Plan: `ninja && test/functional/test_runner.py rpc_net.py`

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7949
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 23, 2020
Summary:
The tests added in [[bitcoin/bitcoin#16850 | PR16850]] assumed that the field `localservices` returned by `getnetworkinfo` was a string representation of an integer, when it is really a string representation of a hex. This fixes the test to decode the field properly and do the correct bitwise comparisons.

This is a backport of Core [[bitcoin/bitcoin#16991 | PR16991]]

Test Plan:
`ninja && ./test/functional/test_runner.py rpc_net.py`

```
$ bitcoin-cli getnetworkinfo
...
 "localservices": "0000000000000425",
  "localservicesnames": [
    "NETWORK",
    "BLOOM",
    "BITCOIN_CASH",
    "NETWORK_LIMITED"
  ],
...

$ python
>>> int("425", 16)
1061
>>> NODE_NETWORK = (1 << 0)
>>> NODE_BLOOM = (1 << 2)
>>> NODE_BITCOIN_CASH = (1 << 5)
>>> NODE_NETWORK_LIMITED = (1 << 10)
>>> NODE_NETWORK | NODE_BLOOM | NODE_BITCOIN_CASH | NODE_NETWORK_LIMITED
1061
>>>

```

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D8083
kwvg added a commit to kwvg/dash that referenced this pull request Nov 6, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Dec 5, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Dec 5, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Dec 12, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Dec 12, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: Add smoke test for getpeerinfo['servicesnames']
5 participants