Skip to content

Conversation

Empact
Copy link
Contributor

@Empact Empact commented Feb 27, 2018

This is prompted by and builds on #12545, a nice cleanup / consolidation of patterns.

In cases where the exception message was meaningful, I tried to represent it as well in a comment.

I expect #12545 will go in first, but I'm happy to squash them if that's preferred.

if time.time() - waitstart > 30:
raise AssertionError("blk00000.dat not pruned when it should be")
# Wait for blk00000.dat to be pruned
wait_until(lambda: not os.path.isfile(self.prunedir+"blk00000.dat"), timeout=30)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is just preserving the previous code, but this should probably be updated to use os.path.join like:

# Before 
os.path.isfile(self.prunedir+"blk00000.dat")  
# After
os.path.isfile(os.path.join(self.prunedir, "blk00000.dat"))

There are several instances of this unrelated to this PR, so perhaps it can just be done in a follow up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks there were a few such cases in this file, but not elsewhere.

@conscott
Copy link
Contributor

utACK 3a1fc66cd09ff9bb1cda68683e855b507409aabf

@@ -262,7 +255,7 @@ def prune(index, expected_ret=None):
assert_equal(ret, expected_ret)

def has_block(index):
return os.path.isfile(self.options.tmpdir + "/node{}/regtest/blocks/blk{:05}.dat".format(node_number, index))
return os.path.isfile(os.path.join(self.options.tmpdir, "node{}".format(node_number), "regtest", "blocks", "blk{:05}.dat".format(index)))
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could use shorter code by referring to TestNode.datadir:

return os.path.isfile(os.path.join(self.nodes[node_number].datadir, 'regtest', 'blocks', 'blk{:05}.dat'.format(index)))

@maflcko
Copy link
Member

maflcko commented Mar 17, 2018

utACK 9d7f839

@laanwj laanwj merged commit 9d7f839 into bitcoin:master Mar 19, 2018
laanwj added a commit that referenced this pull request Mar 19, 2018
9d7f839 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b0822 test: Use wait_until in tests where time was used for polling (Ben Woosley)

Pull request description:

  This is prompted by and builds on #12545, a nice cleanup / consolidation of patterns.

  In cases where the exception message was meaningful, I tried to represent it as well in a comment.

  I expect #12545 will go in first, but I'm happy to squash them if that's preferred.

Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
@Empact Empact deleted the wait-until branch April 13, 2018 11:26
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Apr 20, 2018
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request May 13, 2018
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
9d7f839 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b0822 test: Use wait_until in tests where time was used for polling (Ben Woosley)

Pull request description:

  This is prompted by and builds on bitcoin#12545, a nice cleanup / consolidation of patterns.

  In cases where the exception message was meaningful, I tried to represent it as well in a comment.

  I expect bitcoin#12545 will go in first, but I'm happy to squash them if that's preferred.

Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 19, 2020
9d7f839 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b0822 test: Use wait_until in tests where time was used for polling (Ben Woosley)

Pull request description:

  This is prompted by and builds on bitcoin#12545, a nice cleanup / consolidation of patterns.

  In cases where the exception message was meaningful, I tried to represent it as well in a comment.

  I expect bitcoin#12545 will go in first, but I'm happy to squash them if that's preferred.

Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 21, 2020
9d7f839 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b0822 test: Use wait_until in tests where time was used for polling (Ben Woosley)

Pull request description:

  This is prompted by and builds on bitcoin#12545, a nice cleanup / consolidation of patterns.

  In cases where the exception message was meaningful, I tried to represent it as well in a comment.

  I expect bitcoin#12545 will go in first, but I'm happy to squash them if that's preferred.

Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 24, 2020
9d7f839 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b0822 test: Use wait_until in tests where time was used for polling (Ben Woosley)

Pull request description:

  This is prompted by and builds on bitcoin#12545, a nice cleanup / consolidation of patterns.

  In cases where the exception message was meaningful, I tried to represent it as well in a comment.

  I expect bitcoin#12545 will go in first, but I'm happy to squash them if that's preferred.

Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants