tests: Run functional test on Windows and enable it on Appveyor #14007

ken2812221 · 2018-08-20T11:41:26Z

This PR do the following things:

Make functional tests compatible with Windows
Print color output in functional tests for Windows 10
Run util and functional tests on appveyor
Do not run symlink tests on Windows

Note:

The wallet_multiwallet.py fail is unrelated to the test framework, it's a bug related to c++ code or maybe dependencies. bitcoind would exit with 0xC0000005(Access violation) during shutdown occasionally. Disable this for now.
Not using --failfast because this is still in experimental. We should track if there is any other error.
Disable ZMQ tests because the python zmq library could cause access violation sometimes.
Disable feature_notifications because Bitcoin Core handles the command in different thread, whicha can cause a race condition.

maflcko · 2018-08-22T20:22:34Z

I think there is some interference with a firewall or something that kills the bitcoind processes?

maflcko · 2018-08-22T20:23:17Z

appveyor.yml

Could as a first step only run the util tests?

You mean to drop away functional tests and replace it with util tests from this PR, and re-add the funfctional tests in another PR?

ken2812221 · 2018-08-23T04:31:54Z

It is really hard to see blue color on powershell, so change the color to green

DrahtBot · 2018-08-23T04:46:22Z

Note to reviewers: This pull request conflicts with the following ones:

tests: Write the notification message to different files to avoid race condition in feature_notifications.py #14275 (tests: Write the notification message to different files to avoid race condition in feature_notifications.py by ken2812221)
appveyor: script improvement #14241 (appveyor: trivial build cache improvement by ken2812221)
Fix for test runner UnicodeDecodeError when utf-8 is not supported #14237 (Fix for test runner UnicodeDecodeError when utf-8 is not supported by sanket1729)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ken2812221 · 2018-08-23T10:50:07Z

I think there is some interference with a firewall or something that kills the bitcoind processes?

I agree with you, they are all passed on my Windows PC.

maflcko · 2018-08-23T13:37:25Z

They didn't pass for me on Windows 7

ken2812221 · 2018-08-24T01:01:28Z

Oh, it's occasionally. I tested to run it 10 times, 8 times failed, 2 times succeed.

ken2812221 · 2018-08-25T08:26:33Z

This is ready for review, the wallet_multiwallet.py fail is unrelated to the test framework. It is a bug related to c++ code, it would exit with 0xC0000005(Acccess violation) during shutdown occasionally.

maflcko · 2018-08-25T13:52:48Z

test/util/bitcoin-util-test.py

Why is this change needed?

The binary built by MSVC is not at src/, so it needs a variable to indicate the binary location.

maflcko · 2018-08-25T15:00:10Z

appveyor.yml

I think this caching fix can be submitted in an independent pr

ken2812221 · 2018-08-27T14:31:40Z

I think we could disable those problematic tests for now.

ken2812221 · 2018-08-27T16:39:33Z

I think there is some interference with a firewall or something that kills the bitcoind processes?

Solve this problem by using new connection every time we call RPC. Not sure why would that happen.

maflcko · 2018-08-27T17:13:49Z

What happened to the cache fix? I think we are not yet properly caching the intermediate compile results?

ken2812221 · 2018-08-27T19:42:13Z

@MarcoFalke I add the build cache in #14086

ken2812221 · 2018-08-29T10:57:43Z

Update: Based on #14086, following #14092

ken2812221 · 2018-09-05T09:50:22Z

#14086 has been merged, this is ready for review.

maflcko · 2018-09-05T12:58:27Z

test/functional/combine_logs.py

I think this check can be removed. Color is off by default and if someone specifies color, they have a reason to do so (maybe write to a file that is read on a different os/platform)

maflcko · 2018-09-05T13:03:52Z

test/functional/test_framework/authproxy.py

I think this is no longer supported? So could remove the named arg "connection"?

maflcko · 2018-09-05T13:05:04Z

test/functional/test_framework/authproxy.py

