Remove TFM parsing from xunit test target #160
Conversation
|
I made the changes affected repos to ensure net451 tests are only compiled on Windows. Will push once this is merged |
|
❌ blocked. Conditional
|
|
When this dotnet/project-system#1444 makes it into a build, we can return to conditioning TargetFrameworks on OS. |
natemcmaster
force-pushed
the
namc/test
branch
from
93690b2 to
d123f34
Compare
natemcmaster
force-pushed
the
namc/test
branch
2 times, most recently
from
4ef804a to
cbf1a96
Compare
|
🔔 Updated and rebased. Ready for review. |
natemcmaster
force-pushed
the
namc/test
branch
from
b6764f8 to
51922dd
Compare
| <Target Name="TestProjects"> | ||
|
|
||
| <ItemGroup Condition="'@(ProjectsToTest)'==''"> | ||
| <!-- put unit test projects ahead of functional tests --> |
There was a problem hiding this comment.
Do we actually have this as a convention? Tests -> Functional? Otherwise this is kinda misleading.
There was a problem hiding this comment.
Plus we don't super care about ordering. Ideally we should try and these in parallel.
There was a problem hiding this comment.
This is something EF has requested. Running unit tests first to catch errors early is nice, esp when functional tests can take a long time to run.
There was a problem hiding this comment.
Can we explicit about it - requiring functional tests to include the term FunctionalTests? Alternatively have repo.props determine order if it's necessary
| <!-- put unit test projects ahead of functional tests --> | ||
| <ProjectsToTest Include="$(RepositoryRoot)test\*\*.Tests.csproj" Exclude="$(ExcludeFromTest)" /> | ||
| <ProjectsToTest Include="$(RepositoryRoot)test\*\*.Test.csproj" Exclude="$(ExcludeFromTest)" /> | ||
| <ProjectsToTest Include="$(RepositoryRoot)test\*\*.csproj" Exclude="@(ProjectsToTest);$(ExcludeFromTest)" /> |
There was a problem hiding this comment.
Wouldn't this suffice? Also, why not make ExcludeFromTest an item group?
There was a problem hiding this comment.
And item group might be better. I ran into this dotnet/msbuild#1576 which means setting ExcludeFromTest in repo.targets correctly is not easy.
There was a problem hiding this comment.
Hmm, nvm. ItemGroups and remove syntax suffer from the same issue.
| <!-- TODO if VS Test doesn't make minimal output the default log setting, we can set that here. cref https://github.com/Microsoft/vstest/issues/301 --> | ||
| <VSTestLogger Condition=" '$(VSTestLogger)'=='' AND '$(TEAMCITY_VERSION)' != '' ">trx</VSTestLogger> | ||
| <IgnoreFailingTestProjects>false</IgnoreFailingTestProjects> | ||
| <ContinueOnTestError Condition="'$(KOREBUILD_IGNORE_DOTNET_TEST_EXIT_CODE)' == '1'">true</ContinueOnTestError> |
There was a problem hiding this comment.
Should this be IgnoreFailingTestProjects? ContinueOnTestError isn't used elsewhere.
There was a problem hiding this comment.
Yup, good catch. Missed that rename.
build/targets/DefaultItems.targets
Outdated
| <SharedSourceDirectories Remove="@(ExcludeSharedSourceDirectories)" /> | ||
|
|
||
| <!-- put unit test projects ahead of functional tests --> | ||
| <ProjectsToTest Include="$(RepositoryRoot)test\*\*.Tests.csproj" /> |
There was a problem hiding this comment.
<ItemGroup Condition="'@(ProjectsToTest)'==''">
<_FunctionalTests Include="$(RepositoryRoot)test\*FunctionalTest*.csproj" />
<ProjectsToTest Include="$(RepositoryRoot)test\*Test*.csproj" Exclude="@(_FuctionalTests)" />
<ProjectsToTest Include="@(_FunctionalTests)" />
...
</ItemGroup>There was a problem hiding this comment.
👍
Although, the thinking behind not doing Condition="'@(ProjectsToTest)'==''" is that the most common scenario here is that we will want to exclude a project from testing ExcludeFromTest that was picked up by default globs. Redefining the default globs is cumbersome.
There was a problem hiding this comment.
Sure, we could add it if we need it later on.
natemcmaster
force-pushed
the
namc/test
branch
from
88da556 to
6ba347c
Compare
Requires update to projects that use .NET Framework in test projects. These should be updated to only include .NET Framework when building/testing on Windows.