[qa] Fix pyton syntax in rpc tests #7335

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1601-rpcSyntax changing 34 files +80 −129

MarcoFalke commented at 7:12 PM on January 13, 2016: member

This is fixing four python syntax errors in the rpc tests. The rest is just removing imports or semicolons.
[qa] Fix pyton syntax in rpc tests 7777994846
MarcoFalke force-pushed on Jan 13, 2016
jonasschnelli added the label Tests on Jan 13, 2016
laanwj commented at 12:49 PM on January 14, 2016: member

This is quite invasive. Wouldn't syntax errors cause the tests to not run at all?
in qa/rpc-tests/disablewallet.py:None in 7777994846
```
   9 | @@ -10,6 +10,7 @@
  10 |  from test_framework.test_framework import BitcoinTestFramework
  11 |  from test_framework.util import *
  12 |  
  13 | +
```
GIJensen commented at 4:01 PM on January 14, 2016:

nit: This additional newline makes this a double-newline. This doesn't seem to match the style of the other files.

MarcoFalke commented at 1:42 PM on January 16, 2016:

Not sure where this came from. Still, PEP suggests two empty lines, so I will just leave it as is.
GIJensen commented at 4:05 PM on January 14, 2016: none

utACK 77779948, this looks like good cleanup to me.
MarcoFalke commented at 5:43 PM on January 14, 2016: member
@laanwj Indeed this is touching 129 lines, but all of it should be uncontroversial:
- Python will silently ignore trailing semicolons and I don't feel strong about this change. Still, I think we should not promote such usage.
- Excessive imports won't hurt performance too much but they fuzz the code, imo.
- Finally, python will only complain about syntax errors on runtime. Luckily all of the syntax errors behave like they are not there: You can do raise AssertionFailure("Failed to mine a version=4 blocks") to raise a NameError: name 'AssertionFailure' is not defined which makes travis fail as well but I think raise AssertionError("Failed to mine a version=4 blocks") is much clearer.
luke-jr commented at 5:08 PM on January 15, 2016: member

Concept ACK
laanwj commented at 9:51 AM on January 16, 2016: member

@MarcoFalke Thanks for the explanation, makes sense. utACK
fanquake commented at 12:19 PM on January 16, 2016: member

Ran this through the extended test suite, no issues.
laanwj merged this on Jan 18, 2016
laanwj closed this on Jan 18, 2016
laanwj referenced this in commit 3ec5bb0a6e on Jan 18, 2016
laanwj referenced this in commit d8b062d752 on Jan 18, 2016
MarcoFalke deleted the branch on Jan 18, 2016
str4d referenced this in commit ac9c0e2848 on Jan 31, 2017
str4d referenced this in commit a9445db62f on Feb 8, 2017
zkbot referenced this in commit 36df5a92f8 on Feb 9, 2017
zkbot referenced this in commit dd8b38316f on Feb 9, 2017
zkbot referenced this in commit 253c610783 on Feb 9, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels