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

Build testhost configuration agnostic #35112

Open
ViktorHofer opened this issue Apr 17, 2020 · 11 comments
Open

Build testhost configuration agnostic #35112

ViktorHofer opened this issue Apr 17, 2020 · 11 comments

Comments

@ViktorHofer
Copy link
Member

Compose the testhost configuration agnostic (don't encode the configuration in the layout) and allow both Debug and Release built tests to run against it. This removes the necessity of an additional full vertical build for testing on different configurations. Additionally this allows to encode the path to the testhost folder into the .runsettings file to make dotnet test just work.

cc @ericstj @Anipik

@ghost
Copy link

ghost commented Apr 17, 2020

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

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Apr 17, 2020
@safern
Copy link
Member

safern commented Apr 17, 2020

So this means that if I build for Debug locally and then for Release, Release build will override the testhost. Is that the desired behavior? Our libraries are not Configuration agnostic as we have multiple Debug.Asserts and others.

@ericstj
Copy link
Member

ericstj commented Apr 17, 2020

TestHost would still be OS specific but the idea was that we could eliminate Debug/Release differences and make it last-in-wins. That would let folks test a mix of debug and release easily. Today you can only do all debug / all release and as a result you need to do a vertical build for each if you wish to run tests for that configuration. A hook was added for "RuntimeConfiguration" to allow for a different CoreCLR (Release) with libraries (debug).

@safern
Copy link
Member

safern commented Apr 17, 2020

TestHost would still be OS specific

I meant Configuration agnostic.

but the idea was that we could eliminate Debug/Release differences and make it last-in-wins.

But they're still different.

Today you can only do all debug / all release and as a result you need to do a vertical build for each if you wish to run tests for that configuration.

Even if we don't encode the configuration on the layout is the same thing. If I have built for Debug and now want to run against a Release shared framework, I have to do the vertical again and that will override the Debug shared framework I had on the testhost, and now I can't run them both. If I wanted to test product differences in between them, I would have to go and do a full vertical again for Debug before been able to run the tests.

Maybe I'm miss-understanding how would this work.

@ericstj
Copy link
Member

ericstj commented Apr 17, 2020

This issue assumes that folks don't wish to maintain separate debug & release testhost paths in the same clone. It presumes that folks would instead prefer a single one. It sounds like you're in a different camp.

@safern
Copy link
Member

safern commented Apr 17, 2020

It sounds like you're in a different camp.

Not really. I agree it would make lives easier in the sense of infra related stuff, but I was just trying to think of scenarios that other people might use/do as this would break those.

@ericstj
Copy link
Member

ericstj commented Apr 17, 2020

The premise of this issue that the new behavior is "better", but I think that's based on a couple opinions. @safern you are correct to bring up the fact that the behavior is different. How do you propose we decide if it's better?

@safern
Copy link
Member

safern commented Apr 17, 2020

@safern you are correct to bring up the fact that the behavior is different. How do you propose we decide if it's better?

I think we should scout and follow the infra rolling changes if we do it. But I think we could either ask on the teams group, or send an email and also consider gitter for externals. Just to get a sense of how many people actually rely on the fact of building multiple testhost and using them without cleaning.

cc: @jkotas @stephentoub

@jkotas
Copy link
Member

jkotas commented Apr 17, 2020

My thoughts:

  • Having one testhost location that does not depend on configuration sounds good to me. I think it is the right default option for convenience of the inner dev-loop.
  • I think this change should be coupled together with the change to build everything as Release by default when you build from root. Again, I think it is the right default option for convenience of the inner-dev loop.
  • There is no clean isolation between Debug and Release today away, e.g. you cannot build Debug and Release in parallel (you will get file sharing violations, etc.). I do not trust the current build environment to place the right bits into the right flavor for me - got burned a few times. If it is easier, it would be fine with me to not have Debug/Release captured in the artifacts paths at all.
  • The name of the top level artifacts directory is hardcoded. I would like to have an option to specify my custom top level artifacts directory (e.g. using environment variable) and have it respected, so that I can build e.g. Checked and Release in parallel; or can easily have binaries with change A and change B side-by-side. It is a very common workflow for me and I have to deal with it by having multiple enlistments or renaming artifacts directory back and forth that is easy to get lost in.

@khushal1996
Copy link
Contributor

I am still facing a similar issue today. I am building clr in debug mode and libraries in release mode but the default command still tries to access the dotnet.exe from the debug directory.

 '"C:\Users\kmodi\Documents\Git_repos\runtime\artifacts\bin\testhost\net8.0-windows-Debug-x64\dotnet.exe"' is not recognized as an internal or external command,
  operable program or batch file.
  ----- end Fri 05/05/2023 13:26:48.83 ----- exit code 9009 ----------------------------------------------------------
C:\Users\kmodi\Documents\Git_repos\runtime\eng\testing\tests.targets(184,5): error : One or more tests failed while running tests from 'System.Runtime.Tests'. [C:\Users\kmodi\Documents\Git_repos\runtime\src\libraries\System.Runtime\tests\System.Runtime.Tests.csproj::TargetFramew
ork=net8.0-windows]

@ViktorHofer
Copy link
Member Author

You likely invoked dotnet test or dotnet build /t:Test without the libraries configuration: -c Release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants