test: Declare nodes type in test_framework.py. #20954

1. 
   kiminuo commented at 6:16 pm on January 17, 2021: contributor

   Motivation

   When I wanted to understand better https://github.com/bitcoin/bitcoin/pull/19145/files#diff-4bebbd3b112dc222ea7e75ef051838ceffcee63b9e9234a98a4cc7251d34451b test, I noticed that navigation in PyCharm/VS Code did not work for nodes variable. I think this is frustrating, especially for newcomers.

   Summary

   • This PR modifies Python 3.5 type comments to Python 3.6+ types and adds a proper type for nodes instance attribute.
   • This PR does not change behavior.
   • This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.

   End result

   1. Open test/functional/feature_abortnode.py
   2. Move your caret to: self.nodes[0].generate[caret here](3)
   3. Use “Go to definition” [F12] should work now.

   I have tested this on PyCharm (Windows, Ubuntu) and VS Code (Windows, Ubuntu).

   Note: Some TestNode methods (e.g. self.nodes[0].getblock(...) ) use __call__ mechanism and navigation does not work for them even with this PR.

2. DrahtBot added the label Tests on Jan 17, 2021
3. 
   DrahtBot commented at 11:18 pm on January 17, 2021: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19168 (Refactor: Improve setup_clean_chain semantics by fjahr)

   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.

4. Change type definitions for "chain" and "setup_clean_chain" from type comments to Python 3.6+ types. Additionally, set type for "nodes". 5353b0c64d
5. kiminuo force-pushed on Jan 18, 2021
6. kiminuo marked this as ready for review on Jan 18, 2021
7. 
   brunoerg commented at 7:56 pm on January 18, 2021: member

   Shouldn’t the types be added in all vars (network_thread, rpc_timeout..)?
8. 
   kiminuo commented at 8:01 pm on January 18, 2021: contributor

   Shouldn’t the types be added in all vars (network_thread, rpc_timeout..)?

   Yes but as I wrote in the description “This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.”

9. 
   laanwj commented at 8:58 pm on January 18, 2021: member

   Looks correct to me (and the CI does mypy checks on the test framework). ACK 5353b0c64d32e44fc411464e080d4b00fae7124e
10. 
    brunoerg commented at 9:38 pm on January 18, 2021: member

    Shouldn’t the types be added in all vars (network_thread, rpc_timeout..)?

    Yes but as I wrote in the description “This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.”

    No problem. Concept ACK.

11. theStack approved
12. 
    theStack commented at 9:14 pm on January 30, 2021: member

    I was curious and tried this out with PyCharm 2020.3.3 (Community Edition) and Python 3.9.1 under Win10. Navigation to the implementation of generate via Ctrl-Alt-B (I guess the proposed shortcut F12 is avilable in VSCode?) did work both in master and PR branch.

    That said, the patch looks correct.

    ACK 5353b0c64d32e44fc411464e080d4b00fae7124e

13. MarcoFalke merged this on Jan 31, 2021
14. MarcoFalke closed this on Jan 31, 2021

    ---

15. sidhujag referenced this in commit 9864bb9dfd on Feb 2, 2021
16. PastaPastaPasta referenced this in commit a815d52d02 on Sep 17, 2021
17. Fabcien referenced this in commit 5c70b37ed4 on Feb 1, 2022
18. DrahtBot locked this on Aug 16, 2022

Contributors
kiminuo DrahtBot brunoerg laanwj theStack

Labels
Tests