Skip to content

Conversation

CaitieM20
Copy link
Contributor

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
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
Contributor 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'.

@davidfowl
Copy link
Member

@danmoseley can you give this another review?

@davidfowl
Copy link
Member

@danmoseley @radical

@radical
Copy link
Member

radical commented Sep 2, 2025

Fixed the merge conflicts, and add missing tests for 9.4 .

Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution @CaitieM20!

@radical radical merged commit 5029999 into dotnet:main Sep 2, 2025
588 of 590 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.5 milestone Sep 2, 2025
Copilot AI pushed a commit that referenced this pull request Sep 3, 2025
* updating test templates to dispose of resources correctly

* comment out code

* making sure comment lines weren't unnecessarily changed

* Update src/Aspire.ProjectTemplates/templates/aspire-xunit/9.4/IntegrationTest1.cs

Co-authored-by: Copilot <[email protected]>

* integrating feedback

* Add missing tests for 9.4

* cleanup

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
This was referenced Oct 1, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-templates community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants