-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Exterminate tests #21052
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
Exterminate tests #21052
Conversation
The copy would fail on Windows even with the GNUWin32 tools. Use %empty-directory to clean up and create the structure for the test. NFC.
Ensure that we use the interpreter explicitly when running the touch.py script. This is important for platforms that do not support shebangs for interpreter execution.
These attempt to execute a python script without an interpreter. This happens to work on Linux and Darwin, but will not work on all OSes, and definitely not Windows. Mark the tests as unsupported for the time being. These particular ones actually cause python's multiprocess to fail.
@jrose-apple any idea what to do about the top most commit here? It seems that the wrapper is causing pain :-(. |
CC: @jmittert |
Is there a way to make it use a batch script or something else implicitly executable on Windows? Or does that only work from the command line and not arbitrary subprocess spawns? |
Yeah, unfortunately, that doesn't work from the process creation :-(. That requires an explicit path to the interpreter (even the batch files need to be launched with |
Hm. I'd rather not slow down all these tests by building-and-linking a wrapper just to call Python, but I can't think of anything better. Alternately, we could change |
I was leaning towards the latter, it would be the most flexible and shouldn't slow down things very much. |
@jrose-apple - any opinions on how you want to handle that? perhaps |
Passing down the Python path is probably fine. I don't love the idea of dummy tokens in the frontend path, but I guess spaces would be a problem for spaces-in-path, and this is just for testing anyway. Tabs? Newlines? Semicolons? |
Closing this as I believe that this should no longer be needed thanks to #22638 |
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.