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

Show error message on Framework35 #1723

Merged
merged 7 commits into from
Aug 9, 2018

Conversation

smadala
Copy link
Contributor

@smadala smadala commented Aug 7, 2018

Description

  • Show error message when user pass /Framework:Framework35 as we don't have testhost built for CLR 2.0

Related issue

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/493289

@smadala smadala requested a review from singhsarab August 9, 2018 06:25
@smadala smadala self-assigned this Aug 9, 2018
@@ -276,11 +276,14 @@
<data name="FileNotFound" xml:space="preserve">
<value>'{0}' not found.</value>
</data>
<data name="Framework35NotSupported" xml:space="preserve">
<value>Framework35 not supported. Use Framework40 or above to run tests in CLR 4.0 "compatibly mode".</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:
Framework35 is not supported. For projects targeting .Net Framework 3.5, please use Framework40 to run tests in CLR 4.0 "compatibility mode".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@singhsarab singhsarab left a comment

Choose a reason for hiding this comment

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

Approving with suggestions.

@cltshivash @pvlakshm please review the change in behaviour

string runSettingsXml = @"<?xml version=""1.0"" encoding=""utf-8""?>
<RunSettings>
<RunConfiguration>
<TargetFrameworkVersion>Framework35</TargetFrameworkVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test with explicit version, .NetFramework,version=3.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -75,7 +81,8 @@ public void HandleRawMessage(string rawMessage)

public void HandleLogMessage(TestMessageLevel level, string message)
{
// No Op
this.TestMessageLevels.Add(level);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead use a List of keyValuePairs or define a struct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@smadala smadala merged commit 310968a into microsoft:master Aug 9, 2018
@smadala smadala deleted the add-warning-fx35-assembly branch August 9, 2018 11:41
@pvlakshm
Copy link
Contributor

Are we erroring out if Framework35 is specified? We should not. Today, we run such tests in CLR 4.0's compat mode. Just call that out as a warning so the user is more aware.

@smadala
Copy link
Contributor Author

smadala commented Aug 10, 2018

@pvlakshm In Tpv1 if user passes Framework35, tests runs in CLR 2.0 process. From VS 15.5 we have changed that behavior to run tests CLR 4.0 process in compact mode. If we don't error out user might think his tests running in CLR2.0 process.

As we don't support running tests in CLR 2.0 process, we should remove the /Framework:Framework35.

@smadala
Copy link
Contributor Author

smadala commented Aug 10, 2018

Just call that out as a warning so the user is more aware.

Error makes more awareness. More details refer #1643 (comment)

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

Successfully merging this pull request may close these issues.

3 participants