Skip to content
This repository was archived by the owner on Dec 5, 2022. It is now read-only.

Conversation

@vagisha-nidhi
Copy link
Contributor

No description provided.

@vagisha-nidhi
Copy link
Contributor Author

@ViktorHofer Can you please go through this?


```
Since these environment variables should always be set when the test host is started, the tests should always run in a separate process.
For this, the `/InIsolation` flag will be set when there are environment variables so that the test host is always invoked.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to not set the InIsolation flag when env vars are set as others who aren't running in isolation want to use this feature as well, not for the testhost invocation but inside the test app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In certain specific cases, we run the tests inside vstest.console process without starting the testhost process. Since we are planning to set the environment variables when starting the test host, these cases will not be encountered for. This flag will insure that the test host is launched with the correct variables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we support both cases?

  1. Set env vars before the testhost process is started
  2. If tests are executed directly in vstest.console, set the env vars before invoking the tests.

<!-- File name extension must be .runsettings -->
<RunSettings>
<RunConfiguration>
<EnvironmentVariables>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you usually use plural or singular? I.e. you have RunSettings (plural) but also RunConfiguration (singular).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the EnvironmentVariables, it made sense to use plural to infer that multiple values are allowed.

@ViktorHofer
Copy link
Member

Should we close this PR?

@AbhitejJohn AbhitejJohn merged commit f66716a into microsoft:master Mar 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants