Skip to content

ci: Fix and enable tests on Windows #22922

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

Merged
merged 2 commits into from
Sep 9, 2021
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 8, 2021

Only a cherry-picked list. --extended can be enabled in a follow-up.

@maflcko
Copy link
Member Author

maflcko commented Sep 8, 2021

In the tests that use the P2P interface, I am running into event loop (garbage collection) issues, but this hack works around them:

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index f382e0fdb3..0b7ef1c803 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -152,7 +152,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             self.success = TestStatus.FAILED
         finally:
             exit_code = self.shutdown()
-            sys.exit(exit_code)
+            os._exit(exit_code)
 
     def parse_args(self):
         previous_releases_path = os.getenv("PREVIOUS_RELEASES_DIR") or os.getcwd() + "/releases"

@hebasto
Copy link
Member

hebasto commented Sep 8, 2021

Concept ACK.

In the tests that use the P2P interface, I am running into event loop (garbage collection) issues...

I also saw such issues while working on the same problem.

... but this hack works around them:

That is great! Any link to an explanation or so (just curios)?

@hebasto
Copy link
Member

hebasto commented Sep 8, 2021

https://api.cirrus-ci.com/v1/task/5800162858631168/logs/functional_tests.log:


C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call python test\functional\test_runner.py --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 rpc_help,feature_config_args,rpc_signer,tool_wallet,feature_presegwit_node_upgrade --failfast 
WARNING! Test 'rpc_help,feature_config_args,rpc_signer,tool_wallet,feature_presegwit_node_upgrade' not found in full test list.
No valid test scripts specified. Check that your test is in one of the test lists in test_runner.py, or run test_runner.py with no arguments to run all tests

C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 

@maflcko
Copy link
Member Author

maflcko commented Sep 8, 2021

That is great! Any link to an explanation or so (just curios)?

I have no idea, but os._exit might skip the garbage collector.

@DrahtBot DrahtBot added the Tests label Sep 8, 2021
@hebasto
Copy link
Member

hebasto commented Sep 8, 2021

"Win64 native" is GREEN 🐅

@hebasto
Copy link
Member

hebasto commented Sep 8, 2021

Almost ACK fa0c194

Locally I've got an error:

Arguments: ()
--- Logging error ---
Traceback (most recent call last):
  File "C:\Python39\lib\logging\__init__.py", line 1082, in emit
    stream.write(msg + self.terminator)
  File "C:\Python39\lib\encodings\cp1252.py", line 19, in encode
    return codecs.charmap_encode(input,self.errors,encoding_table)[0]
UnicodeEncodeError: 'charmap' codec can't encode character '\u20bf' in position 110: character maps to <undefined>
Call stack:
  File "C:\Users\hebasto\bitcoin\test\functional\feature_presegwit_node_upgrade.py", line 57, in <module>
    SegwitUpgradeTest().main()
  File "C:\Users\hebasto\bitcoin\test\functional\test_framework\test_framework.py", line 154, in main
    exit_code = self.shutdown()
  File "C:\Users\hebasto\bitcoin\test\functional\test_framework\test_framework.py", line 314, in shutdown
    self.log.info("Cleaning up {} on exit".format(self.options.tmpdir))
Message: 'Cleaning up C:\\Users\\hebasto\\AppData\\Local\\Temp\\test_runner_\u20bf_\U0001f3c3_20210908_221814\\feature_presegwit_node_upgrade_0 on exit'
Arguments: ()

I think, the python setup on my Windows machine is broken somehow.


UPDATE: Sorry for my mistake. Just forgot to set PYTHONUTF8=1.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK fa0c194, tested locally on Windows 10 Pro 20H2 (build 19042.1165):

>python test\functional\test_runner.py --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 rpc_help feature_config_args rpc_signer feature_presegwit_node_upgrade "tool_wallet.py --descriptors" --failfast
Running Unit Tests for Test Framework Modules
..........
----------------------------------------------------------------------
Ran 10 tests in 0.769s

