Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Jun 20, 2016

Try RPC tests in Travis windows builds.

Just a test, don't merge this yet.

@laanwj laanwj added the Tests label Jun 20, 2016
@laanwj
Copy link
Member Author

laanwj commented Jun 20, 2016

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)

@maflcko
Copy link
Member

maflcko commented Jun 20, 2016

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

@laanwj
Copy link
Member Author

laanwj commented Jun 20, 2016

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
Copy link
Member Author

laanwj commented Jun 20, 2016

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
Copy link
Member Author

laanwj commented Jun 20, 2016

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to fix the nit in this pull.

@maflcko
Copy link
Member

maflcko commented Jun 20, 2016

Thanks.

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

@laanwj laanwj force-pushed the 2016_05_win_reenable_rpc_tests branch from 3d2d6f8 to 39d28c6 Compare June 20, 2016 11:20
@laanwj laanwj changed the title tests: Try re-enabling RPC tests for Windows tests: Re-enable RPC tests for Windows Jun 20, 2016
@laanwj
Copy link
Member Author

laanwj commented Jun 20, 2016

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.

@maflcko
Copy link
Member

maflcko commented Jun 20, 2016

utACK 39d28c6

@laanwj
Copy link
Member Author

laanwj commented Jun 20, 2016

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

@maflcko
Copy link
Member

maflcko commented Jun 20, 2016

I've added a commit to fa58f94 which should improve that, but I think after enabling qt builds again, we are again at the limit...

@laanwj
Copy link
Member Author

laanwj commented Jun 21, 2016

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

laanwj added 2 commits June 21, 2016 11:30
`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.
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`.
@laanwj laanwj force-pushed the 2016_05_win_reenable_rpc_tests branch from 39d28c6 to dc05f6d Compare June 21, 2016 09:30
@maflcko
Copy link
Member

maflcko commented Jun 21, 2016

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.

@maflcko
Copy link
Member

maflcko commented Jun 21, 2016

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
Copy link
Member Author

laanwj commented Jun 21, 2016

What is the three-second timeout waiting for?

@maflcko
Copy link
Member

maflcko commented Jun 21, 2016

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.

@maflcko
Copy link
Member

maflcko commented Jun 21, 2016

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.

@maflcko
Copy link
Member

maflcko commented Jul 5, 2016

@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.

@laanwj
Copy link
Member Author

laanwj commented Jul 11, 2016

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

@laanwj
Copy link
Member Author

laanwj commented Jul 11, 2016

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

@maflcko
Copy link
Member

maflcko commented Aug 8, 2016

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
Copy link
Member Author

laanwj commented Aug 13, 2016

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

@maflcko
Copy link
Member

maflcko commented Aug 17, 2016

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

@theuni
Copy link
Member

theuni commented Aug 18, 2016

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

@laanwj
Copy link
Member Author

laanwj commented Aug 26, 2016

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

@laanwj laanwj closed this Aug 26, 2016
@laanwj
Copy link
Member Author

laanwj commented Oct 20, 2016

@theuni how is the qt bump going?

laanwj added a commit that referenced this pull request Feb 12, 2018
faefd29 qa: Prepare functional tests for Windows (MarcoFalke)

Pull request description:

  * Pass `sys.executable` when calling a python script via the subprocess
    module
  * Don't remove the log file while it is still open and written to
  * Properly use os.pathsep and os.path.sep when modifying the PATH
    environment variable
  * util-tests: Use os.path.join for Windows compatibility

  Ref:  #8227

Tree-SHA512: c507a536af104b3bde4366b6634099db826532bd3e7c35d694b5883c550920643b3eab79c76703ca67e1044ed09979e855088f7324321c8d52112514e334d614
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 17, 2020
faefd29 qa: Prepare functional tests for Windows (MarcoFalke)

Pull request description:

  * Pass `sys.executable` when calling a python script via the subprocess
    module
  * Don't remove the log file while it is still open and written to
  * Properly use os.pathsep and os.path.sep when modifying the PATH
    environment variable
  * util-tests: Use os.path.join for Windows compatibility

  Ref:  bitcoin#8227

Tree-SHA512: c507a536af104b3bde4366b6634099db826532bd3e7c35d694b5883c550920643b3eab79c76703ca67e1044ed09979e855088f7324321c8d52112514e334d614
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants