Skip to content
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

Fix tests on Windows, add Windows CI #1186

Merged
merged 2 commits into from Feb 13, 2023
Merged

Fix tests on Windows, add Windows CI #1186

merged 2 commits into from Feb 13, 2023

Conversation

mhils
Copy link
Member

@mhils mhils commented Feb 13, 2023

No description provided.

@@ -2524,7 +2524,7 @@ def check_recovery(
b"-passin",
b"pass:" + passwd,
*extra,
)
).replace(b"\r\n", b"\n")
Copy link
Member Author

Choose a reason for hiding this comment

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

_runopenssl is also used with binary output, so we can't replace this in there for all invocations.

@@ -2838,23 +2845,24 @@ def test_wantWriteError(self):
"""
client_socket, server_socket = socket_pair()
# Fill up the client's send buffer so Connection won't be able to write
# anything. Only write a single byte at a time so we can be sure we
# anything. Start by sending larger chunks (Windows Socket I/O is slow)
# and continue by writing a single byte at a time so we can be sure we
Copy link
Member Author

Choose a reason for hiding this comment

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

This brings this test down from 16s to <20ms on my machine.

Copy link
Member

Choose a reason for hiding this comment

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

Socket I/O performance joins VFS performance on my Windows is bad list 😄

@mhils
Copy link
Member Author

mhils commented Feb 13, 2023

The Windows CI job fails to upload coverage right now. Do we care about this?

I do maintain https://github.com/mhils/better-codecov-action, but I don't want to impose my homegrown actions on anyone else. 😛

@reaperhulk
Copy link
Member

Haha, well you're doing the work. We could also adopt the coverage system cryptography uses (which eliminates codecov entirely), although it'd probably need to be updated to report back via a status job rather than just failing the build when it's under 100% since we don't have 100% here :)

Either way let's take it as a follow up PR after this if you want to tackle it.

@reaperhulk reaperhulk merged commit cac7478 into pyca:main Feb 13, 2023
@mhils mhils deleted the windows branch February 13, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants