qa: Fix service flag comparison check in rpc_net test #16936

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:fix_servicesnames_test changing 2 files +9 −14
  1. luke-jr commented at 10:16 PM on September 22, 2019: member

    #16850 was basically completely broken...

  2. luke-jr commented at 10:16 PM on September 22, 2019: member

    This fixes it, but the stupid linters don't like it:

    test/functional/test_framework/messages.py:46: unused variable 'NODE_GETUTXO' (60% confidence)

    Can I just get rid of the linter, or is there another approach that would be preferred?

  3. fanquake added the label Tests on Sep 22, 2019
  4. promag commented at 10:24 PM on September 22, 2019: member

    You could add to test/lint/lint-python-dead-code-whitelist?

  5. luke-jr force-pushed on Sep 22, 2019
  6. luke-jr force-pushed on Sep 22, 2019
  7. Bugfix: QA: Fix service flag comparison check in rpc_net test 78487ef8ae
  8. luke-jr force-pushed on Sep 22, 2019
  9. luke-jr commented at 11:30 PM on September 22, 2019: member

    There, got linter happy.

  10. theStack commented at 12:30 AM on September 23, 2019: member

    ACK 78487ef

    That was quite an interesting chain of coincidences and fails; the fact that the tested servicesflags (0x809) didn't contain any hexadecimal symbols a-f covered the wrong base for the hexstring-to-integer conversion, which in turn was covered by a too weak assertion policy in assert_net_servicesnames(), also allowing more names in the list than just the ones represented in the flags. An additional assertion bin(servicesflags).count('1') == len(servicesnames) would have also solved the problem, but the PR approach, getting rid of all the if statements, is more generic and flexible.

  11. luke-jr referenced this in commit 4801f335f0 on Sep 23, 2019
  12. MarcoFalke added this to the milestone 0.19.0 on Sep 23, 2019
  13. MarcoFalke commented at 3:44 PM on September 23, 2019: member

    I wouldn't be surprised if other users also stepped into this trap. The field should probably be a normal json number, not a hex encoded string version of the int.

    Concept ACK anyway

  14. luke-jr commented at 5:39 PM on September 23, 2019: member

    Yes, agree it should be a Number ideally.

  15. MarcoFalke commented at 5:44 PM on September 23, 2019: member

    unsigned ACK 78487ef8ae9d56d3285955c258c8fe0abf55b516 (looked at the diff on GitHub)

  16. DrahtBot commented at 1:37 PM on September 25, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16961 (test: Remove python dead code linter by laanwj)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  17. in test/lint/lint-python-dead-code-whitelist:16 in 78487ef8ae 
      11 | @@ -12,6 +12,8 @@ InvalidOPIFConstruction  # unused class (test/functional/data/invalid_txs.py)
  12 |  _.is_compressed  # unused property (test/functional/test_framework/key.py)
  13 |  legacy  # unused variable (test/functional/test_framework/address.py)
  14 |  msg_generic  # unused class (test/functional/test_framework/messages.py)
  15 | +NODE_BLOOM  # actually used (test/functional/test_framework/messages.py)
  16 | +NODE_GETUTXO  # actually used (test/functional/test_framework/messages.py)
    MarcoFalke commented at 5:36 PM on September 25, 2019:

    This file was deleted in master. Mind to remove those changes again and force push? (No need to rebase, if you don't want to)

  18. DrahtBot added the label Needs rebase on Sep 25, 2019
  19. DrahtBot commented at 6:00 PM on September 25, 2019: member

    <!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase

  20. laanwj renamed this:
    Bugfix: QA: Fix service flag comparison check in rpc_net test
    qa: Fix service flag comparison check in rpc_net test
    on Sep 26, 2019
  21. MarcoFalke added the label Up for grabs on Sep 27, 2019
  22. laanwj commented at 9:48 AM on September 30, 2019: member

    Closing in favor of #16991

  23. laanwj removed the label Up for grabs on Sep 30, 2019
  24. laanwj removed this from the milestone 0.19.0 on Sep 30, 2019
  25. fanquake closed this on Sep 30, 2019
  26. fanquake removed the label Needs rebase on Sep 30, 2019
  27. laanwj referenced this in commit c3a8e097b1 on Sep 30, 2019
  28. DrahtBot locked this on Dec 16, 2021
Contributors
luke-jrpromagtheStackMarcoFalkeDrahtBotlaanwj
Labels
Tests
Linked (view graph)
#16850 test: `servicesnames` field in `getpeerinfo` and `getnetworkinfo`#16991 qa: Fix service flag comparison check in rpc_net test (luke-jr)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me