Skip to content

[qa] Handle zombies and cluttered tmpdirs #9098

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
Merged
merged 2 commits into from
Nov 8, 2016
Merged

[qa] Handle zombies and cluttered tmpdirs #9098

merged 2 commits into from
Nov 8, 2016

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 7, 2016

The first commit will make the tests fail and print a warning when the tmpdir exists.

The second commit will make the tests skip over any existing zombie daemons with high probability.

Both commit should improve the bad user experience reported in #9086. If the second commit is too controversial, I am happy to drop.

@maflcko maflcko added the Tests label Nov 7, 2016
laanwj
laanwj reviewed Nov 8, 2016
View reviewed changes
qa/pull-tester/rpc-tests.py
@@ -246,14 +246,18 @@ def __init__(self, num_tests_parallel, test_list=None, flags=None):
self.test_list = test_list
self.flags = flags
self.num_running = 0
# In case there is a graveyard of zombie bitcoinds, we can apply a
# psoudorandom offset to hopefully jump over them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pseudorandom :)

@laanwj
Copy link
Member

laanwj commented Nov 8, 2016

utACK, hopefully improves the port-allocation issue in practice without introducing random collisions.

[qa] rpc-tests: Apply random offset to portseed
fab0f07 
This helps to skip over resources, which are blocked by regtest bitcoind
zombie nodes
@maflcko
Copy link
Member Author

maflcko commented Nov 8, 2016

When tests are run in parallel through rpc-tests.py the ports must never collide, otherwise it is a bug. (Running several rpc-tests.py is not supported right now, though)

The second commit should only have an effect when there is at least one zombie instance, which still listens on a port and prevents subsequent tests from executing.

@laanwj
Copy link
Member

laanwj commented Nov 8, 2016

When tests are run in parallel through rpc-tests.py the ports must never collide, otherwise it is a bug.

Agreed.
(I guess there's the case of collisions with random other software listening on ports, but avoiding this is extremely difficult and this change doesn't make it statistically worse)

@laanwj laanwj merged commit fab0f07 into bitcoin:master Nov 8, 2016
laanwj added a commit that referenced this pull request Nov 8, 2016
@laanwj
Merge #9098: [qa] Handle zombies and cluttered tmpdirs
71ab476 
fab0f07 [qa] rpc-tests: Apply random offset to portseed (MarcoFalke)
fae19aa [qa] test_framework: Exit when tmpdir exists (MarcoFalke)
@maflcko maflcko deleted the Mf1611-qaZombies branch November 8, 2016 10:00
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
@luke-jr
[qa] test_framework: Exit when tmpdir exists
ecd7db5 
Github-Pull: bitcoin#9098
Rebased-From: fae19aa
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
@luke-jr
[qa] rpc-tests: Apply random offset to portseed
58eab24 
This helps to skip over resources, which are blocked by regtest bitcoind
zombie nodes

Github-Pull: bitcoin#9098
Rebased-From: fab0f07
sickpig referenced this pull request in sickpig/BitcoinUnlimited Aug 16, 2017
@sickpig
[qa] rpc-tests: Apply random offset to portseed
32ed8fe 
This helps to skip over resources, which are blocked by regtest bitcoind
zombie nodes

Github-Pull: #9098
Rebased-From: fab0f07
codablock pushed a commit to codablock/dash that referenced this pull request Jan 15, 2018
@laanwj @codablock
Merge bitcoin#9098: [qa] Handle zombies and cluttered tmpdirs
1121f2f 
fab0f07 [qa] rpc-tests: Apply random offset to portseed (MarcoFalke)
fae19aa [qa] test_framework: Exit when tmpdir exists (MarcoFalke)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
@laanwj @andvgal
Merge bitcoin#9098: [qa] Handle zombies and cluttered tmpdirs
1e396ec 
fab0f07 [qa] rpc-tests: Apply random offset to portseed (MarcoFalke)
fae19aa [qa] test_framework: Exit when tmpdir exists (MarcoFalke)
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request Feb 24, 2019
@laanwj @CryptoCentric
Merge bitcoin#9098: [qa] Handle zombies and cluttered tmpdirs
8556322 
fab0f07 [qa] rpc-tests: Apply random offset to portseed (MarcoFalke)
fae19aa [qa] test_framework: Exit when tmpdir exists (MarcoFalke)
@str4d str4d mentioned this pull request Nov 21, 2020
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Dec 2, 2020
@zkbot
Auto merge of #4874 - str4d:rpc-tests-py, r=str4d
7a0a268 
Backport migration from rpc-tests.sh to rpc-tests.py

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6567
- bitcoin/bitcoin#6523
- bitcoin/bitcoin#6616
- bitcoin/bitcoin#6788
  - Only the commit fixing `rpc-tests.py`