Could move this into a method def _set_conn(self): to avoid duplication?

test/functional/test_framework/authproxy.py

maflcko · 2018-09-05T13:06:17Z

test/functional/test_framework/test_node.py

Hmm, any rationale for this? I'd prefer to throw if the log was not unicode.

an error message thrown by boost is always localized(translated) and use ANSI encoding, so it would always throw an error. I don't know any solution to fix this. There is no way to specify the error message format.

For now, I could revert this because appveyor is using English, but it would cause problem if the user is running a non-English Windows. It can be marked as an issue with #13869.

Done in #14192

maflcko · 2018-09-05T13:11:09Z

test/functional/test_runner.py

I'd prefer to keep the throw on non-unicode log files.

Same as above.

ken2812221 · 2018-09-07T14:22:42Z

appveyor.yml

I want to avoid re-uploading cache files if there is no change in #14086, I chose to delete the stats file entirely. But that was a mistake, it would also clear the cache size information which extremely increase cache size.

After then, I realize that clcache has a command to clear hit and miss info but retain size info: clcache -z

See: https://github.com/frerich/clcache/blob/master/README.asciidoc#options

ken2812221 · 2018-09-07T22:52:47Z

@NicolasDorier @sipsorcery Would you mind reviewing this?

sipsorcery · 2018-09-08T19:50:26Z

tACK 1223343

NicolasDorier · 2018-09-10T07:39:20Z

ran several time, do not pass. Though it is not blocking for me, can as well reactivate the test once the fix is done later down the road.

tACK 529c5274de6fae782154c2845b6a691db46b0d4e

ken2812221 · 2018-09-10T07:42:36Z

Thanks for your review 👍

maflcko · 2018-09-11T22:55:30Z

test/util/bitcoin-util-test.py

Hmm, this env var is not documented and makes the code a bit harder to read. Would it be too ugly to call mkdir src && mv ./bitcoin*.exe ./src in the appveyor after compile?

How about now?

maflcko · 2018-09-12T00:25:24Z

Thanks, re-ACK

ken2812221 · 2018-09-12T20:31:03Z

@jnewbery Would you mind reviewing the test changes here. Most of code that changed in this PR are previously written by you.

jnewbery · 2018-09-12T21:08:41Z

Sure, I can review the tests: Make it possible to run functional tests on Windows commit, but it probably won't be until next week.

laanwj · 2018-09-13T07:19:34Z

test/functional/test_runner.py

if you have to use ctypes directly here, with magic numbers, can you please at least add a comment about what the numbers here mean, what it is doing

test/functional/test_runner.py

laanwj · 2018-09-13T07:28:30Z

test/functional/test_framework/authproxy.py

I guess this is meant as a temporary work-around until we find out why? (might want to add a comment in that regard) being able to re-use RPC connections is a big deal, it's not something we should simply accept as-is.

laanwj · 2018-09-13T08:19:34Z

utACK d5678479266bb087b37ee503000ab161d9e44664

ken2812221 · 2018-09-16T23:42:36Z

Update: Disabled 1 additional tests for now because of #14225 merged.

feature_notifications
Bitcoin Core handle notifications in different thread, which can cause a race condition.
mining_getblocktemplate_longpoll
I'm not entirely sure what's going on here. Seems it doesn't take timeout=600 value.
Fixed

ken2812221 · 2018-09-17T00:43:05Z

@MarcoFalke connection is still used in mining_getblocktemplate_longpoll.py. Please re-review.

maflcko · 2018-09-17T19:30:21Z

utACK 661ac15

