Skip to content
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

Enable VS Test Explorer without the -vs switch #36126

Merged
merged 2 commits into from
May 8, 2020

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented May 8, 2020

Creating an intermediate runsettings file outside of the app's OutDir to enable VS Test Explorer support without the -vs switch.

I'll remove the -vs switch in the next batched rollout and for now added a warning that the switch will be removed eventually.

Either a clean build or a libs.pretest (subset) build is necessary to consume the change locally.

cc @ericstj @danmosemsft @stephentoub @ManickaP

@ghost
Copy link

ghost commented May 8, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

eng/build.ps1 Show resolved Hide resolved
@ViktorHofer
Copy link
Member Author

Failure is #29642

@ViktorHofer ViktorHofer merged commit fd82afe into dotnet:master May 8, 2020
@ViktorHofer ViktorHofer deleted the VSTestExplorerSupport branch May 8, 2020 20:12
@krwq
Copy link
Member

krwq commented May 8, 2020

Thank you @ViktorHofer!

@GrabYourPitchforks
Copy link
Member

Does this mean that when we run build -s libs.pretest -rc Debug|Checked|Release, VS will automatically pick up that runtime configuration for the purpose of running tests?

@safern
Copy link
Member

safern commented May 14, 2020

Does this mean that when we run build -s libs.pretest -rc Debug|Checked|Release, VS will automatically pick up that runtime configuration for the purpose of running tests?

For running the tests it should because it will use the testhost, but what I don't think it will work, will be building/intellisense of projects that have a ReferenceFromRuntime to S.P.CoreLib for example. We should figure out how to make that work.

@ViktorHofer
Copy link
Member Author

Does this mean that when we run build -s libs.pretest -rc Debug|Checked|Release, VS will automatically pick up that runtime configuration for the purpose of running tests?

It picks the runtime that's present in the matching testhost folder. Means if you build against Debug it picks the runtime that's present in the debug testhost which depends on the -runtimeconfiguration value that you chose when building from root.

I don't think it will work, will be building/intellisense of projects that have a ReferenceFromRuntime to S.P.CoreLib for example. We should figure out how to make that work.

There are hacky ways to support that, like setting environment variables inside VS or opening it with an env var being set. You could temporarily set the <RuntimeConfiguration> property inside the project as well. @safern why do you think is the runtime's configuration for ReferenceFromRuntime important?

As a side note, going forward I hope we can use a P2P against the right CoreLib instead of a named reference.

@safern
Copy link
Member

safern commented May 14, 2020

@safern why do you think is the runtime's configuration for ReferenceFromRuntime important?

Because of the way we resolve the reference from runtime. We translate it to a reference to System.Private.CoreLib.dll and that uses RuntimeConfiguration to resolve the location of corelib to reference it.

@ViktorHofer
Copy link
Member Author

OK, so switching these ReferenceFromRuntime items into actual P2Ps will solve that.

@ericstj
Copy link
Member

ericstj commented May 14, 2020

Are you guys over-thinking this? I wouldn't expect CoreLib's surface area to change based on RuntimeConfiguration (though, unfortunately it does based on OS). As such it won't really matter what S.P.C is used at build time so long as it exists.

FWIW I don't see how using a P2P is going to change anything here, except maybe force a rebuild of a different S.P.C than is in testhost.

@ViktorHofer
Copy link
Member Author

Are you guys over-thinking this? I wouldn't expect CoreLib's surface area to change based on RuntimeConfiguration (though, unfortunately it does based on OS). As such it won't really matter what S.P.C is used at build time so long as it exists.

I think @safern's point is that referenceFromRuntime resolves CoreLib based on the RuntimeConfiguration switch via a named reference. It doesn't build it, if it doesn't exist. Imagine you build CoreLib in Release and now try to build a library which contains a <ReferenceFromRuntime Include="System.Private.CoreLib" /> item. The reference won't be resolved as CoreLib hasn't been built in the configured it is probed for which would be Debug.

@ericstj
Copy link
Member

ericstj commented May 14, 2020

I see. We could drop a props file as part of pretest that would indicate what CoreLib to use and use that when building in VS. I can also imagine a P2P making this better, but I'm not sure how far out that work is.

Let's stop this discussion in merged PR. Test the hypothetical and file an issue if something's broken.

@ViktorHofer
Copy link
Member Author

Filed #36464 to continue the discussion.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants