qa: Fix “RuntimeError: Event loop is closed” on Windows #22987

1. 
   hebasto commented at 6:12 pm on September 15, 2021: member

   On master (2161a058552ac938f2079b311a2d12f5d1772d01), running functional tests that use the P2P interface ends with an error:

   0RuntimeError: Event loop is closed
   

   This PR fixes this bug, and enables more functional tests on Windows MSVC CI task.

   More details about bugfix:

   • What’s New In Python 3.7
   • https://bugs.python.org/issue33792
   • actual change done in Python 3.8

   Excluded tests, that are listed in the EXCLUDE_TESTS environment variable, need more thorough investigation to be enabled.

2. qa: Fix "RuntimeError: Event loop is closed" on Windows f55932678f
3. ci: Enable more functional tests on Windows MSVC task 357f0c7233
4. DrahtBot added the label Tests on Sep 15, 2021
5. hebasto added the label Windows on Sep 15, 2021
6. 
   fanquake commented at 1:15 am on September 16, 2021: member

   In master we still require a minimum Python version of 3.6. From what I can see, WindowsSelectorEventLoopPolicy didn’t exist until Python 3.8.
7. 
   hebasto commented at 9:04 am on September 16, 2021: member

   In master we still require a minimum Python version of 3.6. From what I can see, WindowsSelectorEventLoopPolicy didn’t exist until Python 3.8.

   Right. Would it be sufficient to update our docs and specifically for Windows users mention that they are required to install Python 3.8+ ?

   Or add a version check?

8. 
   MarcoFalke commented at 10:07 am on September 16, 2021: member

   This should be fine. Python doesn’t care about unexecuted code.

   I am running python3.6 on my machine with the following diff and everything passed:

    0diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
    1index ec563cc290..1cf23ef191 100755
    2--- a/test/functional/test_framework/p2p.py
    3+++ b/test/functional/test_framework/p2p.py
    4@@ -578,7 +578,7 @@ class NetworkThread(threading.Thread):
    5         NetworkThread.listeners = {}
    6         NetworkThread.protos = {}
    7         if sys.platform == 'win32':
    8-            asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
    9+            asyncio.foo_bar(asyncio.FooBar())
   10         NetworkThread.network_event_loop = asyncio.new_event_loop()
   11 
   12     def run(self):
   

   Again, I don’t think anyone of our users is running the functional tests on windows, so it seems odd to even think about spending time to support running them on older versions of python.

9. 
   MarcoFalke commented at 10:08 am on September 16, 2021: member

   review ACK 357f0c723308b296c6250946c26cf476d9ecfda2 🌆

   Signature:

    0-----BEGIN PGP SIGNED MESSAGE-----
    1Hash: SHA512
    2
    3review ACK 357f0c723308b296c6250946c26cf476d9ecfda2 🌆
    4-----BEGIN PGP SIGNATURE-----
    5
    6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
    7pUgHSAwAjiVz1b6sTwNnO3B+lE+GAE8Uza3wmZE0U6wzpHAyiFlN3IbN9nDIMXHF
    8XhI5v36mNT5OgrBtR7KlRbbEuhPLpz145KZkuWvbGfqo7IqIUOOrttgmmAqSnMyb
    9O/ksg37qt7pE9bQwwdqNUNVDpy6el0ijAOVX+M4GW9JStz8LxgJMG9qPxv/1ZQzW
   10u2x5GCpUrxvJ7DjufwK/6+Z0WN2I0G8/c0M3+56xHrB5Y3yCm3K1Dm4olZty2GBh
   11a0vpeVrKywKyDmLS1SQp4NdNPnAes2LYI4zijleYFaQYxGTgi/tQ2Dz8WCKrqIMK
   124VGbyNTIy20EsQCHNkpuv168dyETSTyJ8wFUQs11x1kRcCeM5inWbrWwNXuOK/sO
   13BvQcQ0qHKM+86iW/xL1+VgWS/SNQ4DrUBo4BO0NecosxuNYDCFYi1eWKwOUVSNDV
   14UPhN0yB/jP/b/xIxXXPVOgw5Agn5psrVqLB+JYE+mWHDTF5aoUswFdwBrQilzksT
   15EnyWZnM4
   16=U9D7
   17-----END PGP SIGNATURE-----
   

   Timestamp of file with hash 81344abdd7d21141fc132bf636bb4c3a9b6e24b5ef0ed9a189290709b209a0d8 -

10. 
    MarcoFalke commented at 12:08 pm on September 17, 2021: member

    I reset the ci run 4 times, all times it did pass.

    Would be good to merge this before the std::fs refactor, so that it has better test coverage.

11. 
    fanquake commented at 6:32 am on September 18, 2021: member

    This should be fine. Python doesn’t care about unexecuted code.

    Sure. However I do care about implicit / undocumented dependency changes. At a minimum the commit / PR description should mention the change in requirements.

12. 
    MarcoFalke commented at 6:33 am on September 18, 2021: member

    The PR description links to “what is new in Python 3.7”. Is there anything else it can say?
13. 
    fanquake commented at 6:35 am on September 18, 2021: member

    Is there anything else it can say?

    Link to / mention the actual minimum requirement, which is 3.8?

14. 
    hebasto commented at 6:45 am on September 18, 2021: member

    Is there anything else it can say?

    Link to / mention the actual minimum requirement, which is 3.8?

    Added.

15. fanquake referenced this in commit de2af19dc8 on Sep 18, 2021
16. 
    fanquake commented at 9:02 am on September 18, 2021: member

    This has been merged.
17. fanquake closed this on Sep 18, 2021

    ---

18. sidhujag referenced this in commit b512dc254c on Sep 19, 2021
19. hebasto deleted the branch on Sep 24, 2021
20. DrahtBot locked this on Oct 30, 2022

Contributors
hebasto fanquake MarcoFalke

Labels
Windows Tests