Run unit tests as part of the build (and CI) #789
Conversation
.vsts-ci.yml
Outdated
| env: | ||
| SYSTEM_ACCESSTOKEN: $(System.AccessToken) | ||
| HelixAccessToken: $(HelixApiAccessToken) | ||
| - script: ./eng/common/msbuild.sh --warnaserror false --ci |
There was a problem hiding this comment.
Why do you need to add another step, Ideally we would run both with just the one call.
There was a problem hiding this comment.
You can't unless we run it on PIs as well since there is no way specify which project runs on which queue. If I create a target which call sendToHelix twice then I hit this issue: dotnet/arcade#4107
Other thing is that it makes sense to first run unit tests before running any hardware tests so that should be fine
|
How many device binding unit test projects do we have? If there aren't many, it is probably easier for now to just run these tests as part of the build. That way you won't really have to deal with Helix or build infrastructure, and you will get coverage in Windows, Linux, and OSX. Because they are unit tests, they shouldn't need to run on any specific hardware. |
|
@joperezr we have 6 unit tests projects not including System.Device.Gpio (and one coming in the PR). I don't mind either way but not sure how to do what you just said with targets (unless you meant to run |
|
seems something got stuck in helix. The tests took less than 30s on my box |
|
What's wrong with GitHub? My checks are also failing with strange Azure DevOps problems. |
|
@MarkCiliaVincenti not sure, haven't seen any e-mails internally yet |
…pendent tests on Windows (there is none)
krwq
force-pushed
the
run-unit-tests-as-part-of-ci
branch
from
48d4791 to
8b4465c
Compare
|
Rebased to restart CI since it was showing green when clicking on link but red on github |
There is an internal thread about problems with Azure Devops that we are seeing, this is very likely the cause to issues you were seeing. |
krwq
changed the title
build.proj
Outdated
| <Target Name="Test"> | ||
| <MSBuild Projects="@(UnitTestProjects)" Targets="VSTest" | ||
| ContinueOnError="ErrorAndContinue" | ||
| Properties="Verbosity=normal" /> |
There was a problem hiding this comment.
Let's just check to see if this is the best way to print out failures from running the tests. In general we try to avoid passing down global properties as they leak down into the the projects and mess up incremental build. cc: @ericstj
There was a problem hiding this comment.
Also if you're running tests as part of the build, you want to publish the xunit results in the yaml file as part of the build in CI so that test results show in the test explorer: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/test/publish-test-results?view=azure-devops&tabs=yaml#yaml-snippet
|
Ok lack of output seems to be VSTest issue: microsoft/vstest#2224 I'll remove the hack and let's wait until they fix it |
There was a problem hiding this comment.
LGTM. Only open question would be if we should keep the unit tests running by default, but I'm fine to keeping them doing so for now. If we start seeing this is a problem, we can turn them off on default build.cmd. Thanks for fixing this @krwq
Noticed that unit tests don't get run as part of the CI (only System.Device.Gpio.Tests do).
Marked as draft until I finish testing