-
Notifications
You must be signed in to change notification settings - Fork 353
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
RemoteExecutor support on platforms which cannot spawn a process #5129
Comments
/cc @stephentoub @danmosemsft |
To be clear you don't want it to run them in proc you want them to skip. We would probably want to scan the tests and see whether we would lose critical coverage. Some of them could run fine inproc if test parallelism was disabled. |
Dan's right that some of them would be ok to run in-process if parallelism was disabled. But many rely on either mutating process-wide state in a way that will cause subsequent tests to fail, rely on no other relevant tests having already run and mutated process-wide state, are explicitly about testing cross-process communication and will likely deadlock if invoked synchronously, rely on the process to have been configured in a certain way in order to pass (e.g. environment variables set on the process), test things like fail fast that will actually crash the process on purpose, etc. We would need to audit them all individually to opt them in to such a mode, at which point we might as well just mark the tests for the relevant tests accordingly. If the request is to nop the calls, I expect a couple would still fail, where the test calling RemoteExecutor is expecting it to throw, and so we'd need to opt-out those tests, but that's a much smaller number and should be addressible by fixing the tests that fail. If we did this, I would want us to add a test somewhere that validates that RemoteExecutor is only nop'ing for specific platforms, as otherwise we could silently be losing important coverage on the platforms where it is relevant. Even if we add this, I agree with Dan that we'll need to think about the coverage we're losing from doing this, and figure out how to address the test gaps. Just disabling ~400 tests without remedy makes me sad. cc: @ViktorHofer |
My current idea was to throw |
For test runs in UWP where (at the time) one could not spawn a process, we had a variant of RemoteExecutor that used a server process that the system provided (I think). Perhaps these platforms have something like that? |
That wording is not accurate. In general, we are looking for a solution to handle these tests on platforms which support only single process. (I'll update the issue title) |
BTW, here is what we did for UAP/UWP. Does iOS have a similar concept? |
I also recall that it was slow, perhaps because the mechanism wasn't expected to get the kind of traffic our tests applied to it. |
In theory, we can make two identical apps and the main one will activate the other (via URL Schemes) and pass test names (or make a generic remote executor and execute dynamic tests via interpreter) but it sounds quite complicated and we'd like to at least run the non-remoteexecutor based tests for now. Currently 59 test assemblies have |
I am also guessing it would have the same problem I mentioned, that launching apps is not expected to be fast and lightweight. |
@EgorBo do we know how many of the tests are testing APIs that are expected to work on iOS like setting CurrentCulture? We'd need to manually run these in a standalone app for now. |
I don't believe there should be any -- we used to use RemoteExecutor for these as UWP only supported process-wide culture; but that was removed in 9b88ca1 and it should now use the IDisposable thing ThreadCultureChange. |
This looks a bit scary @danmosemsft is the agreement what needs to be done on this? |
This is on us to take a first look as part of getting tests passing we will disable these and see what coverage is being lost. If we need an OS specific mechanism we will likely need help. I added to this project https://github.com/dotnet/runtime/projects/48 |
How will we know the test failure is caused by a test using RemoteExecutor ? Will it crash, hang or print some warning? |
I would expect it to fail with a callstack that has Process.Start on top (assuming that method call throws). It would make sense for now to modify RemoteExecutor.cs to, when in Android/iOS, throw new SkipTestException("Cannot spawn processes"). And possibly open an issue with the list. |
Of course we would be happy to do this when we are the point we can run iOS tests, if you have not done it already. I think it would be an inevitable part of our work. |
You would still need to mark all those tests as |
Currently the RemoteExecutor is disabled for single file CoreCLR and NativeAOT. While these form factors of .NET don't support starting arbitrary managed exes, they do support spawning processes. With a little tweaking the remote executor can work for these form factors: #11460 |
RemoteExecutor should have a special "no-op" mode to be able to run hundreds of the existing tests on platforms where you can't spawn processes (e.g. iOS, tvOS, etc). It looks like there are more than 400 RemoteExecutor-based tests in dotnet/runtime and disabling them all for iOS one by one can be painful. We had it in mono/mono: https://github.com/mono/mono/blob/master/mcs/class/test-helpers/RemoteExecutorTestBase.Mobile.cs#L19
The text was updated successfully, but these errors were encountered: