tests: Re-enable RPC tests for Windows #8227

laanwj commented at 8:14 AM on June 20, 2016: member

Try RPC tests in Travis windows builds.

Just a test, don't merge this yet.

laanwj added the label Tests on Jun 20, 2016

laanwj commented at 9:07 AM on June 20, 2016: member

p2p-versionbits-warning.py fails while shutting down the nodes. ~~This seems to be a randomly occuring issue, not related to the specific test.~~


stderr:
   File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/test_framework.py", line 144, in main
    self.run_test()
  File "/home/travis/build/bitcoin/bitcoin/build/../qa/rpc-tests/p2p-versionbits-warning.py", line 154, in run_test
    self.test_versionbits_in_alert_file()
  File "/home/travis/build/bitcoin/bitcoin/build/../qa/rpc-tests/p2p-versionbits-warning.py", line 99, in test_versionbits_in_alert_file
    assert(self.vb_pattern.match(alert_text))
Traceback (most recent call last):
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 121, in _request
    return self._get_response()
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 159, in _get_response
    http_response = self.__conn.getresponse()
  File "/usr/lib/python3.4/http/client.py", line 1147, in getresponse
    response.begin()
  File "/usr/lib/python3.4/http/client.py", line 351, in begin
    version, status, reason = self._read_status()
  File "/usr/lib/python3.4/http/client.py", line 321, in _read_status
    raise BadStatusLine(line)
http.client.BadStatusLine: ''
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/build/bitcoin/bitcoin/build/../qa/rpc-tests/p2p-versionbits-warning.py", line 161, in <module>
    VersionBitsWarningTest().main()
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/test_framework.py", line 165, in main
    stop_nodes(self.nodes)
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/util.py", line 347, in stop_nodes
    node.stop()
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/coverage.py", line 49, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 144, in __call__
    response = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/home/travis/build/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 125, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.4/http/client.py", line 1065, in request
    self._send_request(method, url, body, headers)
  File "/usr/lib/python3.4/http/client.py", line 1103, in _send_request
    self.endheaders(body)
  File "/usr/lib/python3.4/http/client.py", line 1061, in endheaders
    self._send_output(message_body)
  File "/usr/lib/python3.4/http/client.py", line 906, in _send_output
    self.send(msg)
  File "/usr/lib/python3.4/http/client.py", line 841, in send
    self.connect()
  File "/usr/lib/python3.4/http/client.py", line 819, in connect
    self.timeout, self.source_address)
  File "/usr/lib/python3.4/socket.py", line 509, in create_connection
    raise err
  File "/usr/lib/python3.4/socket.py", line 500, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

~~It looks like this isn't working, or that the http shutdown is taking too long and thus quitting without returning anything: https://github.com/bitcoin/bitcoin/blob/master/src/httpserver.cpp#L485~~

~~Conclusion: not safe to enable P2P tests for windows yet in travis. They tend to work, but this shutdown issue would introduce too many false positives.~~ (this conclusion was wrong, see below)

MarcoFalke commented at 9:11 AM on June 20, 2016: member

Isn't it also failing the assert(self.vb_pattern.match(alert_text))?

laanwj commented at 9:12 AM on June 20, 2016: member

Ok that's one confusing error, yes that one's failing too. That seems specific to this test. Let's see if it is repeatable.

laanwj commented at 10:01 AM on June 20, 2016: member

Okay at least the error is consistent. I retriggered Travis and it happens again, same place, same error.

And the 32-bit windows build fails due to the same reason.

Edit: I can also reproduce it locally in a depends build.

laanwj commented at 10:43 AM on June 20, 2016: member

I've pushed a commit that should get past this problem.

in qa/rpc-tests/p2p-versionbits-warning.py:None in 3d2d6f8f5d outdated

  73 | @@ -74,7 +74,7 @@ def setup_network(self):
  74 |          self.nodes.append(start_node(0, self.options.tmpdir, self.node_options))
  75 |  
  76 |          import re

MarcoFalke commented at 10:51 AM on June 20, 2016:

Nit: I think it is good practice to put those in the header.

laanwj commented at 10:52 AM on June 20, 2016:

