qa: Make TestNodeCLI command optional in send_cli #12089

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1801-qaCliOptions changing 2 files +10 −8

MarcoFalke commented at 1:12 PM on January 4, 2018: member
Makes the command optional, since there are valid bitcoin-cli calls that have no command:
- bitcoin-cli -?
- bitcoin-cli -getinfo
- ...
Also, rename self.args to self.options, since that is the name in the bitcoin-cli -help documentation.
MarcoFalke added the label Tests on Jan 4, 2018
MarcoFalke force-pushed on Jan 4, 2018
MarcoFalke renamed this:
~~qa: Don't remember TestNodeCLI options between calls~~
qa: TestNodeCLI Rework output parsing, Prevent options mixups, Make command optional
on Jan 4, 2018
MarcoFalke closed this on Jan 4, 2018
MarcoFalke deleted the branch on Jan 4, 2018
MarcoFalke renamed this:
~~qa: TestNodeCLI Rework output parsing, Prevent options mixups, Make command optional~~
qa: make command optional
on Jan 12, 2018
MarcoFalke renamed this:
~~qa: make command optional~~
qa: Make command optional
on Jan 12, 2018
MarcoFalke restored the branch on Jan 12, 2018
MarcoFalke renamed this:
~~qa: Make command optional~~
qa: Make TestNodeCLI command optional in send_cli
on Jan 12, 2018
MarcoFalke reopened this on Jan 12, 2018
MarcoFalke force-pushed on Jan 12, 2018
jnewbery commented at 5:22 PM on January 22, 2018: member

Tested ACK fab04c3f55715f8d63ae88d087e9ea83f525a670

in test/functional/test_framework/test_node.py:250 in fab04c3f55 outdated

 248 | -        p_args = [self.binary, "-datadir=" + self.datadir] + self.args
 249 | +        p_args = [self.binary, "-datadir=" + self.datadir] + self.options
 250 |          if named_args:
 251 |              p_args += ["-named"]
 252 | -        p_args += [command] + pos_args + named_args
 253 | +        if command:

ryanofsky commented at 7:55 PM on January 22, 2018:

Suggest s/command/command is not None/ so if 0 or False or [] or "" is passed, it will trigger appropriate python type errors or "method not found" error instead of "too few parameters" error.

in test/functional/test_framework/test_node.py:216 in fab04c3f55 outdated
```
 212 | @@ -213,16 +213,16 @@ class TestNodeCLI():
 213 |      """Interface to bitcoin-cli for an individual node"""
 214 |  
 215 |      def __init__(self, binary, datadir):
 216 | -        self.args = []
 217 | +        self.options = []
```
ryanofsky commented at 8:09 PM on January 22, 2018:

Rename seems appropriate, but ideally it would be in a separate commit, since it is bigger than the real change, and doesn't even overlap with it.
ryanofsky commented at 10:06 PM on January 22, 2018: member

utACK fab04c3f55715f8d63ae88d087e9ea83f525a670, but could be cleaned up slightly.
qa: Rename cli.args to cli.options
That is the name in bitcoin-cli -help
ffffb10a9f
MarcoFalke force-pushed on Jan 23, 2018
qa: Make TestNodeCLI command optional in send_cli fae7b14a04
MarcoFalke force-pushed on Jan 23, 2018
ryanofsky commented at 9:21 PM on January 23, 2018: member

utACK fae7b14a0468fba43ca8ce87249be69e13cea608. Thanks for update! Only changes since last review were what was suggested.
jnewbery commented at 9:30 PM on January 23, 2018: member

Tested ACK fae7b14a0468fba43ca8ce87249be69e13cea608
MarcoFalke merged this on Jan 24, 2018
MarcoFalke closed this on Jan 24, 2018
MarcoFalke referenced this in commit 126000ba9e on Jan 24, 2018
MarcoFalke deleted the branch on Jan 24, 2018
PastaPastaPasta referenced this in commit 3a34555989 on Apr 7, 2020
PastaPastaPasta referenced this in commit 01d3f009c5 on Apr 8, 2020
ckti referenced this in commit 8197e6763f on Mar 28, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels