[qa] mininode: Only allow named args in wait_until #8857

1. 
   MarcoFalke commented at 9:38 AM on October 1, 2016: member

   This pull will disallow non-keyword arguments in wait_until to prevent further BUGS such as #8854.

   <strike>This commit aims to prevent nasty bugs in the future. The majority of calls assumed that timeout is the second positional argument (even though it was not). So moving it to the second position will likely prevent bugs in the future.

   <strike>As a side effect, this will fix incorrectly passed args in the following scripts:

   $ git grep 'wait_until' qa/ | grep -v 'timeout='
   qa/rpc-tests/maxuploadtarget.py:        success = wait_until(received_pong, timeout)
   qa/rpc-tests/p2p-mempool.py:        success = wait_until(received_pong, timeout)
   

   <strike>Reviewers should call

   $ egrep -r -I '.*?wait_until\([^,]+,' qa/
   

   <strike>and verify that each second arg is either timeout or a named arg.

   This was discovered by @sdaftuar in #8854.

2. MarcoFalke added the label Tests on Oct 1, 2016
3. MarcoFalke added the label Needs backport on Oct 1, 2016
4. MarcoFalke added this to the milestone 0.13.1 on Oct 1, 2016
5. 
   laanwj commented at 3:04 PM on October 1, 2016: member

   Concept ACK, though IMO using named arguments is even more readable for ancillary arguments like timeout.

6. 
   MarcoFalke commented at 3:19 PM on October 1, 2016: member

   Agree, but note that the arguments are variables with the name timeout, but are not named arguments in maxuploadtarget.py and p2p-mempool.py. I don't think we can catch such issues during review in the future, we should probably force to name them...

7. 
   laanwj commented at 3:25 PM on October 1, 2016: member

   Nice solution. utACK https://github.com/bitcoin/bitcoin/pull/8857/commits/faf9c3a359959a07a1ee1d55102363fb58b43116

8. 
   btcdrak commented at 7:04 PM on October 1, 2016: contributor

   utACK faf9c3a

9. [qa] mininode: Only allow named args in wait_until fa666094cf
10. MarcoFalke force-pushed on Oct 2, 2016
11. 
    MarcoFalke commented at 10:22 AM on October 2, 2016: member

    Squashed both commits into fa66609

12. MarcoFalke renamed this:
    [qa] mininode: Fix order of positional args in wait_until
    [qa] mininode: Only allow named args in wait_until
    on Oct 2, 2016
13. 
    btcdrak commented at 5:13 PM on October 2, 2016: contributor

    re-utACK fa66609

14. MarcoFalke merged this on Oct 2, 2016
15. MarcoFalke closed this on Oct 2, 2016

    ---

16. MarcoFalke referenced this in commit 6013c73b33 on Oct 2, 2016
17. MarcoFalke deleted the branch on Oct 2, 2016
18. MarcoFalke referenced this in commit 1f60d45504 on Oct 3, 2016
19. MarcoFalke removed the label Needs backport on Oct 13, 2016
20. codablock referenced this in commit fe0becf001 on Sep 19, 2017
21. codablock referenced this in commit 16532f8ee0 on Jan 12, 2018
22. andvgal referenced this in commit 078027ca98 on Jan 6, 2019
23. DrahtBot locked this on Sep 8, 2021

Contributors

MarcoFalkelaanwjbtcdrak

Milestone
0.13.1