Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1083 +/- ##
=======================================
Coverage 99.33% 99.33%
=======================================
Files 15 15
Lines 896 896
Branches 205 205
=======================================
Hits 890 890
Misses 4 4
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the project from .NET 9.0 to .NET 10.0, updating target frameworks, SDK version, and package versions across the codebase.
- Updates all target frameworks from net9.0 to net10.0
- Updates .NET SDK from version 9.0.307 to 10.0.100
- Updates System.Text.Json package from 8.0.5 to 10.0.0
- Removes
usingstatements in test code and modifies test assertions
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/HttpClientInterception.Tests/JustEat.HttpClientInterception.Tests.csproj | Updates target framework to net10.0 |
| tests/HttpClientInterception.Tests/InterceptingHttpMessageHandlerTests.cs | Removes using statements for disposable objects |
| tests/HttpClientInterception.Tests/Examples.cs | Removes TaskCanceledException assertion from test |
| tests/HttpClientInterception.Benchmarks/JustEat.HttpClientInterception.Benchmarks.csproj | Updates target frameworks to net472 and net10.0, removing net8.0 and net9.0 |
| samples/SampleApp/SampleApp.csproj | Updates target framework to net10.0 and removes System.Text.Json override |
| samples/SampleApp.Tests/SampleApp.Tests.csproj | Updates target framework to net10.0 and reformats WebApplicationFactoryContentRootAttribute |
| global.json | Updates SDK version to 10.0.100 |
| Directory.Packages.props | Updates System.Text.Json to version 10.0.0 |
| .vsconfig | Updates Visual Studio runtime component to 10.0 |
| .vscode/launch.json | Updates debug output path to net10.0 |
Comments suppressed due to low confidence (8)
tests/HttpClientInterception.Tests/Examples.cs:570
- Disposable 'HttpResponseMessage' is created but not disposed.
OnMissingRegistration = (request) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound)),
tests/HttpClientInterception.Tests/InterceptingHttpMessageHandlerTests.cs:71
- Disposable 'HttpResponseMessage' is created but not disposed.
var expected = new HttpResponseMessage(HttpStatusCode.OK);
tests/HttpClientInterception.Tests/InterceptingHttpMessageHandlerTests.cs:72
- Disposable 'HttpRequestMessage' is created but not disposed.
var request = new HttpRequestMessage(HttpMethod.Options, "https://google.com/foo");
tests/HttpClientInterception.Tests/InterceptingHttpMessageHandlerTests.cs:155
- Disposable 'HttpResponseMessage' is created but not disposed.
var response = new HttpResponseMessage(HttpStatusCode.Accepted)
{
Content = new StringContent(string.Empty),
};
tests/HttpClientInterception.Tests/Examples.cs:32
- This assignment to builder is useless, since its value is never read.
var builder = new HttpRequestInterceptionBuilder()
.Requests()
.ForHost("public.je-apis.com")
.WithStatus(HttpStatusCode.InternalServerError)
.RegisterWith(options);
tests/HttpClientInterception.Tests/Examples.cs:619
- This assignment to builder is useless, since its value is never read.
var builder = new HttpRequestInterceptionBuilder()
.Requests().For((request) => request.RequestUri.Host == "google.com").HavingPriority(1)
.Responds().WithContent(@"First")
.RegisterWith(options)
.Requests().For((request) => request.RequestUri.Host.Contains("google", StringComparison.OrdinalIgnoreCase)).HavingPriority(2)
.Responds().WithContent(@"Second")
.RegisterWith(options)
.Requests().For((request) => request.RequestUri.PathAndQuery.Contains("html", StringComparison.OrdinalIgnoreCase)).HavingPriority(3)
.Responds().WithContent(@"Third")
.RegisterWith(options)
.Requests().For((request) => true).HavingPriority(null)
.Responds().WithContent(@"Fourth")
.RegisterWith(options);
tests/HttpClientInterception.Tests/Examples.cs:753
- This assignment to builder1 is useless, since its value is never read.
var builder1 = new HttpRequestInterceptionBuilder()
.ForAll([IsHttpGetForJustEatGitHubOrg, _ => requestCount < 2])
.WithInterceptionCallback(IncrementRequestCount)
.Responds()
.WithStatus(HttpStatusCode.TooManyRequests)
.WithSystemTextJsonContent(new { message = "Too many requests" })
.RegisterWith(options);
tests/HttpClientInterception.Tests/Examples.cs:762
- This assignment to builder2 is useless, since its value is never read.
var builder2 = new HttpRequestInterceptionBuilder()
.ForAll([IsHttpGetForJustEatGitHubOrg, _ => requestCount >= 2])
.WithInterceptionCallback(IncrementRequestCount)
.Responds()
.WithStatus(HttpStatusCode.OK)
.WithSystemTextJsonContent(new { id = 1516790, login = "justeattakeaway", url = "https://api.github.com/orgs/justeattakeaway" })
.RegisterWith(options);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var handler = Substitute.For<HttpMessageHandler>(); | ||
| var expected = new HttpResponseMessage(HttpStatusCode.OK); | ||
| var request = new HttpRequestMessage(HttpMethod.Options, "https://google.com/foo"); |
There was a problem hiding this comment.
Removing 'using' statements for IDisposable objects (HttpResponseMessage, HttpRequestMessage, HttpMessageHandler, and HttpClient at line 79) will cause resource leaks. These objects implement IDisposable and should be properly disposed to release unmanaged resources like network connections and handles.
| // Act | ||
| await Should.ThrowAsync<TaskCanceledException>( | ||
| () => client.GetAsync("http://www.google.co.uk", cts.Token)); | ||
| await client.GetAsync("http://www.google.co.uk", cts.Token); |
There was a problem hiding this comment.
Removing the TaskCanceledException assertion eliminates the test's verification logic. The test named 'Inject_Latency_For_Http_Get_With_Cancellation' should verify that the operation throws TaskCanceledException when cancelled. Without this assertion, the test no longer validates the expected behavior.
No description provided.