Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[qa] Bug fixes and refactor #7778

Merged
merged 4 commits into from Apr 3, 2016

Conversation

Projects
None yet
3 participants
Member

MarcoFalke commented Mar 31, 2016

It is not possible to switch to py3 without fixing those bugs or doing the refactor. (E.g. python2 will silently cast floats to ints where needed, but I'd rather explicitly use ints)

To aid review, each commit in this pull will either fix bugs or do some refactor.

@jonasschnelli jonasschnelli added the Tests label Mar 31, 2016

Member

MarcoFalke commented Apr 1, 2016

Extended tests pass and enabling python deprecation warnings only give the following "false positive" warnings:

$ qa/pull-tester/rpc-tests.py -extended | grep -C 1 DeprecationWarning
/home/marco/workspace/bitcoin/qa/rpc-tests/test_framework/blocktools.py:49: DeprecationWarning: classic int division
  halvings = int(height/150) # regtest
/home/marco/workspace/bitcoin/qa/rpc-tests/replace-by-fee.py:33: DeprecationWarning: classic long division
  while node.getbalance() < satoshi_round((amount + fee)/COIN):
/home/marco/workspace/bitcoin/qa/rpc-tests/replace-by-fee.py:39: DeprecationWarning: classic long division
  txid = node.sendtoaddress(new_addr, satoshi_round((amount+fee)/COIN))
/home/marco/workspace/bitcoin/qa/rpc-tests/replace-by-fee.py:396: DeprecationWarning: classic long division
  split_value = int((initial_nValue-fee)/(MAX_REPLACEMENT_LIMIT+1))

@laanwj laanwj and 1 other commented on an outdated diff Apr 2, 2016

qa/rpc-tests/test_framework/mininode.py
else:
- r = chr(255) + struct.pack("<Q", len(l))
+ r = struct.pack("B", 255) + struct.pack("<Q", len(l))
@laanwj

laanwj Apr 2, 2016

Owner

What about:

r = struct.pack("<BQ", 255, len(l))

etc?

@MarcoFalke

MarcoFalke Apr 2, 2016

Member

Sure, this makes sense. I've tried to make the diff easier to read but I will address you nit, as it makes the code smaller and possibly easier to read.

@laanwj

laanwj Apr 3, 2016

Owner

Thanks, yes I agree with regard to keeping the minimum diff, but indeed it's better to do this small cleanup (which someone is bound to do later) at the same time.

Owner

laanwj commented Apr 3, 2016

Tested ACK 4444806

@laanwj laanwj merged commit 4444806 into bitcoin:master Apr 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 3, 2016

Merge #7778: [qa] Bug fixes and refactor
4444806 [qa] mininode: Combine struct.pack format strings (MarcoFalke)
faaa3c9 [qa] mininode: Catch exceptions in got_data (MarcoFalke)
fa2cea1 [qa] rpc-tests: Properly use integers, floats (MarcoFalke)
fa524d9 [qa] Use python2/3 syntax (MarcoFalke)

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1604-qaFixesRefactor branch Apr 3, 2016

btcdrak added a commit to btcdrak/bitcoin that referenced this pull request Apr 3, 2016

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Apr 5, 2016

zander added a commit to zander/bitcoinclassic that referenced this pull request Jun 16, 2016

zander added a commit to zander/bitcoinclassic that referenced this pull request Jun 16, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 11, 2016

[qa] Bug fixes and refactor
nomnombtc: minus changes to maxuploadtarget.py

Github-Pull: #7778
Rebased-From: fa524d9 fa2cea1 faaa3c9 4444806

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

[qa] Bug fixes and refactor
nomnombtc: minus changes to maxuploadtarget.py

Github-Pull: #7778
Rebased-From: fa524d9 fa2cea1 faaa3c9 4444806

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 13, 2016

[qa] Bug fixes and refactor
nomnombtc: minus changes to maxuploadtarget.py

Github-Pull: #7778
Rebased-From: fa524d9 fa2cea1 faaa3c9 4444806

sickpig added a commit to sickpig/BitcoinUnlimited that referenced this pull request Nov 14, 2016

rebroad pushed a commit to rebroad/bitcoin that referenced this pull request Dec 7, 2016

@dagurval dagurval referenced this pull request in bitcoinxt/bitcoinxt Aug 21, 2017

Merged

Use python2/3 syntax #249

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment