Skip to content

Commit 0ceb4ac

Browse files
committed
Address CodeRabbit review feedback
- Fix double-dispose of HttpClientHandler in example (use disposeHandler: true) - Switch serialization tests from .Result to async/await - Add [Category("Integration")] to integration tests for CI filtering
1 parent 4872a12 commit 0ceb4ac

2 files changed

Lines changed: 44 additions & 36 deletions

File tree

examples/WatermarkAndRotate/Program.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@
2222

2323
static async Task<string> CreateWatermarkedAndRotatedPdf(string destinationDirectory, GotenbergSharpClientOptions options)
2424
{
25-
using var handler = new HttpClientHandler();
26-
using var authHandler = !string.IsNullOrWhiteSpace(options.BasicAuthUsername) && !string.IsNullOrWhiteSpace(options.BasicAuthPassword)
27-
? new BasicAuthHandler(options.BasicAuthUsername, options.BasicAuthPassword) { InnerHandler = handler }
28-
: null;
25+
var handler = new HttpClientHandler();
26+
HttpMessageHandler effectiveHandler = handler;
2927

30-
using var httpClient = new HttpClient(authHandler ?? (HttpMessageHandler)handler)
28+
if (!string.IsNullOrWhiteSpace(options.BasicAuthUsername) && !string.IsNullOrWhiteSpace(options.BasicAuthPassword))
29+
effectiveHandler = new BasicAuthHandler(options.BasicAuthUsername, options.BasicAuthPassword) { InnerHandler = handler };
30+
31+
using var httpClient = new HttpClient(effectiveHandler, disposeHandler: true)
3132
{
3233
BaseAddress = options.ServiceUrl,
3334
Timeout = options.TimeOut

test/GotenbergSharpClient.Tests/CrossCuttingOptionsTests.cs

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public void SetStampOptions_TextStamp_SetsProperties()
153153
#region Serialization Tests
154154

155155
[Test]
156-
public void RotationOptions_SerializesCorrectly()
156+
public async Task RotationOptions_SerializesCorrectly()
157157
{
158158
var options = new RotationOptions
159159
{
@@ -163,16 +163,17 @@ public void RotationOptions_SerializesCorrectly()
163163

164164
var httpContents = options.ToHttpContent().ToList();
165165

166-
httpContents.FirstOrDefault(c =>
167-
c.Headers.ContentDisposition?.Name == "rotateAngle")!
168-
.ReadAsStringAsync().Result.Should().Be("90");
169-
httpContents.FirstOrDefault(c =>
170-
c.Headers.ContentDisposition?.Name == "rotatePages")!
171-
.ReadAsStringAsync().Result.Should().Be("1-3");
166+
var angleContent = httpContents.FirstOrDefault(c =>
167+
c.Headers.ContentDisposition?.Name == "rotateAngle")!;
168+
(await angleContent.ReadAsStringAsync()).Should().Be("90");
169+
170+
var pagesContent = httpContents.FirstOrDefault(c =>
171+
c.Headers.ContentDisposition?.Name == "rotatePages")!;
172+
(await pagesContent.ReadAsStringAsync()).Should().Be("1-3");
172173
}
173174

174175
[Test]
175-
public void SplitOptions_SerializesCorrectly()
176+
public async Task SplitOptions_SerializesCorrectly()
176177
{
177178
var options = new SplitOptions
178179
{
@@ -183,16 +184,17 @@ public void SplitOptions_SerializesCorrectly()
183184

184185
var httpContents = options.ToHttpContent().ToList();
185186

186-
httpContents.FirstOrDefault(c =>
187-
c.Headers.ContentDisposition?.Name == "splitMode")!
188-
.ReadAsStringAsync().Result.Should().Be("intervals");
189-
httpContents.FirstOrDefault(c =>
190-
c.Headers.ContentDisposition?.Name == "splitSpan")!
191-
.ReadAsStringAsync().Result.Should().Be("2");
187+
var modeContent = httpContents.FirstOrDefault(c =>
188+
c.Headers.ContentDisposition?.Name == "splitMode")!;
189+
(await modeContent.ReadAsStringAsync()).Should().Be("intervals");
190+
191+
var spanContent = httpContents.FirstOrDefault(c =>
192+
c.Headers.ContentDisposition?.Name == "splitSpan")!;
193+
(await spanContent.ReadAsStringAsync()).Should().Be("2");
192194
}
193195

194196
[Test]
195-
public void WatermarkOptions_SerializesCorrectly()
197+
public async Task WatermarkOptions_SerializesCorrectly()
196198
{
197199
var options = new WatermarkOptions
198200
{
@@ -202,16 +204,17 @@ public void WatermarkOptions_SerializesCorrectly()
202204

203205
var httpContents = options.ToHttpContent().ToList();
204206

205-
httpContents.FirstOrDefault(c =>
206-
c.Headers.ContentDisposition?.Name == "watermarkSource")!
207-
.ReadAsStringAsync().Result.Should().Be("text");
208-
httpContents.FirstOrDefault(c =>
209-
c.Headers.ContentDisposition?.Name == "watermarkExpression")!
210-
.ReadAsStringAsync().Result.Should().Be("DRAFT");
207+
var sourceContent = httpContents.FirstOrDefault(c =>
208+
c.Headers.ContentDisposition?.Name == "watermarkSource")!;
209+
(await sourceContent.ReadAsStringAsync()).Should().Be("text");
210+
211+
var exprContent = httpContents.FirstOrDefault(c =>
212+
c.Headers.ContentDisposition?.Name == "watermarkExpression")!;
213+
(await exprContent.ReadAsStringAsync()).Should().Be("DRAFT");
211214
}
212215

213216
[Test]
214-
public void StampOptions_SerializesCorrectly()
217+
public async Task StampOptions_SerializesCorrectly()
215218
{
216219
var options = new StampOptions
217220
{
@@ -222,21 +225,24 @@ public void StampOptions_SerializesCorrectly()
222225

223226
var httpContents = options.ToHttpContent().ToList();
224227

225-
httpContents.FirstOrDefault(c =>
226-
c.Headers.ContentDisposition?.Name == "stampSource")!
227-
.ReadAsStringAsync().Result.Should().Be("image");
228-
httpContents.FirstOrDefault(c =>
229-
c.Headers.ContentDisposition?.Name == "stampExpression")!
230-
.ReadAsStringAsync().Result.Should().Be("logo.png");
231-
httpContents.FirstOrDefault(c =>
232-
c.Headers.ContentDisposition?.Name == "stampPages")!
233-
.ReadAsStringAsync().Result.Should().Be("1");
228+
var sourceContent = httpContents.FirstOrDefault(c =>
229+
c.Headers.ContentDisposition?.Name == "stampSource")!;
230+
(await sourceContent.ReadAsStringAsync()).Should().Be("image");
231+
232+
var exprContent = httpContents.FirstOrDefault(c =>
233+
c.Headers.ContentDisposition?.Name == "stampExpression")!;
234+
(await exprContent.ReadAsStringAsync()).Should().Be("logo.png");
235+
236+
var pagesContent = httpContents.FirstOrDefault(c =>
237+
c.Headers.ContentDisposition?.Name == "stampPages")!;
238+
(await pagesContent.ReadAsStringAsync()).Should().Be("1");
234239
}
235240

236241
#endregion
237242

238243
#region Integration Tests
239244

245+
[Category("Integration")]
240246
[Test]
241247
public async Task HtmlToPdf_WithRotation_Succeeds()
242248
{
@@ -253,6 +259,7 @@ public async Task HtmlToPdf_WithRotation_Succeeds()
253259
result.Length.Should().BeGreaterThan(0);
254260
}
255261

262+
[Category("Integration")]
256263
[Test]
257264
public async Task HtmlToPdf_WithWatermark_Succeeds()
258265
{

0 commit comments

Comments
 (0)