-
Notifications
You must be signed in to change notification settings - Fork 38.7k
tests: Re-enable RPC tests for Windows #8227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
|
Isn't it also failing the |
|
Ok that's one confusing error, yes that one's failing too. That seems specific to this test. |
|
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. |
|
I've pushed a commit that should get past this problem. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
|
Thanks. If this passes, make sure to remove any remaining |
3d2d6f8 to
39d28c6
Compare
|
It passed! Reorganized the commits and removed any mention of |
|
utACK 39d28c6 |
|
Looks like there are timeout issues, run times for the windows tests are on the limit of what travis allows |
|
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... |
|
Re-triggered this after merging #8216, let's see if this is more reliable. |
`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`.
39d28c6 to
dc05f6d
Compare
|
There was a timeout in but not due to the travis limit of 50 minutes. |
|
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. |
|
What is the three-second timeout waiting for? |
|
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. |
|
Unfortunately running the tests in parallel will hide what actually happened until the stderr and stdout are retrieved. I could come up with a |
|
@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 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. |
|
@MarcoFalke Thanks - cherry-picked your commit, let's see what travis does now. |
|
Ugh win32 build https://travis-ci.org/bitcoin/bitcoin/jobs/143858179: 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. |
|
Good point, that is due to be disabled now: #4545 (comment) |
|
Hmm, 71fd4eb is still taking about 48 minutes with empty cache, when rebased on current master. |
|
FYI I'm working on a qt 5.7 bump that changes the build significantly. I'm hoping for a reasonable speedup there. |
|
Closing this for now, can be reopened after the 5.7 bump. |
|
@theuni how is the qt bump going? |
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
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
Try RPC tests in Travis windows builds.
Just a test, don't merge this yet.