test: add coverage for invalid parameters for `rescanblockchain` #25906

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2022-08-add-test-rescanblockchain changing 1 files +7 −0

brunoerg commented at 8:19 PM on August 22, 2022: contributor

This PR adds test coverage for the following errors: https://github.com/bitcoin/bitcoin/blob/2bd9aa5a44b88c866c4d98f8a7bf7154049cba31/src/wallet/rpc/transactions.cpp#L880-L894
fanquake added the label Tests on Aug 22, 2022
aureleoules commented at 10:20 AM on August 23, 2022: member

ACK 32cd1441d7e178b14ff63154585eaf0f38104cae - i verified this case was not tested.

Maybe outside the scope of this PR but this also seems to be untested. https://github.com/bitcoin/bitcoin/blob/2bd9aa5a44b88c866c4d98f8a7bf7154049cba31/src/wallet/rpc/transactions.cpp#L896-L899
w0xlt approved
w0xlt commented at 1:55 PM on August 23, 2022: contributor

ACK https://github.com/bitcoin/bitcoin/pull/25906/commits/32cd1441d7e178b14ff63154585eaf0f38104cae
brunoerg commented at 2:09 PM on August 23, 2022: contributor

@aureleoules

Maybe outside the scope of this PR but this also seems to be untested.

Cool, I can work on it for other PR. Thanks

in test/functional/wallet_transactiontime_rescan.py:161 in 32cd1441d7 outdated

 158 | @@ -158,5 +159,10 @@ def run_test(self):
 159 |                  assert_equal(tx['time'], cur_time + ten_days + ten_days + ten_days)
 160 |  
 161 |

kristapsk commented at 2:42 PM on August 23, 2022:

Add some log message here? For example:

        self.log.info('Test handling of invalid parameters for rescanblockchain')

brunoerg commented at 4:50 PM on August 23, 2022:

Make sense, done!

brunoerg force-pushed on Aug 23, 2022
brunoerg commented at 4:50 PM on August 23, 2022: contributor

Force pushed addressing @kristapsk's review.
kristapsk approved
kristapsk commented at 5:12 PM on August 23, 2022: contributor

ACK bd47bb56681b0cc0b0c49e9b4e7f3d6e2ae5ad3e
w0xlt approved
w0xlt commented at 7:57 PM on August 23, 2022: contributor

reACK https://github.com/bitcoin/bitcoin/pull/25906/commits/bd47bb56681b0cc0b0c49e9b4e7f3d6e2ae5ad3e

But there is a lint error in CI.
test: add coverage for invalid parameters for `rescanblockchain` d1a0004621
brunoerg force-pushed on Aug 23, 2022
brunoerg commented at 10:07 PM on August 23, 2022: contributor

But there is a lint error in CI.

Thanks, @w0xlt. It was fixed in last force-pushed.
w0xlt approved
w0xlt commented at 3:24 AM on August 24, 2022: contributor

reACK https://github.com/bitcoin/bitcoin/pull/25906/commits/d1a00046214c02684438adcfcd23eea39b86bc7f

CI error in Win64 native [vs2022] check seems unrelated.
MarcoFalke merged this on Aug 24, 2022
MarcoFalke closed this on Aug 24, 2022
sidhujag referenced this in commit 67425f4561 on Aug 24, 2022
bitcoin locked this on Aug 24, 2023

Contributors

Labels