Skip to content
New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: Fix intermitent failure in wallet_basic.py #23141

Closed
wants to merge 1 commit into from
Closed

test: Fix intermitent failure in wallet_basic.py #23141

wants to merge 1 commit into from

Conversation

meshcollider
Copy link
Contributor

assert_fee_amount currently uses round() when calculating the fee rate, which causes intermittent failure in wallet_basic.py:
https://cirrus-ci.com/task/5721255786577920?logs=ci#L145

2021-09-29T23:28:41.198000Z TestFramework (INFO): Test sendtoaddress with fee_rate param (explicit fee rate in sat/vB)
2021-09-29T23:28:49.992000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
    self.run_test()
  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_basic.py", line 500, in run_test
    assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb))
  File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 41, in assert_fee_amount
    raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee)))
AssertionError: Fee of 0.00000255 BTC too low! (Should be 0.00000256 BTC)

We should truncate rather than rounding.

This would also be fixed by #22949 but it makes more sense to make the test consistent with the current code.

@maflcko maflcko changed the title Fix intermitent failure in wallet_basic.py test: Fix intermitent failure in wallet_basic.py Sep 30, 2021
@jonatack
Copy link
Member

jonatack commented Sep 30, 2021 •
edited
Loading

See #23136.

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23136 (test: update fee rate assertion helper in the functional test framework by jonatack)
  • #22364 (wallet: Make a tr() descriptor by default by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Sep 30, 2021
Merged
@meshcollider
Copy link
Contributor Author

Closing as duplicate of #23136

@DrahtBot DrahtBot mentioned this pull request Oct 1, 2021
Merged
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@meshcollider @jonatack @DrahtBot @maflcko