test: bump test timeouts so that functional tests run in valgrind #17770

michiboo · 2019-12-18T06:56:38Z

ci/tests: Bump timeouts so all functional tests run on travis in valgrind #17763

test/functional/feature_block.py

test/functional/interface_rest.py

ci/test/00_setup_env_native_valgrind.sh

test/functional/interface_rest.py

maflcko · 2019-12-25T01:04:10Z

Last failure: https://travis-ci.org/bitcoin/bitcoin/jobs/629019510#L3660

Re-run ci, to see if it is reproducible.

ci/test/00_setup_env_native_valgrind.sh

test/functional/feature_abortnode.py

maflcko · 2020-01-09T23:04:38Z

To fix the wallet_groups timeout, you might need a diff that looks like:

diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py
index 3cf8aaf3d..f2fa1d3e4 100755
--- a/test/functional/wallet_groups.py
+++ b/test/functional/wallet_groups.py
@@ -16,7 +16,7 @@ class WalletGroupTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 3
         self.extra_args = [[], [], ['-avoidpartialspends']]
-        self.rpc_timeout = 120
+        self.rpc_timeout = 240
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()

DrahtBot · 2020-01-14T09:34:00Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16539 (wallet: lower -txmaxfee default from 0.1 to 0.01 BTC by Sjors)

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.

ci/test/00_setup_env_native_valgrind.sh

maflcko · 2020-01-15T16:24:45Z

Looks like feature_assumevalid needs the self.rpc_timeout = 240 bumped as well. Also, there are about 10 tests left which don't run. You could remove the --failfast in ci/test/06_script_b.sh temporarily to run them all even if one fails.

maflcko · 2020-01-20T13:49:03Z

Ok, down to three failures 🚀

feature_assumevalid.py | ✖ Failed | 206 s
feature_cltv.py | ✖ Failed | 205 s

I think they can be excluded for now because they mine too many blocks

feature_help.py | ✖ Failed | 5 s

Timeout should be bumped here, probably

practicalswift · 2020-01-21T16:07:14Z

Concept ACK

Nice first-time contribution! Welcome as a contributor @michiboo :)

maflcko · 2020-01-21T17:32:23Z

ok, one more timeout on travis. Might want to bump that and remove the temporary workarounds in the ci files (failfast and UNIT_TESTS=false).

Then it should be ready for merge.

maflcko · 2020-01-21T17:34:33Z

Also could adjust the commit title to match the pull request title, which is a bit more descriptive.

michiboo · 2020-01-22T05:32:20Z

Hi There is error: The job exceeded the maximum time limit for jobs, and has been terminated.

Not sure how to avoid this.

maflcko

ACK. Let's look at travis some time later, but I'd like to get the bumped timeouts in, so that I can start running valgrind on my infrastructure.

maflcko · 2020-01-23T21:50:13Z

ci/test/00_setup_env_native_valgrind.sh

ok, then maybe revert the changes to this file altogether, but I'd still like to get this merged. The timeout changes help me running the valgrind tests on my own ci infrastructure. We can look into how to enable them on travis later on.

ci/test/00_setup_env_native_valgrind.sh

…n valgrind 2d23082 bump test timeouts so that functional tests run in valgrind (Micky Yun Chan) Pull request description: ci/tests: Bump timeouts so all functional tests run on travis in valgrind #17763 Top commit has no ACKs. Tree-SHA512: 5a8c6e2ea02b715facfcb58c761577be15ae58c45a61654beb98c2c2653361196c2eec521bcae4a9a1bab8e409d6807de771ef4c46d3d05996ae47a22d499d54

This was added in bitcoin#17770, but is identical to the supression above.

d7120f7 valgrind : remove duplicate BCLog::Logger suppression (fanquake) 708e3c7 valgrind: remove rest_blockhash_by_height suppression (fanquake) Pull request description: bitcoin@708e3c7: `Suppress rest_blockhash_by_height` should no longer be needed after bitcoin#18785. bitcoin@d7120f7 : Removes a duplicate `Suppress BCLog::Logger::StartLogging()` suppression that was added in bitcoin#17770. ACKs for top commit: MarcoFalke: ACK d7120f7 practicalswift: ACK d7120f7 -- patch looks correct and valgrind Travis job is happy Tree-SHA512: 45f5b9fa64bf83cada3cd9ad33c245f660376d5b29f51a2531d83133940090df945f5ef26c5847d6ec024ffab9528d55573c5cf9ca5e73795f9abfc971b3d29b