- bitcoin/bitcoin#6791
  - Only the fix to `qa/rpc-tests/README.md`
- bitcoin/bitcoin#6827
- bitcoin/bitcoin#6930
- bitcoin/bitcoin#6804
- bitcoin/bitcoin#7029
- bitcoin/bitcoin#7028
- bitcoin/bitcoin#7027
- bitcoin/bitcoin#7135
- bitcoin/bitcoin#7209
- bitcoin/bitcoin#7635
- bitcoin/bitcoin#7778
- bitcoin/bitcoin#7851
- bitcoin/bitcoin#7814
  - Only the changes to the new .py files in this PR.
- bitcoin/bitcoin#7971
- bitcoin/bitcoin#7972
- bitcoin/bitcoin#8056
  - Only the first commit.
- bitcoin/bitcoin#8098
- bitcoin/bitcoin#8104
- bitcoin/bitcoin#8133
  - Only the `rpc-tests.py` commit.
- bitcoin/bitcoin#8066
- bitcoin/bitcoin#8216
  - Only the last two commits.
- bitcoin/bitcoin#8254
- bitcoin/bitcoin#8400
- bitcoin/bitcoin#8482
  - Excluding the first commit (only affects RPC tests we don't have).
- bitcoin/bitcoin#8551
- bitcoin/bitcoin#8607
  - Only the pull-tester commit, for conflict removal.
- bitcoin/bitcoin#8625
- bitcoin/bitcoin#8713
- bitcoin/bitcoin#8750
- bitcoin/bitcoin#8789
- bitcoin/bitcoin#9098
- bitcoin/bitcoin#9276
  - Excluding the second commit (we don't have the changes it requires).
- bitcoin/bitcoin#9657
- bitcoin/bitcoin#9807
- bitcoin/bitcoin#9766
- bitcoin/bitcoin#9823
zkbot added a commit to zcash/zcash that referenced this pull request Dec 2, 2020
@zkbot
Auto merge of #4874 - str4d:rpc-tests-py, r=str4d
c8896f9 
Backport migration from rpc-tests.sh to rpc-tests.py

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6567
- bitcoin/bitcoin#6523
- bitcoin/bitcoin#6616
- bitcoin/bitcoin#6788
  - Only the commit fixing `rpc-tests.py`
- bitcoin/bitcoin#6791
  - Only the fix to `qa/rpc-tests/README.md`
- bitcoin/bitcoin#6827
- bitcoin/bitcoin#6930
- bitcoin/bitcoin#6804
- bitcoin/bitcoin#7029
- bitcoin/bitcoin#7028
- bitcoin/bitcoin#7027
- bitcoin/bitcoin#7135
- bitcoin/bitcoin#7209
- bitcoin/bitcoin#7635
- bitcoin/bitcoin#7778
- bitcoin/bitcoin#7851
- bitcoin/bitcoin#7814
  - Only the changes to the new .py files in this PR.
- bitcoin/bitcoin#7971
- bitcoin/bitcoin#7972
- bitcoin/bitcoin#8056
  - Only the first commit.
- bitcoin/bitcoin#8098
- bitcoin/bitcoin#8104
- bitcoin/bitcoin#8133
  - Only the `rpc-tests.py` commit.
- bitcoin/bitcoin#8066
- bitcoin/bitcoin#8216
  - Only the last two commits.
- bitcoin/bitcoin#8254
- bitcoin/bitcoin#8400
- bitcoin/bitcoin#8482
  - Excluding the first commit (only affects RPC tests we don't have).
- bitcoin/bitcoin#8551
- bitcoin/bitcoin#8607
  - Only the pull-tester commit, for conflict removal.
- bitcoin/bitcoin#8625
- bitcoin/bitcoin#8713
- bitcoin/bitcoin#8750
- bitcoin/bitcoin#8789
- bitcoin/bitcoin#9098
- bitcoin/bitcoin#9276
  - Excluding the second commit (we don't have the changes it requires).
- bitcoin/bitcoin#9657
- bitcoin/bitcoin#9807
- bitcoin/bitcoin#9766
- bitcoin/bitcoin#9823
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laanwj laanwj laanwj left review comments

Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maflcko @laanwj