Add tests for ErrorResponse without JSON#108
Conversation
WalkthroughAdds two new test classes that validate RestClient handling of error responses: one for text/plain payloads (including a custom error type) and one for XML payloads. Each test file defines test service interfaces and error-response types and uses a mocked HTTP pipeline. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test (xUnit)
participant RC as RestClientFactory / RestClient
participant Mock as Mock HTTP Handler
Note over Test,RC: Setup - create RestClient with Mock handler
Test->>RC: Invoke GetResource / GetResourceAsync
RC->>Mock: Send HTTP request
Mock-->>RC: 400 Bad Request with error body (text/plain or XML)
RC--xTest: Throw RestClientException (StatusCode=400, ErrorResponse present)
Test->>RC: exception.GetErrorResponse<T>()
RC-->>Test: Parsed error payload (string / CustomErrorMessage / XmlErrorResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.cs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (2)Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (2)
Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (6)
28-29: Drop 'Async' suffix in test method name (coding guidelines)Rename to comply with “Never include 'Async' in method names.”
- public async Task TestErrorResponse_Xml_BadRequest_Async() + public async Task TestErrorResponse_Xml_BadRequest()
17-25: Make CreateTestService expression-bodiedSimplifies boilerplate and matches the guideline.
- private ITestService CreateTestService() - { - return _restClientFactory.CreateBuilder() - .WithXml() - .With(_loggerFactory.CreateLogger<ErrorResponseXmlTest>()) - .With(_mockHttp.ToHttpClient()) - .BaseUri(BaseUri) - .Build<ITestService>(); - } + private ITestService CreateTestService() => + _restClientFactory.CreateBuilder() + .WithXml() + .With(_loggerFactory.CreateLogger<ErrorResponseXmlTest>()) + .With(_mockHttp.ToHttpClient()) + .BaseUri(BaseUri) + .Build<ITestService>();
64-72: Prefer a record for the XML DTOUse a record class per guideline; keep set accessors for XmlSerializer.
- [XmlRoot("error")] - public class XmlErrorResponse + [XmlRoot("error")] + public record class XmlErrorResponse { [XmlElement("code")] public int Code { get; set; } [XmlElement("message")] public string Message { get; set; } = ""; }
47-54: Also assert exception.Message for parity with text/plain testKeep assertions consistent across MIME types.
var errorResponse = exception.GetErrorResponse<XmlErrorResponse>(); Assert.Equal(400, errorResponse.Code); Assert.Equal("Invalid request parameter", errorResponse.Message); + Assert.Equal("Invalid request parameter", exception.Message);
40-40: Prefer 'application/xml' over 'text/xml'Use the modern, standard media type.
- .Respond(HttpStatusCode.BadRequest, "text/xml", errorXml); + .Respond(HttpStatusCode.BadRequest, "application/xml", errorXml);
30-31: Remove AAA commentsGuideline: only “why” comments. These narrate “what”.
Also applies to: 44-45, 47-48
Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (3)
19-26: Use expression-bodied buildersShorter and idiomatic for simple factory helpers.
- private ITestService CreateTestService() - { - return _restClientFactory.CreateBuilder() - .With(_loggerFactory.CreateLogger<ErrorResponseTextPlainTest>()) - .With(_mockHttp.ToHttpClient()) - .BaseUri(BaseUri) - .Build<ITestService>(); - } + private ITestService CreateTestService() => + _restClientFactory.CreateBuilder() + .With(_loggerFactory.CreateLogger<ErrorResponseTextPlainTest>()) + .With(_mockHttp.ToHttpClient()) + .BaseUri(BaseUri) + .Build<ITestService>(); @@ - private ITestServiceWithCustomError CreateTestServiceWithCustomError() - { - return _restClientFactory.CreateBuilder() - .With(_loggerFactory.CreateLogger<ErrorResponseTextPlainTest>()) - .With(_mockHttp.ToHttpClient()) - .BaseUri(BaseUri) - .Build<ITestServiceWithCustomError>(); - } + private ITestServiceWithCustomError CreateTestServiceWithCustomError() => + _restClientFactory.CreateBuilder() + .With(_loggerFactory.CreateLogger<ErrorResponseTextPlainTest>()) + .With(_mockHttp.ToHttpClient()) + .BaseUri(BaseUri) + .Build<ITestServiceWithCustomError>();Also applies to: 28-35
71-77: Assert exception.Message in custom-error test as wellParities with the first test and validates propagated message.
var customError = exception.GetErrorResponse<CustomErrorMessage>(); Assert.Equal("Invalid request parameter", customError.Message); + Assert.Equal("Invalid request parameter", exception.Message);
41-41: Remove AAA commentsKeep only “why” comments per guideline.
Also applies to: 47-47, 50-50, 62-62, 68-68, 71-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Activout.RestClient.Test/ErrorResponseTextPlainTest.cs(1 hunks)Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Use [] list syntax for collections
Use file-scoped namespaces
Use primary constructors wherever possible
Use records for data transfer objects (DTOs) and immutable data structures
Use var for local variable declarations when possible
Use expression-bodied members for simple methods and properties
Use pattern matching for type checks and deconstruction
Use using statements for resource management
Use async and await for asynchronous programming
Never include "Async" in method names
Only include comments that explain why something is done, not what is done
Files:
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.csActivout.RestClient.Test/ErrorResponseTextPlainTest.cs
🧬 Code graph analysis (2)
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (2)
Activout.RestClient/Services.cs (2)
IRestClientFactory(21-30)Services(9-36)Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (5)
ITestService(19-26)Fact(37-56)Fact(58-77)Get(83-84)Get(91-92)
Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (2)
Activout.RestClient/Services.cs (2)
IRestClientFactory(21-30)Services(9-36)Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (3)
ITestService(17-25)Fact(27-54)Get(60-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (1)
1-6: No action requiredGlobal using for Xunit is present at
Activout.RestClient.Xml.Test/GlobalUsings.cs, so the [Fact] attribute is properly available.
1 participant
Summary by CodeRabbit