test: improve functional tests compatibility with BSD/macOS #19368

S3RK commented at 2:32 AM on June 24, 2020: member

Rationale: a few minor changes to make experience of running tests on macOS a bit better 1.pidof is not available on BSD/macOS, while pgrep is present on BSD, Linux and macOS 2. Add retry as a workaround for a weird behavior when writing to a socket (https://bugs.python.org/issue33450). Stacktrace attached

Man pages: https://www.freebsd.org/cgi/man.cgi?query=pgrep&apropos=0&sektion=1&manpath=FreeBSD+6.0-RELEASE&arch=default&format=html https://man7.org/linux/man-pages/man1/pgrep.1.html

Related to #19281

Stacktrace example:

...
33/161 - feature_abortnode.py failed, Duration: 63 s

stdout:
2020-06-11T10:46:43.947000Z TestFramework (INFO): Initializing test directory /var/folders/2q/d5w9zh614r7g5c8r74ln3g400000gq/T/test_runner_₿_🏃_20200611_174102/feature_abortnode_128
2020-06-11T10:46:45.199000Z TestFramework (INFO): Waiting for crash
2020-06-11T10:47:15.921000Z TestFramework (INFO): Node crashed - now verifying restart fails
2020-06-11T10:47:47.068000Z TestFramework (INFO): Stopping nodes
[node 1] Cleaning up leftover process


stderr:
Traceback (most recent call last):
  File "/Users/xxx/Projects/bitcoin/test/functional/feature_abortnode.py", line 50, in <module>
    AbortNodeTest().main()
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main
    exit_code = self.shutdown()
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 266, in shutdown
    self.stop_nodes()
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 515, in stop_nodes
    node.stop_node(wait=wait)
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_node.py", line 318, in stop_node
    self.stop(wait=wait)
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 142, in __call__
    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1107, in request
    self._send_request(method, url, body, headers)
  File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1152, in _send_request
    self.endheaders(body)
  File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1103, in endheaders
    self._send_output(message_body)
  File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 936, in _send_output
    self.send(message_body)
  File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 908, in send
    self.sock.sendall(data)
OSError: [Errno 41] Protocol wrong type for socket

test: use pgrep for better compatibility

pidof is not available on BSD system, while pgrep is present on BSD, Linux and macOS

8cf9d15b82

fanquake added the label Tests on Jun 24, 2020

test: retry when write to a socket fails on macOS

If the socket is tearing down macOS will return EPROTOTYPE instead of EPIPE.
Because python doesn't handle this internally we have to do a workaround and retry the request.
See https://bugs.python.org/issue33450

3a7e79478a

MarcoFalke commented at 11:01 AM on June 25, 2020: member

Oh nice. Does that mean we can enable the tests again?

https://github.com/bitcoin/bitcoin/blob/67881de0e3b1cef1d0f978582765a8aeeb09c21a/ci/test/00_setup_env_mac_host.sh#L14

S3RK commented at 2:29 AM on June 26, 2020: member

@MarcoFalke oh, I didn't even know we had already experienced this problem, I thought it's just me 😄 I've looked at the comment and related travis logs and indeed the error is the same as in my stacktrace.

From what I understood the behaviour we see on macOS is similar to BrokenPipeError and ConnectionResetError on linux/BSD, it's also triggered by reset connection. So It's reasonable to expect that the same workaround (reconnect and retry) would work. Though it is still a mystery to me why the connection would be reset in the first place, but I'm afraid that's beyond my understanding.

I think we can try to enable the tests again. Would you suggest to have some prior discussion on IRC maybe or just re-enable the tests in this PR and let reviewers decide?

MarcoFalke commented at 11:59 AM on June 26, 2020: member

just re-enable the tests in this PR and let reviewers decide?

That seems fine

in test/functional/test_runner.py:403 in 8cf9d15b82 outdated

 401 | -        if subprocess.check_output(["pidof", "bitcoind"]) not in [b'']:
 402 | +        # pgrep exits with code zero when one or more matching processes found
 403 | +        if subprocess.run(["pgrep", "-x", "bitcoind"], stdout=subprocess.DEVNULL).returncode == 0:
 404 |              print("%sWARNING!%s There is already a bitcoind process running on this system. Tests may fail unexpectedly due to resource contention!" % (BOLD[1], BOLD[0]))
 405 | -    except (OSError, subprocess.SubprocessError):
 406 | +    except OSError:

laanwj commented at 2:56 PM on June 29, 2020:

~~Why remove the catch for subprocess.SubprocessError?~~ Edit: I understand now, the semantics of subprocess.run are different in that it never raises subprocess.SubProcesserror, so this is enough to keep the same behavior as before.

laanwj commented at 3:02 PM on June 29, 2020: member

ACK 3a7e79478ab41af7c53ce14d9fca9815bffe1f73 Verified that the pgrep works on FreeBSD whereas pidof does not. It's somewhat of a pity that we need to rely on a shell commend being present here at all, but for this single optional check it's also not worth introducing an dependency on, say, the psutil module.

laanwj commented at 3:07 PM on June 29, 2020: member

Though it is still a mystery to me why the connection would be reset in the first place, but I'm afraid that's beyond my understanding.

There are various edge cases where a connection reset can happen. For example at shutdown it's not guaranteed that stop RPC command will return a value. It usually does, but on a busy system it might not, and the RPC server is shut down before it manages to send the reply.

laanwj merged this on Jul 1, 2020

laanwj closed this on Jul 1, 2020

S3RK deleted the branch on Jul 1, 2020

deadalnix referenced this in commit 27ed27aa11 on Aug 28, 2021

DrahtBot locked this on Feb 15, 2022