-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add Windows CI #10681
Add Windows CI #10681
Conversation
This reverts commit 74b5bbf.
This pull request introduces 1 alert when merging 7e823e0 into 60c1867 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 317e632 into 60c1867 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 2fb0996 into 60c1867 - view on LGTM.com new alerts:
|
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.
I approve
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.
Looks good, two nits to maybe address, or not :)
This pull request introduces 1 alert when merging fe6825a into 9f50bb3 - view on LGTM.com new alerts:
|
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.
The LGTM alert indeed was introduced here because you removed the usage of Path
in tests/pools/test_pool_config.py
and it seems like i made the Windows CI unhappy again with some path related issues via #11625, sorry :) Seems like its related to the path string matching in with pytest.raises(ValueError, match=f"Path doesn't exist: {test_path}")
?
This pull request introduces 1 alert when merging adcbd02 into 5844c6c - view on LGTM.com new alerts:
|
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.
aok
Draft for:
py
call inInstall.ps1
.--numprocesses 0
in case xdist is usable on Windows.Items to note.
The build workflow script currently sets
-n 0
for all windows tests and ignores the configParallel
setting. Windows tests do work locally quite well with multiple subprocesses, however, the GH runners stumble with 4 processes in some tests. Subsequent PRs can explore expanding the config to have a dict with per OS settings.The IPv6 test is skipped (also skipped on macOS).