test: Fix TestShell to allow running in Jupyter Notebook #22201

pull josibake wants to merge 1 commits into bitcoin:master from josibake:josibake-parse-only-known-arguments-in-testshell changing 1 files +4 −0
  1. josibake commented at 3:27 PM on June 9, 2021: member

    this fixes argparse to use parse_known_args. previously, if an unknown argument was passed, argparse would fail with an unrecognized arguments: %s error.

    why

    the documentation mentions being able to run TestShell in a REPL interpreter or a jupyter notebook. when i tried to run inside a jupyter notebook, i got the following error:

    image

    this was due to the notebook passing the filename of the notebook as an argument. this is a known problem with notebooks and argparse, documented here: https://stackoverflow.com/questions/48796169/how-to-fix-ipykernel-launcher-py-error-unrecognized-arguments-in-jupyter

    testing

    to test, make sure you have jupyter notebooks installed. you can do this by running:

    pip install notebook
    

    or following instructions from here.

    once installed, start a notebook (jupyter notebook), launch a python3 kernel and run the following snippet:

    import sys
    
    # make sure this is the path for your system
    sys.path.insert(0, "/path/to/bitcoin/test/functional")
    from test_framework.test_shell import TestShell
    
    test = TestShell().setup(num_nodes=2, setup_clean_chain=True)
    

    you should see the following output, without errors: image

    if you are unfamiliar with notebooks, here is a short guide on using them: https://jupyter.readthedocs.io/en/latest/running.html

  2. fanquake added the label Tests on Jun 9, 2021
  3. fanquake commented at 4:59 AM on June 11, 2021: member

    Concept ACK

  4. theStack commented at 2:03 PM on June 18, 2021: contributor

    Concept ACK @josibake: Could you provide short instruction for reviewers on how to test this (or at least a link showing how to set up a jupyter notebook)?

  5. josibake commented at 4:31 PM on June 18, 2021: member

    sure thing, @theStack. i updated the description to include instructions on installing and running a notebook, as well as links back to the jupyter documentation

  6. theStack approved
  7. theStack commented at 10:38 AM on June 19, 2021: contributor

    ACK d66924a09c35bfea63a885d008e2e93d1cce9a6e 🪐

    Tested with Jupyter Notebook 6.1.6 / Python 3.8.8 on OpenBSD 6.9; also checked that TestShell used directly still works as expected.

  8. in test/functional/test_framework/test_framework.py:200 in d66924a09c outdated
     193 | @@ -194,7 +194,10 @@ def parse_args(self):
     194 |                              help="Run test using legacy wallets", dest='descriptors')
     195 |  
     196 |          self.add_options(parser)
     197 | -        self.options = parser.parse_args()
     198 | +        # Running TestShell in a Jupyter notebook causes additional args to be passed
     199 | +        # (e.g -f notebook-file.json)
     200 | +        # Using parse_known_args ensures we only parse the arguments defined above
     201 | +        self.options, _ = parser.parse_known_args()
    


    maflcko commented at 10:45 AM on June 19, 2021:

    I like the error message on typos of the arguments and slightly prefer the alternative solution presented here: https://stackoverflow.com/a/56349168/2084795


    theStack commented at 10:56 AM on June 19, 2021:

    Nice idea. Hopefully it doesn't end up like this: https://stackoverflow.com/a/67280166 (I don't know any details about Jupyter though, on what situations which extra arguments are passed etc.)


    josibake commented at 3:17 PM on June 21, 2021:

    great call out, @MarcoFalke , i confirmed this does silently accept typos in cli args, which would be super hard to debug when using the test args from the command line.

    i updated to the linked solution and, per @theStack 's comment, confirmed this was the only extra argument being passed. it doesn't guarantee new arguments wont pop up in the future and break this, but it seems better to use this approach for now and if/when more arguments break it, we can reevaluate the whole approach for a more robust solution.

  9. add dummy file param to fix jupyter
    testshell in jupyter was failing due to an extra arg.
    this adds a dummy -f param, which allows TestShell to
    be used in a command line or jupyter environment
    168b6c317c
  10. josibake force-pushed on Jun 21, 2021
  11. josibake requested review from maflcko on Jun 21, 2021
  12. maflcko commented at 4:00 PM on June 21, 2021: member

    review ACK 168b6c317ca054c1287c36be532964e861f44266

  13. practicalswift commented at 8:26 PM on June 21, 2021: contributor

    cr ACK 168b6c317ca054c1287c36be532964e861f44266

  14. maflcko merged this on Jun 22, 2021
  15. maflcko closed this on Jun 22, 2021

  16. bitcoin locked this on Jun 23, 2021
  17. josibake deleted the branch on Jan 26, 2024

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-18 12:14 UTC

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