Yeah, unless there is a specific reason for late binding (there doesn't seem to be here)

MarcoFalke commented at 3:09 PM on July 5, 2016:

Feel free to fix the nit in this pull.

MarcoFalke commented at 10:51 AM on June 20, 2016: member

Thanks.

If this passes, make sure to remove any remaining -win hacks from the script.

laanwj force-pushed on Jun 20, 2016

laanwj renamed this:
~~tests: Try re-enabling RPC tests for Windows~~
tests: Re-enable RPC tests for Windows
on Jun 20, 2016

laanwj commented at 11:21 AM on June 20, 2016: member

It passed! Reorganized the commits and removed any mention of -win in rpc-tests.py. I may be looking past it, but I didn't find any use of that option anywhere.

MarcoFalke commented at 11:48 AM on June 20, 2016: member

utACK 39d28c6

laanwj commented at 12:39 PM on June 20, 2016: member

Looks like there are timeout issues, run times for the windows tests are on the limit of what travis allows

MarcoFalke commented at 3:04 PM on June 20, 2016: member

I've added a commit to https://github.com/bitcoin/bitcoin/pull/8216/commits/fa58f94ff7f097260ebee791008dab368c7ac318 which should improve that, but I think after enabling qt builds again, we are again at the limit...

laanwj commented at 8:31 AM on June 21, 2016: member

Re-triggered this after merging #8216, let's see if this is more reliable.

tests: Accept quotes around alert message (windows compat)

`p2p-versionbits-warning.py` uses an alertnotify that writes to a file
using `echo`.
Windows `echo` doesn't strip quotes.
So accept the alert message to be surrounded by quotes in the regular
expression.

265adb3b96

tests: Re-enable RPC tests for Windows

Re-enable the RPC tests for Windows. The issues preventing running the
RPC tests on windows have been resolved by upgrading to Trusty. Remove
the `-win` option from `rpc-tests.py`.

dc05f6d1a5

laanwj force-pushed on Jun 21, 2016

MarcoFalke commented at 11:09 AM on June 21, 2016: member

There was a timeout in

  File "qa/pull-tester/rpc-tests.py", line 254, in get_next

    (stdout, stderr) = proc.communicate(timeout=3)

but not due to the travis limit of 50 minutes.

MarcoFalke commented at 11:13 AM on June 21, 2016: member

This should not happen: It can either mean there is a zombie bitcoind running which can not be stopped (a bug we should fix) or the windows tests are running generally really slow. Bumping the timeout to 5 secs or more should fix it then.

laanwj commented at 11:26 AM on June 21, 2016: member

What is the three-second timeout waiting for?

MarcoFalke commented at 11:31 AM on June 21, 2016: member

If it has to wait, there is most likely a bug elsewhere in the framework. Ideally there should be no timeout set, but it has to be set to anything to change the default: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate.

MarcoFalke commented at 11:35 AM on June 21, 2016: member

Unfortunately running the tests in parallel will hide what actually happened until the stderr and stdout are retrieved.

I could come up with a -legacy arg to run the test sequentially with real time stdout like was done previously, for debugging purposes.

MarcoFalke commented at 3:02 PM on July 5, 2016: member

@laanwj I ran this locally to see what is the cause of the intermittent fails, but I couldn't figure it out. It has nothing to do with the timeout=3. (You could set it to less without any issues)

I'd suggest you cherry-pick/pull fa9d398 and we just merge it. Someone can figure out later why parallel test don't work fine with wine/win.

[qa] Don't run win-tests in parallel to avoid fails 71fd4ebbb0

laanwj commented at 11:08 AM on July 11, 2016: member

@MarcoFalke Thanks - cherry-picked your commit, let's see what travis does now.

laanwj commented at 12:50 PM on July 11, 2016: member

Ugh win32 build https://travis-ci.org/bitcoin/bitcoin/jobs/143858179: The job exceeded the maximum time limit for jobs, and has been terminated.

MarcoFalke commented at 9:12 AM on August 8, 2016: member

The job exceeded the maximum time limit for jobs, and has been terminated.

Dropping the java block tester at the same time could improve the run time.

laanwj commented at 1:54 PM on August 13, 2016: member

Good point, that is due to be disabled now: #4545 (comment)

MarcoFalke commented at 8:41 PM on August 17, 2016: member

Hmm, 71fd4eb is still taking about 48 minutes with empty cache, when rebased on current master.

theuni commented at 4:09 AM on August 18, 2016: member

FYI I'm working on a qt 5.7 bump that changes the build significantly. I'm hoping for a reasonable speedup there.

laanwj commented at 11:39 AM on August 26, 2016: member

Closing this for now, can be reopened after the 5.7 bump.

laanwj closed this on Aug 26, 2016

laanwj commented at 4:41 PM on October 20, 2016: member

@theuni how is the qt bump going?

laanwj referenced this in commit b4d85490f0 on Feb 12, 2018

PastaPastaPasta referenced this in commit 26cd80ecd7 on Jun 13, 2020

PastaPastaPasta referenced this in commit a494429137 on Jun 13, 2020

PastaPastaPasta referenced this in commit 0ca57b5c3f on Jun 14, 2020

PastaPastaPasta referenced this in commit 018cd339c4 on Jun 17, 2020

PastaPastaPasta referenced this in commit 91b7040f54 on Jun 17, 2020

PastaPastaPasta referenced this in commit ab1bb64ba5 on Jun 17, 2020

PastaPastaPasta referenced this in commit a8da932ff0 on Jun 17, 2020

MarcoFalke locked this on Sep 8, 2021