OK
Remaining jobs: [rpc_help.py, feature_config_args.py, feature_presegwit_node_upgrade.py, tool_wallet.py --descriptors]
Remaining jobs: [rpc_help.py, feature_config_args.py, tool_wallet.py --descriptors]
Remaining jobs: [feature_config_args.py, tool_wallet.py --descriptors]
Remaining jobs: [tool_wallet.py --descriptors]

TEST                              | STATUS    | DURATION

feature_config_args.py            | ✓ Passed  | 112 s
feature_presegwit_node_upgrade.py | ✓ Passed  | 27 s
rpc_help.py                       | ✓ Passed  | 33 s
rpc_signer.py                     | ✓ Passed  | 23 s
tool_wallet.py --descriptors      | ✓ Passed  | 104 s

ALL                               | ✓ Passed  | 299 s (accumulated)
Runtime: 127 s

@maflcko maflcko merged commit e4aa9b1 into bitcoin:master Sep 9, 2021
@maflcko maflcko deleted the 2109-testWin branch September 9, 2021 06:25
fanquake added a commit to bitcoin-core/gui that referenced this pull request Sep 10, 2021
…ts on Windows

c427a58 doc: Set PYTHONUTF8=1 for functional tests on Windows (Hennadii Stepanov)

Pull request description:

  The `PYTHONUTF8` environment variable is defined in [PEP 540](https://www.python.org/dev/peps/pep-0540/), and it is actually used in our CI: https://github.com/bitcoin/bitcoin/blob/5e3380b9f59481fc18e05b9d651c3c733abe4053/.cirrus.yml#L89

  This PR documents such usage to avoid users' [errors](bitcoin/bitcoin#22922 (comment)).

ACKs for top commit:
  MarcoFalke:
    cr ACK c427a58

Tree-SHA512: 441b8cecfe47d548cfe403b0e1cd0aef25c1a70ff556434ead1f1e26372919931ac6f208a4ed6fd8dcca46e8709245e4fb06f95259a43c8e1221473ce1ee497b
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 11, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 11, 2021
…ndows

c427a58 doc: Set PYTHONUTF8=1 for functional tests on Windows (Hennadii Stepanov)

Pull request description:

  The `PYTHONUTF8` environment variable is defined in [PEP 540](https://www.python.org/dev/peps/pep-0540/), and it is actually used in our CI: https://github.com/bitcoin/bitcoin/blob/5e3380b9f59481fc18e05b9d651c3c733abe4053/.cirrus.yml#L89

  This PR documents such usage to avoid users' [errors](bitcoin#22922 (comment)).

ACKs for top commit:
  MarcoFalke:
    cr ACK c427a58

Tree-SHA512: 441b8cecfe47d548cfe403b0e1cd0aef25c1a70ff556434ead1f1e26372919931ac6f208a4ed6fd8dcca46e8709245e4fb06f95259a43c8e1221473ce1ee497b
@hebasto
Copy link
Member

hebasto commented Sep 15, 2021

@MarcoFalke

In the tests that use the P2P interface, I am running into event loop (garbage collection) issues...

Fixed in #22987.

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 13, 2022
…ndows

c427a58 doc: Set PYTHONUTF8=1 for functional tests on Windows (Hennadii Stepanov)

Pull request description:

  The `PYTHONUTF8` environment variable is defined in [PEP 540](https://www.python.org/dev/peps/pep-0540/), and it is actually used in our CI: https://github.com/bitcoin/bitcoin/blob/5e3380b9f59481fc18e05b9d651c3c733abe4053/.cirrus.yml#L89

  This PR documents such usage to avoid users' [errors](bitcoin#22922 (comment)).

ACKs for top commit:
  MarcoFalke:
    cr ACK c427a58

Tree-SHA512: 441b8cecfe47d548cfe403b0e1cd0aef25c1a70ff556434ead1f1e26372919931ac6f208a4ed6fd8dcca46e8709245e4fb06f95259a43c8e1221473ce1ee497b
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
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