Skip to content

Caitie/update test templates #10662

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Caitie/update test templates #10662

wants to merge 6 commits into from

Conversation

CaitieM20
Copy link

Description

Updating the Test Templates to properly dispose of resources used (Cancellation Token, HttpRequest, HttpResponse). We should ensure our templates use best practices. These were changes I needed to make before checking in to my own project.

Updated MSTest Template to leverage the CancellationToken from TestContext, XUnit Template already does this.

Fixes # (issue)

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • [] No
    • No
  • Does the change require an update in our Aspire docs?

@Copilot Copilot AI review requested due to automatic review settings July 24, 2025 21:44
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 24, 2025
Copilot
Copilot AI reviewed Jul 24, 2025
View reviewed changes
Copy link
Contributor

@Copilot 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 Aspire test project templates to follow proper resource disposal best practices by adding using declarations for disposable resources and improving cancellation token usage across multiple test framework templates.

  • Updates resource disposal patterns for HttpClient, HttpResponse, and CancellationTokenSource
  • Standardizes cancellation token usage to leverage framework-provided tokens in MSTest templates
  • Applies consistent best practices across XUnit, NUnit, and MSTest project templates for versions 9.3 and 9.4

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Aspire.ProjectTemplates/templates/aspire-xunit/9.4/IntegrationTest1.cs Added using declarations for CancellationTokenSource, HttpClient, and HttpResponse
src/Aspire.ProjectTemplates/templates/aspire-xunit/9.3/IntegrationTest1.cs Added using declarations for CancellationTokenSource, HttpClient, and HttpResponse
src/Aspire.ProjectTemplates/templates/aspire-nunit/9.4/IntegrationTest1.cs Added using declarations for CancellationTokenSource, HttpClient, and HttpResponse
src/Aspire.ProjectTemplates/templates/aspire-nunit/9.3/IntegrationTest1.cs Added using declarations for CancellationTokenSource, HttpClient, and HttpResponse
src/Aspire.ProjectTemplates/templates/aspire-mstest/9.4/IntegrationTest1.cs Added TestContext property and updated to use framework cancellation token plus using declarations
src/Aspire.ProjectTemplates/templates/aspire-mstest/9.3/IntegrationTest1.cs Added TestContext property and updated to use framework cancellation token plus using declarations
Comments suppressed due to low confidence (2)

@danmoseley
Copy link
Member

LGTM save the errors copilot pointed out above.
@radical do we have tests that attempt to uncomment and build this template content? I assume not (eg., there are real commnts mixed with commented code). If so it's important to test all the commented code locally. I would do this by installing the templates and newing up each one, then manually uncommenting .. it's a pain.

@danmoseley
Copy link
Member

I think it would be fine if you like to only update the 9.4 ones. The 9.3 ones will go away soon.

@CaitieM20
Copy link
Author

@CaitieM20 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@radical
Copy link
Member

radical commented Jul 24, 2025

@radical do we have tests that attempt to uncomment and build this template content? I assume not (eg., there are real commnts mixed with commented code).

We do uncomment and run the tests in the individual test templates. And they are failing with IntegrationTest1.cs(22,10): error CS1674: 'CancellationToken': type used in a using statement must implement 'System.IDisposable'.

CaitieM20 and others added 3 commits July 25, 2025 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesNK JamesNK JamesNK left review comments

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area-templates community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CaitieM20 @danmoseley @radical @JamesNK