Skip to content

Conversation

@nohwnd
Copy link
Contributor

@nohwnd nohwnd commented Jul 4, 2025

Update to align with our current recommendations on how to use MSTest.

Pull Request Checklist

  • I have read and followed the CONTRIBUTING.md guidelines.
  • My contribution adds a new instruction, prompt, or chat mode file in the correct directory.
  • The file follows the required naming convention.
  • The content is clearly structured and follows the example format.
  • I have tested my instructions, prompt, or chat mode with GitHub Copilot.
  • I have run node update-readme.js and verified that README.md is up to date.

Description


Type of Contribution

  • New instruction file.
  • New prompt file.
  • New chat mode file.
  • Other (please specify): Updating current instruction file.

Additional Notes


By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.

Update to align with our current recommendations on how to use MSTest.
Copilot AI review requested due to automatic review settings July 4, 2025 08:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the MSTest guidance in csharp-mstest.prompt.md to align with current best practices.

  • Switched from referencing multiple MSTest packages to a single MSTest package
  • Updated data-driven attribute recommendation from [DataTestMethod] to [TestMethod]
  • Changed the exception assertion from Assert.ThrowsException<T> to Assert.Throws<T>

## Data-Driven Tests

- Use `[DataTestMethod]` combined with data source attributes
- Use `[TestMethod]` combined with data source attributes

Choose a reason for hiding this comment

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

GitHub isn't allowing me to add a comment on the specific line. But on line 43, there is a mention of CsvDataSource. I'm not aware of such attribute 😄
We only have DataSourceAttribute which can be set to use Csv. A sample for using Csv is https://github.com/microsoft/testfx/blob/b1edce21fa74ac0444f92355d230a498196a3382/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/DataSourceTests.cs, which is a bit involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://grep.app/search?q=%5BCsvDataSource%5D yeah that one seemed curious to me as well, does not seem to exist.

Choose a reason for hiding this comment

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

Probably worth removing it then

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah as they are outside the diff commenting isn't supported.

Let's get that fixed and then I can merge these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


- Use a separate test project with naming convention `[ProjectName].Tests`
- Reference Microsoft.NET.Test.Sdk, MSTest.TestAdapter, and MSTest.TestFramework packages
- Reference MSTest package
Copy link

Choose a reason for hiding this comment

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

Consider noting MSTest.Sdk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can suggest it, but not all repos use MTP and mstest.sdk is still not nicely supported in VS because of the nuget tooling, so I would not default to it.

## Data-Driven Tests

- Use `[DataTestMethod]` combined with data source attributes
- Use `[TestMethod]` combined with data source attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah as they are outside the diff commenting isn't supported.

Let's get that fixed and then I can merge these changes.

@aaronpowell aaronpowell merged commit 064bbfe into github:main Jul 8, 2025
2 checks passed
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.

4 participants