Skip to content

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

Closed
wants to merge 3 commits into from
Closed

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Dec 5, 2018

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

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.
@compnerd
Copy link
Member Author

compnerd commented Dec 5, 2018

@jrose-apple any idea what to do about the top most commit here? It seems that the wrapper is causing pain :-(.

@compnerd
Copy link
Member Author

compnerd commented Dec 5, 2018

CC: @jmittert

@jrose-apple
Copy link
Contributor

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?

@compnerd
Copy link
Member Author

compnerd commented Dec 5, 2018

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 cmd.exe /c).

@jrose-apple
Copy link
Contributor

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 -driver-use-frontend-path to take multiple arguments (if we're very careful about escaping) or add some other functionality for testing this.

@compnerd
Copy link
Member Author

compnerd commented Dec 6, 2018

I was leaning towards the latter, it would be the most flexible and shouldn't slow down things very much.

@compnerd
Copy link
Member Author

compnerd commented Dec 6, 2018

@jrose-apple - any opinions on how you want to handle that? perhaps -driver-use-frontend-path C:\Python27\python.exe*wrapper.py ? Or do want to search for the interpreter in PATH? Doing so means that we can't guarantee that the interpreter used is the same as the lit process.

@jrose-apple
Copy link
Contributor

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?

@compnerd
Copy link
Member Author

Closing this as I believe that this should no longer be needed thanks to #22638

@compnerd compnerd closed this Feb 23, 2019
@compnerd compnerd deleted the exterminate-tests branch February 23, 2019 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants