Prefer wait_until over polling with time.sleep #12553

pull Empact wants to merge 2 commits into bitcoin:master from Empact:wait-until changing 5 files +16 −37

Empact commented at 6:37 AM on February 27, 2018: member

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.
Empact force-pushed on Feb 27, 2018
laanwj added the label Tests on Feb 27, 2018

in test/functional/feature_pruning.py:82 in f873046962 outdated

  82 | -        while os.path.isfile(self.prunedir+"blk00000.dat"):
  83 | -            time.sleep(0.1)
  84 | -            if time.time() - waitstart > 30:
  85 | -                raise AssertionError("blk00000.dat not pruned when it should be")
  86 | +        # Wait for blk00000.dat to be pruned
  87 | +        wait_until(lambda: not os.path.isfile(self.prunedir+"blk00000.dat"), timeout=30)

conscott commented at 2:13 PM on February 27, 2018:

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.

Empact commented at 10:56 PM on February 27, 2018:

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

Empact force-pushed on Feb 27, 2018
conscott commented at 5:34 PM on February 28, 2018: contributor

utACK 3a1fc66cd09ff9bb1cda68683e855b507409aabf
laanwj assigned MarcoFalke on Mar 6, 2018

in test/functional/feature_pruning.py:258 in 3a1fc66cd0 outdated

 254 | @@ -262,7 +255,7 @@ def prune(index, expected_ret=None):
 255 |                  assert_equal(ret, expected_ret)
 256 |  
 257 |          def has_block(index):
 258 | -            return os.path.isfile(self.options.tmpdir + "/node{}/regtest/blocks/blk{:05}.dat".format(node_number, index))
 259 | +            return os.path.isfile(os.path.join(self.options.tmpdir, "node{}".format(node_number), "regtest", "blocks", "blk{:05}.dat".format(index)))

MarcoFalke commented at 12:45 AM on March 13, 2018:

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)))

Empact force-pushed on Mar 13, 2018
test: Use wait_until in tests where time was used for polling 81b0822772
test: Use os.path.join consistently in feature_pruning tests 9d7f839a20
Empact force-pushed on Mar 16, 2018
MarcoFalke commented at 8:53 PM on March 17, 2018: member

utACK 9d7f839a206b7392aebfa436ae0f9a29d33d2960
laanwj merged this on Mar 19, 2018
laanwj closed this on Mar 19, 2018
laanwj referenced this in commit 93634f296e on Mar 19, 2018
Empact deleted the branch on Apr 13, 2018
MarcoFalke referenced this in commit 06f6bbf64f on Apr 20, 2018
MarcoFalke referenced this in commit 0e98f96e42 on Apr 20, 2018
HashUnlimited referenced this in commit 107d686cc3 on May 13, 2018
ccebrecos referenced this in commit f62300bf33 on Sep 14, 2018
PastaPastaPasta referenced this in commit bebfc238a4 on Mar 14, 2020
PastaPastaPasta referenced this in commit 5c7c972bcc on Mar 19, 2020
PastaPastaPasta referenced this in commit b40bf71a85 on Mar 21, 2020
PastaPastaPasta referenced this in commit 8bf0a5c3c7 on Mar 24, 2020
ckti referenced this in commit 994fac81b6 on Mar 28, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12545 test: Use wait_until to ensure ping goes out