…Appveyor 661ac15 appveyor: Run functional tests on appveyor (Chun Kuan Lee) 2148c36 tests: Make it possible to run functional tests on Windows (Chun Kuan Lee) Pull request description: This PR do the following things: - Make functional tests compatible with Windows - Print color output in functional tests for Windows 10 - Run util and functional tests on appveyor - Do not run symlink tests on Windows Note: - The wallet_multiwallet.py fail is unrelated to the test framework, it's a bug related to c++ code or maybe dependencies. `bitcoind` would exit with 0xC0000005(Access violation) during shutdown occasionally. Disable this for now. - Not using `--failfast` because this is still in experimental. We should track if there is any other error. - Disable ZMQ tests because the python zmq library could cause access violation sometimes. - Disable `feature_notifications` because Bitcoin Core handles the command in different thread, whicha can cause a race condition. Tree-SHA512: b76db137d264e62a5c130e1cbca7a2ca002a7a0f4153fa0b92c1ea6c9c09ef0533e11c49bdbd566c472d8ff59f245758feb5e5a6ec6cb6bb66a1c67bab5fa48a

jnewbery

tested ACK. I think there's one change that needs to be reverted. @ken2812221 - can you take a look for me?

jnewbery · 2018-09-24T21:48:04Z

test/functional/test_runner.py

    # Remove the test cases that the user has explicitly asked to exclude.
    if args.exclude:
-        exclude_tests = [re.sub("\.py$", "", test) + ".py" for test in args.exclude.split(',')]
+        exclude_tests = [re.sub("\.py$", "", test) + (".py" if ".py" not in test else "") for test in args.exclude.split(',')]


I don't understand this change. The point of:

