test: Fix combine_logs.py for AppVeyor build #16973

pull mzumsande wants to merge 1 commits into bitcoin:master from mzumsande:201909_test_pathsep changing 1 files +5 −5

mzumsande commented at 9:13 PM on September 26, 2019: member

Fixes #16894

This fixes the problem of AppVeyor builds not showing debug.log if a functional test fails, because the windows separator \ doesn't work together with the regex in combine_logs.py.

A fix was already attempted in #16896, however, that PR became inactive and was marked "up for grabs", plus it's a really small change.

As suggested by jamesob, this PR uses pathlib: For the glob and to convert the path to a posix-style string, it leaves the regex as is (in contrast to #16896 which adjusted the regex).

I tested this locally on Windows and Ubuntu.

in test/functional/combine_logs.py:80 in 5a384e5146 outdated

  76 | @@ -78,10 +77,11 @@ def read_logs(tmp_dir):
  77 |      for each of the input log files."""
  78 |  
  79 |      # Find out what the folder is called that holds the debug.log file
  80 | -    chain = glob.glob("{}/node0/*/debug.log".format(tmp_dir))
  81 | -    if chain:
  82 | -        chain = chain[0]  # pick the first one if more than one chain was found (should never happen)
  83 | -        chain = re.search('node0/(.+?)/debug\.log$', chain).group(1)  # extract the chain name
  84 | +    chain = ''

MarcoFalke commented at 9:26 PM on September 26, 2019:

Python doesn't have scope "blocks", so this is not needed. chain is in this functions' scope after assignment

mzumsande commented at 9:35 PM on September 26, 2019:

Thanks, removed the line.

mzumsande force-pushed on Sep 26, 2019
mzumsande force-pushed on Sep 26, 2019
mzumsande force-pushed on Sep 26, 2019
DrahtBot added the label Tests on Sep 26, 2019

in test/functional/combine_logs.py:81 in 88be9c9671 outdated

  76 | @@ -78,10 +77,10 @@ def read_logs(tmp_dir):
  77 |      for each of the input log files."""
  78 |  
  79 |      # Find out what the folder is called that holds the debug.log file
  80 | -    chain = glob.glob("{}/node0/*/debug.log".format(tmp_dir))
  81 | -    if chain:
  82 | -        chain = chain[0]  # pick the first one if more than one chain was found (should never happen)
  83 | -        chain = re.search(r'node0/(.+?)/debug\.log$', chain).group(1)  # extract the chain name
  84 | +    # pick the first one if more than one chain was found (should never happen)
  85 | +    debugpath = next(pathlib.Path(tmp_dir).glob('node0/**/debug.log'),None)

promag commented at 11:06 PM on September 26, 2019:

nit, space after ,.

in test/functional/combine_logs.py:83 in 88be9c9671 outdated

  82 | -        chain = chain[0]  # pick the first one if more than one chain was found (should never happen)
  83 | -        chain = re.search(r'node0/(.+?)/debug\.log$', chain).group(1)  # extract the chain name
  84 | +    # pick the first one if more than one chain was found (should never happen)
  85 | +    debugpath = next(pathlib.Path(tmp_dir).glob('node0/**/debug.log'),None)
  86 | +    if debugpath:
  87 | +        chain = re.search(r'node0/(.+?)/debug\.log$', debugpath.as_posix()).group(1)  # extract the chain name

promag commented at 11:14 PM on September 26, 2019:

nit,

glob = pathlib.Path(tmp_dir).glob('node0/**/debug.log')
path = next(glob, None)
if path:
    assert next(glob, None) == None      # should never happen
    chain = ...
    ...

mzumsande commented at 10:37 AM on September 27, 2019:

Done, that makes it a bit more readable. Also added assert.

promag commented at 11:17 PM on September 26, 2019: member

ACK, just a nit suggestion.

in test/functional/combine_logs.py:83 in 88be9c9671 outdated

  76 | @@ -78,10 +77,10 @@ def read_logs(tmp_dir):
  77 |      for each of the input log files."""
  78 |  
  79 |      # Find out what the folder is called that holds the debug.log file
  80 | -    chain = glob.glob("{}/node0/*/debug.log".format(tmp_dir))
  81 | -    if chain:
  82 | -        chain = chain[0]  # pick the first one if more than one chain was found (should never happen)

fjahr commented at 9:50 AM on September 27, 2019:

Nit, not critical here but if it should never happen I would throw an exception instead

mzumsande commented at 10:37 AM on September 27, 2019:

I added an assert, as suggested above.

fjahr commented at 9:52 AM on September 27, 2019: member

ACK tested on macOS
mzumsande force-pushed on Sep 27, 2019
test: Fix combine_logs.py for AppVeyor build d478a472eb
mzumsande force-pushed on Sep 27, 2019
thijstriemstra approved
MarcoFalke commented at 5:19 PM on October 10, 2019: member

Sorry, forgot about this. I thought it was already merged.
MarcoFalke referenced this in commit d5a770b70d on Oct 10, 2019
MarcoFalke merged this on Oct 10, 2019
MarcoFalke closed this on Oct 10, 2019

in test/functional/combine_logs.py:80 in d478a472eb

  76 | @@ -78,10 +77,11 @@ def read_logs(tmp_dir):
  77 |      for each of the input log files."""
  78 |  
  79 |      # Find out what the folder is called that holds the debug.log file
  80 | -    chain = glob.glob("{}/node0/*/debug.log".format(tmp_dir))
  81 | -    if chain:
  82 | -        chain = chain[0]  # pick the first one if more than one chain was found (should never happen)
  83 | -        chain = re.search(r'node0/(.+?)/debug\.log$', chain).group(1)  # extract the chain name
  84 | +    glob = pathlib.Path(tmp_dir).glob('node0/**/debug.log')

MarcoFalke commented at 5:31 PM on October 10, 2019:

Is this recursive on purpose or would 'node0/*/debug.log' suffice?

mzumsande commented at 12:10 AM on October 11, 2019:

Not on purpose. Non-recursive would suffice, I can't imagine a scenario where debug.log ends up in any subdirectories of node0.

sidhujag referenced this in commit ef6d2956e9 on Oct 11, 2019
mzumsande deleted the branch on May 10, 2020
jasonbcox referenced this in commit ee0e7ccc93 on Oct 28, 2020
Munkybooty referenced this in commit f7624c6ca7 on Oct 7, 2021
Munkybooty referenced this in commit 7a3163e7a1 on Oct 14, 2021
Munkybooty referenced this in commit daf0ebcd5e on Oct 16, 2021
Munkybooty referenced this in commit b69cc33f99 on Feb 1, 2022
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#16894 AppVeyor CI: combine_logs.py fails to fetch debug logs #16896 test: replaced hard coded folders with os.path.join & replaced os.path.sep