Testhost sharing between discovery & execution

[cvpoienaru]

Testhost sharing between discovery & execution.

Known issues:

• Parallel discovery doesn't work well for .NET Framework scenarios yet, but this should be fixed by the Parallel Discovery work.

Follow ups:

• Add a LRU cache to limit the number of testhosts we end up spinning;
• Enable M:N mapping between testhosts and test sessions;
• Add better discovery/run settings matching against the settings used for test session creation (currently we use string.Equals for matching);
• Change the proxy enqueueing/dequeueing mechanism to make the usage of ids transparent to the caller;
• Remove obsolete attributes from the public API once it's considered to be fully stable;

[nohwnd]

Made it a draft because you said it is still work in progress. 🙂

[cvpoienaru]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[nohwnd]

I am missing those new tests for this functionality that you mentioned. There are few edits in tests files, but I don't see any new tests.

[cvpoienaru]

@nohwnd, the new tests are not committed yet, I have them on my local machine. Will do the commit once I have full test coverage, but it may take a while since writing comprehensive tests is a tedious endeavor.

[Sanan07]

Partially reviewed, cause the change is big.
Will do rest of later :)

[nohwnd]

@nohwnd, the new tests are not committed yet, I have them on my local machine. Will do the commit once I have full test coverage, but it may take a while since writing comprehensive tests is a tedious endeavor.

Oh okay, I thought you just marked it as ready to review. But it was few days ago. Ping me when ready pls.

[cvpoienaru]

@nohwnd now the tests are finally ready ... It's been a while but better late than never 😄

[nohwnd]

To solve those conflicts, you are probably just best of just grabbing all from master, and letting Roslyn analyzers to show you the compilation errors, I see you have few apis added to unshipped, and 1 changed in shipped (is that intentional?) Unless there are differences per TFM, the change should go into the files that are directly in the PublicAPI folder, because that file is common api for all tfms. If there is difference between netstandard1.0 and others, it should go to the file in net folder. Otherwise in the tfm specific folders.

[nohwnd]

If you need more help merging this, let me know.

[cvpoienaru]

To solve those conflicts, you are probably just best of just grabbing all from master, and letting Roslyn analyzers to show you the compilation errors, I see you have few apis added to unshipped, and 1 changed in shipped (is that intentional?) Unless there are differences per TFM, the change should go into the files that are directly in the PublicAPI folder, because that file is common api for all tfms. If there is difference between netstandard1.0 and others, it should go to the file in net folder. Otherwise in the tfm specific folders.

Thanks for the suggestion, @nohwnd, that's exactly what I did and it worked better than actually trying to solve the conflicts by 3-way merge. I also moved the unshipped APIs for each TFM in the unshipped file in the PublicAPI root folder as you suggested as they were not TFM specific in the first place.

Yes, the change in the shipped file is intentional. I have actually changed that API but it should be ok. Even though it's a public API it's not been publicly announced and I'm almost certain that no one uses it. On top of that it's in ITestPlatform and I'm not aware of anyone actually trying to implement their own TestPlatform class (and not sure it's even possible to detect such a custom implementation).

Btw, can you please approve the change if you find it acceptable ? For merging I also need your approval as a code owner for the PublicAPI part :)

[Haplois]

LGTM, with some comments.