feat: Improve image build failure messages#1700
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughTraceProgress now accumulates Docker JSON errors; ImageBuildFailedException formats image+error details; DockerImageOperations creates local TraceProgress instances and throws ImageBuildFailedException with collected errors on build failure. Tests and a failing Dockerfile verify the new error message content. ChangesDocker Build Failure Error Reporting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (2)
tests/Testcontainers.Tests/Unit/Images/ImageFromDockerfileTest.cs (1)
231-247: ⚡ Quick winTest assumes specific Docker error message format.
The assertion on line 246 expects the exact error message
"The command '/bin/sh -c command-that-does-not-exist' returned a non-zero code: 127". While this validates the error collection mechanism, it couples the test to Docker's error message format, which could change across Docker versions or container runtimes (e.g., Podman).Consider whether a more flexible assertion (e.g.,
Contains("command-that-does-not-exist")andContains("127")) would provide sufficient validation while being more resilient to Docker engine variations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Testcontainers.Tests/Unit/Images/ImageFromDockerfileTest.cs` around lines 231 - 247, The test BuildFailureIncludesDetailedErrorMessage in ImageFromDockerfileTest is coupled to a specific Docker error string; update the assertions to be resilient by checking presence of the failing command and exit code rather than the full Docker message — e.g., assert that exception.Message Contains "command-that-does-not-exist" and Contains "127" (keep the existing Assert.StartsWith/Contains for the general framing), referencing the ImageFromDockerfileBuilder.CreateAsync invocation and the ImageBuildFailedException to locate the test.src/Testcontainers/Images/ImageBuildFailedException.cs (1)
26-43: 💤 Low valueConsider removing trailing space in error section label.
Line 38 includes a trailing space in
" Error: "before the newline. While not a functional issue, removing it would improve consistency.♻️ Proposed formatting adjustment
- exceptionInfo.AppendLine(" Error: "); + exceptionInfo.AppendLine(" Error:");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Testcontainers/Images/ImageBuildFailedException.cs` around lines 26 - 43, The CreateMessage method in ImageBuildFailedException.cs appends an error section label with a trailing space (" Error: "); remove the trailing space so the label is " Error:" to keep formatting consistent; update the call to exceptionInfo.AppendLine in CreateMessage (the line that currently uses " Error: ") to use the trimmed label and run tests/formatting to ensure no other spacing expectations rely on the extra space.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Testcontainers/Images/ImageBuildFailedException.cs`:
- Around line 26-43: The CreateMessage method in ImageBuildFailedException.cs
appends an error section label with a trailing space (" Error: "); remove the
trailing space so the label is " Error:" to keep formatting consistent; update
the call to exceptionInfo.AppendLine in CreateMessage (the line that currently
uses " Error: ") to use the trimmed label and run tests/formatting to ensure no
other spacing expectations rely on the extra space.
In `@tests/Testcontainers.Tests/Unit/Images/ImageFromDockerfileTest.cs`:
- Around line 231-247: The test BuildFailureIncludesDetailedErrorMessage in
ImageFromDockerfileTest is coupled to a specific Docker error string; update the
assertions to be resilient by checking presence of the failing command and exit
code rather than the full Docker message — e.g., assert that exception.Message
Contains "command-that-does-not-exist" and Contains "127" (keep the existing
Assert.StartsWith/Contains for the general framing), referencing the
ImageFromDockerfileBuilder.CreateAsync invocation and the
ImageBuildFailedException to locate the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 31b8c739-f38b-4077-ae46-997bf1c1da8d
📒 Files selected for processing (5)
src/Testcontainers/Clients/DockerImageOperations.cssrc/Testcontainers/Clients/TraceProgress.cssrc/Testcontainers/Images/ImageBuildFailedException.cstests/Testcontainers.Tests/Assets/buildFailure/Dockerfiletests/Testcontainers.Tests/Unit/Images/ImageFromDockerfileTest.cs
1 participant
What does this PR do?
This PR adds additional error details when an image build fails.
Previously, the error message was very generic and did not provide any information about the underlying cause of the failure. To diagnose image build issues, developers had to inspect the console output, which is not always obvious. In addition, .NET console messages can be difficult to locate, especially for a library that is primarily executed in test environments.
With this change, the image build error details are included directly in the exception. A failed image build will now throw an
ImageBuildFailedExceptioncontaining the relevant error information, making failures easier to diagnose and troubleshoot.Why is it important?
-
Related issues
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores