[qa] combine_logs: Use ordered list for logfiles #10076

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1703-orderedLog changing 1 files +5 −2

MarcoFalke commented at 2:11 PM on March 25, 2017: member

According to the documentation of glob, "results are returned in arbitrary order". Thus, create each path separately by string formatting.
MarcoFalke added the label Tests on Mar 25, 2017
MarcoFalke assigned jnewbery on Mar 25, 2017
MarcoFalke unassigned jnewbery on Mar 25, 2017
MarcoFalke requested review from jnewbery on Mar 25, 2017

in test/functional/combine_logs.py:13 in fa32e5c16b outdated

   5 | @@ -6,11 +6,11 @@
   6 |  
   7 |  import argparse
   8 |  from collections import defaultdict, namedtuple
   9 | -import glob
  10 |  import heapq
  11 |  import os
  12 |  import re
  13 |  import sys
  14 | +from test_framework.util import MAX_NODES

jnewbery commented at 10:34 PM on March 25, 2017:

micronit: blank line between the standard library imports and local project imports please.

MarcoFalke commented at 9:41 AM on March 28, 2017:

@jnewbery Removed this import

jnewbery commented at 10:34 PM on March 25, 2017: member

Good catch! Tested ACK https://github.com/bitcoin/bitcoin/pull/10076/commits/fa32e5c16b7f281c500ef6064e6d0e0329465c74 with a micronit.
jnewbery approved
jnewbery commented at 12:51 PM on March 26, 2017: member

Actually, thinking about this some more, I don't think I like the dependency on test_framework.utils here. At the moment combine_logs can be run from anywhere, even on a system that doesn't have bitcoin or the test_framework installed. You can tar the tmp directory, upload it somewhere and then do the post-processing on another system. I'd like to preserve that if possible.

Can you change this to use glob and then inspect the name of the directory to get the node index?
jnewbery changes_requested
jnewbery commented at 12:51 PM on March 26, 2017: member

Please remove dependency on test_framework
[qa] combine_logs: Use ordered list for logfiles fa4535df57
MarcoFalke force-pushed on Mar 27, 2017
jnewbery approved
jnewbery commented at 12:42 PM on March 28, 2017: member

Looks great. Thanks Marco.

Tested ACK fa4535df574067700ed7d2227ae03605075b8730
MarcoFalke merged this on Mar 28, 2017
MarcoFalke closed this on Mar 28, 2017
MarcoFalke referenced this in commit 5b029aaedb on Mar 28, 2017
MarcoFalke deleted the branch on Mar 28, 2017
PastaPastaPasta referenced this in commit e51905e481 on Mar 14, 2019
PastaPastaPasta referenced this in commit f00a1cb03d on May 20, 2019
PastaPastaPasta referenced this in commit bfd63c179d on May 21, 2019
PastaPastaPasta referenced this in commit b1ad029030 on May 21, 2019
barrystyle referenced this in commit d7ad21feba on Jan 22, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

MarcoFalke

jnewbery

Labels

Tests