[re.sub("\.py$", "", test) + ".py"

is that if <test_name>.py or <test_name> is given, then it's normalized to <test_name>.py.

Your new version:

re.sub("\.py$", "", test) + (".py" if ".py" not in test else "")

essentially toggles, so <test_name>.py is changed to <test_name> and <test_name> is changed to <test_name>.py.

>>> test1 = 'test_name1.py' >>> test2 = 'test_name2' >>> for test in [test1,test2]: ... print(re.sub("\.py$", "", test) + ".py") # original version ... test_name1.py test_name2.py >>> for test in [test1,test2]: ... print(re.sub("\.py$", "", test) + (".py" if ".py" not in test else "")) # new version ... test_name1 test_name2.py

This breaks using -exclude=<test_name>.py

The goal was to be able to specify --exclude "feature_notifications,wallet_multiwallet,wallet_multiwallet.py --usecli".

I think removing everything from the test_list that .startswith(item.split('.py')[0]) for any item in args.exclude.split(',') should solve all issues?

Oh, I didn't test that case. Should be fixed in #14316

…ers in `--exclude` list c7b3e48 tests: exclude all tests with difference parameters (Chun Kuan Lee) Pull request description: Fix broken exclusion list in functional tests. See bitcoin#14007 (review) Tree-SHA512: b6c2b86fef13e3c00c695adaeeb3e47ee9b48877c71bc605d24201ce931b2ef3ae9f5f199071fa1ec5de2d7aadc478410094c380cc297922e683e9b2569cda03

Summary: Backport of core [[bitcoin/bitcoin#14007 | PR14007]]. Depends on D5958. Test Plan: ninja check-functional The output should be all green. Not blue. Build for windows, then: - Install python if needed and add it to your path - If you copy the project to some place, you need at least the `src`, `test`, `share` and your build directories. - Copy the test_runner.py to the build directory to replace the symlink - Update the config.ini file to match you paths Then in a powershell: $env:PYTHONIOENCODING="utf-8" python .\test\functional\test_runner.py --force Most of the tests should pass, but still far from all. More backports coming. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5964

fanquake added the Tests label Aug 20, 2018

maflcko reviewed Aug 22, 2018

View reviewed changes

DrahtBot mentioned this pull request Aug 23, 2018

build: make 'depends' output less spammy #13654

Closed

1 task

ken2812221 changed the title ~~[WIP] appveyor: Run functional test on appveyor~~ appveyor: Run functional test on appveyor Aug 25, 2018

ken2812221 mentioned this pull request Aug 25, 2018

~~Correctly~~ terminate HTTP server #13501

Closed

maflcko reviewed Aug 25, 2018

View reviewed changes

DrahtBot mentioned this pull request Aug 25, 2018

build: generate MSVC project files via python script #14062

Merged

DrahtBot added Needs rebase and removed Needs rebase labels Aug 26, 2018

ken2812221 changed the title ~~appveyor: Run functional test on appveyor~~ tests: Run functional test on Windows Aug 27, 2018

DrahtBot mentioned this pull request Aug 27, 2018

appveyor: Use clcache to speed up build #14086

Merged

ken2812221 changed the title ~~tests: Run functional test on Windows~~ tests: Run functional test on Windows and enable it on Appveyor Sep 5, 2018

maflcko reviewed Sep 5, 2018

View reviewed changes

ken2812221 commented Sep 7, 2018

View reviewed changes

maflcko reviewed Sep 11, 2018

View reviewed changes

laanwj reviewed Sep 13, 2018

View reviewed changes

test/functional/test_runner.py Outdated Show resolved Hide resolved

laanwj reviewed Sep 13, 2018

View reviewed changes

tests: Make it possible to run functional tests on Windows

2148c36

ken2812221 mentioned this pull request Sep 17, 2018

Utils and libraries: Fix #13371 - move umask operation earlier in AppInit() #13389

Closed

DrahtBot mentioned this pull request Sep 17, 2018

Fix for test runner UnicodeDecodeError when utf-8 is not supported #14237

Closed

appveyor: Run functional tests on appveyor

661ac15

DrahtBot mentioned this pull request Sep 17, 2018

appveyor: script improvement #14241

Merged

This was referenced Sep 19, 2018

scripted-diff: force set python source file encoding to utf-8 #14265

Closed

tests: Write the notification message to different files to avoid race condition in feature_notifications.py #14275

Merged

maflcko merged commit 661ac15 into bitcoin:master Sep 24, 2018

ken2812221 deleted the patch-2 branch September 24, 2018 20:46

jnewbery reviewed Sep 24, 2018

View reviewed changes

ken2812221 mentioned this pull request Sep 25, 2018

tests: exclude all tests with difference parameters in --exclude list #14316

Merged

ken2812221 mentioned this pull request Oct 9, 2018

tests: Some issue about running functional tests on Windows #14446

Closed

5 tasks

Bushstar mentioned this pull request Oct 11, 2018

Updates from bitcoin/master FeatherCoin/Feathercoin#366

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

tests: Run functional test on Windows and enable it on Appveyor #14007

tests: Run functional test on Windows and enable it on Appveyor #14007

Uh oh!

Conversation

ken2812221 commented Aug 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Aug 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 commented Aug 23, 2018

Uh oh!

DrahtBot commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Aug 23, 2018

Uh oh!

maflcko commented Aug 23, 2018

Uh oh!

ken2812221 commented Aug 24, 2018

Uh oh!

ken2812221 commented Aug 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 commented Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Aug 27, 2018

Uh oh!

maflcko commented Aug 27, 2018

Uh oh!

ken2812221 commented Aug 27, 2018

Uh oh!

ken2812221 commented Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Sep 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 commented Sep 7, 2018

Uh oh!

sipsorcery commented Sep 8, 2018

Uh oh!

NicolasDorier commented Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ken2812221 commented Aug 20, 2018 •

edited

Loading

DrahtBot commented Aug 23, 2018 •

edited

Loading

ken2812221 commented Aug 25, 2018 •

edited

Loading

ken2812221 commented Aug 27, 2018 •

edited

Loading

ken2812221 commented Aug 29, 2018 •

edited

Loading

maflcko Sep 5, 2018 •

edited

Loading

ken2812221 Sep 5, 2018 •

edited

Loading

ken2812221 Sep 7, 2018 •

edited

Loading

NicolasDorier commented Sep 10, 2018 •

edited

Loading

laanwj Sep 13, 2018 •

edited

Loading

ken2812221 commented Sep 16, 2018 •

edited

Loading

ken2812221 commented Sep 17, 2018 •

edited

Loading