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

Use MSTest.Sdk (and migrate from VSTest to MTP) #728

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Nov 27, 2024

Fixes #730

@Youssef1313 Youssef1313 requested review from a team as code owners November 27, 2024 19:30
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Few notes:

  1. You haven't enabled MSTest runner on all projects
  2. You will need to update the pipeline setup to use new arguments
  3. For now the new runner doesn't support coverlet through coverlet.collector so you can remove the deps on all projects. If owners are fine, we can switch to MS code coverage otherwise as part of the pipeline update we will need to move to coverlet dotnet tool

@Youssef1313 Youssef1313 changed the title Use TestMethodAttribute and use MSTest.Sdk Use MSTest.Sdk Nov 27, 2024
@Youssef1313
Copy link
Member Author

@joem-msft Can I get a review on this PR please?

Comment on lines -43 to -47
- name: Test - Persistence
run: dotnet bin/Debug/Persistence.Tests/Persistence.Tests.dll

- name: Test - PAModel
run: dotnet bin/Debug/PAModelTests/PAModelTests.dll
Copy link
Member Author

Choose a reason for hiding this comment

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

@joem-msft FYI the assembly YamlValidator.Tests.dll wasn't run in CI. It's better to dotnet test on the solution rather than listing individual dlls.

Copy link
Contributor

Choose a reason for hiding this comment

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

please do not start including the YamlValidator.Tests in the CI builds, as this project has been abandoned and not maintained.

- name: Test - PAModel
run: dotnet bin/Debug/PAModelTests/PAModelTests.dll
- name: Test
run: dotnet test src/PASopa.sln --no-build
Copy link
Member

Choose a reason for hiding this comment

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

For maintainers: I would recommend to enable TRX report and Code Coverage.

Copy link
Contributor

Choose a reason for hiding this comment

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

go ahead and enable if you would like.

@Youssef1313 Youssef1313 changed the title Use MSTest.Sdk Use MSTest.Sdk (and migrate from VSTest to MTP) Feb 18, 2025
@Youssef1313
Copy link
Member Author

@mesganmsft Thanks for reviewing! Is this good to merge now?

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.

Tests should be run from the solution file
4 participants