-
Notifications
You must be signed in to change notification settings - Fork 22
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
Revive unit test project #117
Conversation
cp cs |
<Import Project="..\packages\MSTest.TestAdapter.1.4.0\build\net45\MSTest.TestAdapter.props" Condition="Exists('..\packages\MSTest.TestAdapter.1.4.0\build\net45\MSTest.TestAdapter.props')" /> | ||
<PropertyGroup> | ||
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | ||
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | ||
<ProjectGuid>{14542232-E0B6-4FDD-8B35-8F3EAE889C51}</ProjectGuid> | ||
<OutputType>Library</OutputType> | ||
<AppDesignerFolder>Properties</AppDesignerFolder> | ||
<RootNamespace>STROOPUnitTests</RootNamespace> | ||
<AssemblyName>STROOPUnitTests</AssemblyName> | ||
<TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cp f
same |
<RootNamespace>STROOPUnitTests</RootNamespace> | ||
<AssemblyName>STROOPUnitTests</AssemblyName> | ||
<TargetFramework>net462</TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main project targets 4.6.1. Does this break anything if we switch the unit test project to that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind if we have to switch to v4.6.2, only reason is its just another framework to install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Figured it out! Would you believe this is the second time this week I've tracked down the cause of a bug as "MS decided this shouldn't work with certain TFMs anymore" with no other reason and no docs.
Switched to new format as I did for the main project in #96, but all at once this time because it's relatively simple.
I had to bump the TFM to(And I learned .NET Framework tests run on Linux.)net462
for one of the NuGet packages, but I don't expect that will ever be an issue.Explicitly pinned C# version at 7.1, though it could have been 7.0 but for one file. (When .NET Framework 4.6.1 released the latest would have been 6, but obviously the project adopted 7.0 soon after because tuples are used heavily.) I suggest you resolve all the existing warnings—and maybe add more Analyzers and fix their warnings too—before switching to 12 and adopting new features. Maybe you could go for 7.3 immediately, but 8 would be huge in terms of how much of the codebase you'll want to touch.