test: fix "bitcoind already running" warnings on macOS #17488

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:macos_pidof_test_runner_warning changing 1 files +3 −2

fanquake commented at 4:48 PM on November 15, 2019: member

On macOS, pidof installed via brew returns b'' rather than None. Account for this, to remove spurious warnings from the test_runner.
fanquake added the label Tests on Nov 15, 2019
in test/functional/test_runner.py:368 in cd6dbd0074 outdated
```
 363 | @@ -364,9 +364,10 @@ def main():
 364 |  def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, runs_ci, use_term_control):
 365 |      args = args or []
 366 |  
 367 | -    # Warn if bitcoind is already running (unix only)
 368 | +    # Warn if bitcoind is already running
 369 | +    # On macOS pidof (installed via brew) returns b''
```
MarcoFalke commented at 5:06 PM on November 15, 2019:

Is this a change that happened due to pidof change or due to a python change?

What does pidof bitoind; echo $? print?
fanquake commented at 5:17 PM on November 15, 2019:

Is this a change that happened due to pidof change or due to a python change?

The pidof source used by brew hasn't changed since 2005, so I'd guess a Python change if anything.

What does pidof bitoind; echo $? print?

pidof bitcoind; echo $? 65657 0
MarcoFalke commented at 5:24 PM on November 15, 2019:

I mean for the case where bitcoind is not running. :)
MarcoFalke commented at 5:25 PM on November 15, 2019:

For reference I get:

$ pidof bitoind; echo $? 1
fanquake commented at 6:52 PM on November 15, 2019:

I get:

$ pidof bitcoind; echo $? 0
MarcoFalke commented at 6:57 PM on November 15, 2019:

Ok, so the issue is with the macos pidof
MarcoFalke commented at 6:58 PM on November 15, 2019:

# pidof might fail or return an empty string if the process is not running

test: fix bitcoind already running warnings on macOS

On macOS, pidof installed via brew returns b'' rather than None.
Account for this, to remove spurious warnings from the test_runner.

1c23ea5fe6

in test/functional/test_runner.py:370 in cd6dbd0074 outdated

 363 | @@ -364,9 +364,10 @@ def main():
 364 |  def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, runs_ci, use_term_control):
 365 |      args = args or []
 366 |  
 367 | -    # Warn if bitcoind is already running (unix only)
 368 | +    # Warn if bitcoind is already running
 369 | +    # On macOS pidof (installed via brew) returns b''
 370 |      try:
 371 | -        if subprocess.check_output(["pidof", "bitcoind"]) is not None:
 372 | +        if subprocess.check_output(["pidof", "bitcoind"]) not in [None, b'']:

MarcoFalke commented at 7:00 PM on November 15, 2019:

I think the None can be removed. check_output should either fail or return an (empty) string.

fanquake force-pushed on Nov 15, 2019
laanwj commented at 1:23 PM on November 18, 2019: member

Right, this was incorrect before, subprocess.check_output will never return None.

ACK 1c23ea5fe67b88fd72a1ff640dd1bbb21a34fbf4
laanwj referenced this in commit ca1eeba0b0 on Nov 18, 2019
laanwj merged this on Nov 18, 2019
laanwj closed this on Nov 18, 2019
fanquake deleted the branch on Nov 18, 2019
fanquake referenced this in commit c0dc728206 on Jan 4, 2020
laanwj referenced this in commit bb123c6527 on Jan 8, 2020
MarkLTZ referenced this in commit 2e6abb1507 on Mar 13, 2020
MarkLTZ referenced this in commit ece88bae51 on Mar 13, 2020
Munkybooty referenced this in commit 861ef02929 on Dec 9, 2021
Munkybooty referenced this in commit fc6f77f098 on Dec 9, 2021
Munkybooty referenced this in commit e32f0d5d86 on Dec 9, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#17858 [0.19] Backports