This was added in bitcoin#17770, but is identical to the supression above.

Summary: > Also speed up tx relay while touching the file anyway. This should fix the intermittent error `AssertionError: Mempool sync timed out:` If it doesn't solve the issue completely, we may also need [[bitcoin/bitcoin#17770 | PR17770]], [[bitcoin/bitcoin#18345 | PR18345]] and [[bitcoin/bitcoin#18704 | PR18704]]. These 3 PRs were previous unsuccesful attempts to solve this issue by Core, maybe they are useless or maybe they are part of the solution. This is a backport of Core [[bitcoin/bitcoin#18752 | PR18752]] Test Plan: `ninja && test/functional/test_runner.py mempool_reorg` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Subscribers: majcosta Differential Revision: https://reviews.bitcoinabc.org/D8436

DrahtBot added the Tests label Dec 18, 2019

maflcko reviewed Dec 18, 2019

View reviewed changes

test/functional/feature_block.py Outdated Show resolved Hide resolved

maflcko reviewed Dec 19, 2019

View reviewed changes

test/functional/interface_rest.py Outdated Show resolved Hide resolved

maflcko reviewed Dec 19, 2019

View reviewed changes

ci/test/00_setup_env_native_valgrind.sh Outdated Show resolved Hide resolved

maflcko reviewed Dec 20, 2019

View reviewed changes

test/functional/interface_rest.py Outdated Show resolved Hide resolved

maflcko closed this Dec 25, 2019

maflcko reopened this Dec 25, 2019

maflcko reviewed Dec 25, 2019

View reviewed changes

ci/test/00_setup_env_native_valgrind.sh Outdated Show resolved Hide resolved

maflcko reviewed Jan 3, 2020

View reviewed changes

ci/test/00_setup_env_native_valgrind.sh Outdated Show resolved Hide resolved

test/functional/feature_abortnode.py Outdated Show resolved Hide resolved

fanquake changed the title ~~bump test timeouts so all functional test run on CI (WIP)~~ [WIP] test: bump test timeouts so all functional tests run on CI Jan 8, 2020

maflcko reviewed Jan 14, 2020

View reviewed changes

ci/test/00_setup_env_native_valgrind.sh Outdated Show resolved Hide resolved

maflcko changed the title ~~[WIP] test: bump test timeouts so all functional tests run on CI~~ [WIP] test: bump test timeouts so that functional tests run in valgrind Jan 21, 2020

maflcko changed the title ~~[WIP] test: bump test timeouts so that functional tests run in valgrind~~ test: bump test timeouts so that functional tests run in valgrind Jan 21, 2020

maflcko approved these changes Jan 23, 2020

View reviewed changes

maflcko reviewed Jan 24, 2020

View reviewed changes

ci/test/00_setup_env_native_valgrind.sh Outdated Show resolved Hide resolved

bump test timeouts so that functional tests run in valgrind

2d23082

maflcko merged commit 2d23082 into bitcoin:master Jan 25, 2020

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

fanquake added a commit to fanquake/bitcoin that referenced this pull request May 5, 2020

valgrind : remove duplicate BCLog::Logger suppression

d7120f7

This was added in bitcoin#17770, but is identical to the supression above.

fanquake mentioned this pull request May 5, 2020

valgrind: remove outdated suppressions #18879

Merged

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 5, 2020

valgrind : remove duplicate BCLog::Logger suppression

df07326

This was added in bitcoin#17770, but is identical to the supression above.

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

test: bump test timeouts so that functional tests run in valgrind #17770

test: bump test timeouts so that functional tests run in valgrind #17770

Uh oh!

Conversation

michiboo commented Dec 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Dec 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Jan 9, 2020

Uh oh!

DrahtBot commented Jan 14, 2020

Conflicts

Uh oh!

Uh oh!

maflcko commented Jan 15, 2020

Uh oh!

maflcko commented Jan 20, 2020

Uh oh!

practicalswift commented Jan 21, 2020

Uh oh!

maflcko commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jan 21, 2020

Uh oh!

michiboo commented Jan 22, 2020

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko Jan 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maflcko commented Jan 21, 2020 •

edited

Loading