-
Notifications
You must be signed in to change notification settings - Fork 268
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
Onboard to new dotnet test experience #5111
base: main
Are you sure you want to change the base?
Conversation
…directory when running integration tests
37438c5
to
95f1ae5
Compare
azure-pipelines.yml
Outdated
-configuration $(_BuildConfig) | ||
-ci | ||
-nobl | ||
- script: $(Build.SourcesDirectory)/.dotnet/dotnet test -c $(_BuildConfig) --no-build -bl:$(BUILD.SOURCESDIRECTORY)\artifacts\log\$(_BuildConfig)\TestStep.binlog |
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.
We have 3 options:
- Keep PRs using dotnet test, and official build using Test.cmd
- Update both to use dotnet test, and we won't care about Test.cmd
- Update both pipelines so that they test with both approaches, which will increase CI time.
- Testing both is better to happen in two different jobs (not two steps of the same job). Maybe it's even the time to refactor CI a bit and have multiple stages?
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 think it would be nice if we used dotnet test in both, it would push us to have simple experience for stuff like dotnet test --project-filter !*Integration*
, and overall better excercising of our own experience.
on the other hand this is a departure of what @Evangelink said in some other post (paraprasing): We are under arcade, that does things one way, and we should keep doing it the same way so common gestures in repos remain the same (like running tests).
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.
But in this case I think being able to use our own tools, to experience the ease to use (or lack of), everyday outweights being standard. Especially if test.cmd continues to work.
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.
@nohwnd we can easily add a fake target that would fail when user do -test
with arcade and recommend to use dotnet test
instead. The suggestion to have both for some time was just to ensure we don't miss some tests but we can also have the routine of checking number of tests for some time or to start with minimal number of tests flag set to what was run before moving to dotnet test
.
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 was thinking about source-build here probably, but we did not end up enrolling into that, so I don't have many concerns about other people that are familiar with arcade having to routinely build and test our repo.
# Because the build step is using -ci flag, restore is done in a local .packages directory. | ||
# We need to pass NUGET_PACKAGES so that when dotnet test is doing evaluation phase on the projects, it can resolve .props/.targets from packages and import them. | ||
# Otherwise, props/targets are not imported. It's important that they are imported so that IsTestingPlatformApplication ends up being set. | ||
- script: $(Build.SourcesDirectory)/.dotnet/dotnet test -p:NUGET_PACKAGES=$(BUILD.SOURCESDIRECTORY)\.packages -c $(_BuildConfig) --no-build -bl:$(BUILD.SOURCESDIRECTORY)\artifacts\log\$(_BuildConfig)\TestStep.binlog |
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.
Reminder to self: This step is for the Windows job. I need to switch to the new experience for Linux and MacOS jobs as well.
No description provided.