test: Handle empty string returned by CLI as None in RPC tests #32286

pull BrandonOdiwuor wants to merge 1 commits into bitcoin:master from BrandonOdiwuor:test-fix-cli-jsonnone changing 1 files +2 −0
  1. BrandonOdiwuor commented at 11:26 am on April 16, 2025: contributor

    Partially Fixes #32264

    Some tests are failing when bitcoin-cli returns an empty string. This change treats an empty response as None. See #32264 (comment)

    This fixes the error for:

    • feature_bip68_sequence.py
    • feature_nulldummy.py
    • feature_signet.py
    • mining_mainnet.py
    • rpc_scanblocks.py
    • rpc_scantxoutset.py
    • wallet_descriptor.py –descriptors
  2. test: Handle empty string returned by CLI as None in RPC tests a4041c77f0
  3. DrahtBot commented at 11:26 am on April 16, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32286.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK maflcko, pablomartin4btc, mzumsande, achow101

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  4. DrahtBot added the label Tests on Apr 16, 2025
  5. maflcko commented at 11:46 am on April 16, 2025: member

    lgtm ACK a4041c77f0e20d004524868e70ff12508832c9eb

    I haven’t tested this, but it makes sense to recover an empty string as json-none for legacy RPCs. Obviously, this could lead to errors when an empty string is returned for real, but this should never happen: I checked that all unwrapped json-strings returned are never empty (txid, block hash, hex encodings, addresses, …). I haven’t checked getblocktemplate rejection strings, but this seems fine either way, given that it is only the test suite.

    For new RPCs, they are recommended to return json dicts, always. So this won’t be an issue going forward.

  6. in test/functional/test_framework/test_node.py:926 in a4041c77f0
    918@@ -919,6 +919,8 @@ def send_cli(self, clicommand=None, *args, **kwargs):
    919             # Ignore cli_stdout, raise with cli_stderr
    920             raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr)
    921         try:
    922+            if not cli_stdout.strip():
    923+                return None
    924             return json.loads(cli_stdout, parse_float=decimal.Decimal)
    925         except (json.JSONDecodeError, decimal.InvalidOperation):
    926             return cli_stdout.rstrip("\n")
    


    pablomartin4btc commented at 4:07 pm on April 16, 2025:

    nit: checking for cli_stdout shouldn’t be outside the try? also cos it shouldn’t fail and if it does the except is also accessing it…

    0            raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr)
    1        if not cli_stdout.strip():
    2            return None
    3        try:
    4            return json.loads(cli_stdout, parse_float=decimal.Decimal)
    5        except (json.JSONDecodeError, decimal.InvalidOperation):
    6            return cli_stdout.rstrip("\n")
    

    BrandonOdiwuor commented at 6:42 pm on April 16, 2025:
    I think this was more to keep the logic in one place
  7. pablomartin4btc commented at 4:07 pm on April 16, 2025: member
    ACK a4041c77f0e20d004524868e70ff12508832c9eb
  8. mzumsande commented at 8:51 pm on April 16, 2025: contributor
    ACK a4041c77f0e20d004524868e70ff12508832c9eb
  9. achow101 commented at 11:07 pm on April 16, 2025: member
    ACK a4041c77f0e20d004524868e70ff12508832c9eb
  10. achow101 merged this on Apr 16, 2025
  11. achow101 closed this on Apr 16, 2025

  12. fanquake commented at 11:49 am on April 17, 2025: member
    Backported in #32292.
  13. fanquake referenced this in commit 8b25ce59be on Apr 17, 2025

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: 2025-04-19 21:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me