(feat): Allow to copy tar stream to container before it starts. #1619
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds tar-archive-based resource copying: new TarArchiveMapping record, configuration and builder APIs to register tar streams, client/container methods to copy tar streams into containers, stream-parameter changes for extraction, and new tests and test infra. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder
participant TestcontainersClient
participant DockerContainerOperations
participant DockerEngine
participant Container
Builder->>TestcontainersClient: Provide ContainerConfiguration with TarArchiveMappings
TestcontainersClient->>DockerContainerOperations: CopyTarArchiveAsync(id, tarArchive, containerPath)
DockerContainerOperations->>DockerEngine: API: CopyToContainer (streamed TAR)
DockerEngine-->>DockerContainerOperations: Response
DockerContainerOperations-->>TestcontainersClient: Complete
TestcontainersClient-->>Container: Tar contents available at containerPath
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
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
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Testcontainers/Clients/DockerContainerOperations.cs (1)
112-121: Add validation for seekable stream before accessingStream.Length.Line 114 accesses
tarStream.Lengthwithout checking if the stream supports seeking. For non-seekable streams, this throwsNotSupportedException. SinceCopyTarArchiveAsyncaccepts genericStreamfrom callers, and Docker.DotNet requires seekable streams anyway, add a guard to validate the stream is seekable or provide clear documentation that the stream must support seeking.
🤖 Fix all issues with AI agents
In @src/Testcontainers/Clients/TestcontainersClient.cs:
- Around line 363-367: CopyTarArchiveAsync can pass a stream whose Position is
not at 0 into ExtractArchiveToContainerAsync, causing failed or partial
extractions; align behavior with CopyAsync(DirectoryInfo)/CopyAsync(FileInfo) by
seeking to the start on seekable streams before calling
ExtractArchiveToContainerAsync. Locate CopyTarArchiveAsync and, before passing
tarArchive.TarArchive to ExtractArchiveToContainerAsync, check
tarArchive.TarArchive.CanSeek and call Seek(0, SeekOrigin.Begin) (or otherwise
ensure the stream is reset), so reused or partially-read streams are correctly
extracted; alternatively, if you prefer not to modify the stream, add clear
documentation on CopyTarArchiveAsync/TarArchiveMapping requiring streams to be
positioned at the beginning.
In @tests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs:
- Around line 25-28: The test builds a Linux path incorrectly:
targetDirectoryPath1 currently contains a trailing slash ("/tmp/") and then uses
Path.Combine (OS-specific) to append the file, which can produce wrong
separators on Windows. Change targetDirectoryPath1 to "/tmp" (no trailing slash)
and construct targetFilePaths using Unix separators explicitly (e.g., string
concatenation or interpolation with '/' between targetDirectoryPath1 and
_testFile.Name) instead of Path.Combine so the produced path is always
"/tmp/<filename>" on Linux containers.
- Around line 61-64: The test currently builds container paths using
Path.Combine which uses OS-specific separators; update the container path
construction so it uses Unix-style forward slashes instead. Replace the
Path.Combine usage when building targetFilePaths (and/or the
targetDirectoryPath1 assignment) with a POSIX join like string.Join("/",
string.Empty, "tmp", _testFile.Name) or build targetDirectoryPath1 with
string.Join("/", string.Empty, "tmp", string.Empty) and then append
_testFile.Name using "/" so targetFilePaths.Add uses a Unix-style path; adjust
references to targetDirectoryPath1 and targetFilePaths accordingly.
- Around line 37-55: The test leaks resources because the created Container
(variable container from new ContainerBuilder(...).Build()) isn't disposed if
the test fails and bufferFilePath cleanup is only conditional at the end; change
the pattern to await using for the container (use await using var container =
... or await using (var container = ...) so the container is disposed even on
failure) and move the bufferFilePath deletion into a finally block (or use a
test fixture/teardown) so the temporary file is always removed; reference the
ContainerBuilder/WithCopyTarArchive-built container and the bufferFilePath
cleanup when making these changes.
🧹 Nitpick comments (7)
src/Testcontainers/Configurations/Containers/TarArchiveMapping.cs (1)
5-16: Consider adding parameter validation.The record accepts
nullvalues for bothtarArchiveandcontainerPathwithout validation, which could lead toNullReferenceExceptionorArgumentNullExceptiondeeper in the call stack (e.g., duringExtractArchiveToContainerAsync). Adding guard clauses would provide clearer error messages at the point of misconfiguration.🔧 Suggested validation
public TarArchiveMapping(Stream tarArchive, string containerPath) { + ArgumentNullException.ThrowIfNull(tarArchive); + ArgumentNullException.ThrowIfNull(containerPath); TarArchive = tarArchive; ContainerPath = containerPath; }Note:
ArgumentNullException.ThrowIfNullrequires .NET 6+. For NETSTANDARD2_0 compatibility, use traditional null checks.tests/Testcontainers.Platform.Linux.Tests/FileTestBase.cs (2)
13-14: Document the test file content.The magic number
13(carriage return character) is written to the test file without explanation. Consider using a named constant or adding a comment to clarify the intent.+// Write a single byte to create a non-empty test file using var fileStream = _testFile.Open(FileMode.Create, FileAccess.Write, FileShare.ReadWrite); fileStream.WriteByte(13);
28-31: Consider defensive cleanup in disposal.
DisposeManagedResourcesmay throw if the directory was already deleted or is locked by another process. In test teardown, exceptions during cleanup can mask test failures.🔧 Suggested defensive cleanup
protected virtual void DisposeManagedResources() { - _testFile.Directory!.Delete(true); + try + { + if (_testFile.Directory!.Exists) + { + _testFile.Directory.Delete(true); + } + } + catch + { + // Ignore cleanup failures in tests + } }src/Testcontainers/Builders/IContainerBuilder`2.cs (1)
326-332: Missing[PublicAPI]attribute for consistency.Other interface methods are decorated with
[PublicAPI]from JetBrains.Annotations for API surface documentation. This new method should follow the same pattern.🔧 Suggested fix
/// <summary> /// Copies a tar archive contents to the container before it starts. /// </summary> /// <param name="tarArchive">The <see cref="Stream"/> with the tar archive contents.</param> /// <param name="containerPath">The path where tar archive contents should be placed.</param> /// <returns>A configured instance of <typeparamref name="TBuilderEntity" />.</returns> + [PublicAPI] TBuilderEntity WithCopyTarArchive(Stream tarArchive, string containerPath = "/");tests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs (3)
70-73: Consider addingawait usingfor container disposal consistency.While not strictly necessary if the test passes, wrapping the container with
await usingensures proper cleanup on failure, matching the pattern in other test files likeTarOutputMemoryStreamTest.cs.
136-139: Consider addingawait usingfor container disposal.Same recommendation as other tests in this file.
12-13: Missing[Trait]attribute for platform specification.Other tests in this project use
[Trait(nameof(DockerCli.DockerPlatform), nameof(DockerCli.DockerPlatform.Linux))]to indicate Linux platform requirements. These tests should follow the same pattern for consistency with existing test infrastructure.Also applies to: 57-58, 86-87
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/Testcontainers/Builders/ContainerBuilder3.cs`src/Testcontainers/Builders/IContainerBuilder2.cs`src/Testcontainers/Clients/DockerContainerOperations.cssrc/Testcontainers/Clients/IDockerContainerOperations.cssrc/Testcontainers/Clients/ITestcontainersClient.cssrc/Testcontainers/Clients/TestcontainersClient.cssrc/Testcontainers/Configurations/Containers/ContainerConfiguration.cssrc/Testcontainers/Configurations/Containers/IContainerConfiguration.cssrc/Testcontainers/Configurations/Containers/TarArchiveMapping.cssrc/Testcontainers/Containers/DockerContainer.cssrc/Testcontainers/Containers/IContainer.cstests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cstests/Testcontainers.Platform.Linux.Tests/FileTestBase.cstests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-17T17:58:43.958Z
Learnt from: diegosasw
Repo: testcontainers/testcontainers-dotnet PR: 1583
File: src/Testcontainers.KurrentDb/Testcontainers.KurrentDb.csproj:7-7
Timestamp: 2025-11-17T17:58:43.958Z
Learning: In the testcontainers-dotnet repository, JetBrains.Annotations should use version 2023.3.0 to maintain consistency with the main Testcontainers csproj, rather than always using the latest available version.
Applied to files:
src/Testcontainers/Configurations/Containers/IContainerConfiguration.cssrc/Testcontainers/Clients/DockerContainerOperations.cssrc/Testcontainers/Clients/IDockerContainerOperations.cssrc/Testcontainers/Configurations/Containers/ContainerConfiguration.cssrc/Testcontainers/Builders/ContainerBuilder3.cs`
📚 Learning: 2026-01-02T11:38:24.873Z
Learnt from: HofmeisterAn
Repo: testcontainers/testcontainers-dotnet PR: 1616
File: examples/WeatherForecast/src/WeatherForecast/DatabaseContainer.cs:5-5
Timestamp: 2026-01-02T11:38:24.873Z
Learning: In Testcontainers for .NET 4.10.0 and later, the recommended way to configure PostgreSqlBuilder is to use the parameterized constructor that accepts the image string directly: `new PostgreSqlBuilder("postgres:15.1")` rather than the older pattern of `new PostgreSqlBuilder().WithImage("postgres:15.1")`.
Applied to files:
src/Testcontainers/Builders/ContainerBuilder3.cs`
🧬 Code graph analysis (8)
src/Testcontainers/Configurations/Containers/IContainerConfiguration.cs (1)
src/Testcontainers/Configurations/Containers/TarArchiveMapping.cs (1)
TarArchiveMapping(7-11)
src/Testcontainers/Clients/DockerContainerOperations.cs (1)
src/Testcontainers.Xunit/Logger.cs (1)
Logger(3-20)
src/Testcontainers/Clients/TestcontainersClient.cs (1)
src/Testcontainers/Containers/DockerContainer.cs (16)
Task(312-315)Task(318-321)Task(324-344)Task(347-354)Task(357-364)Task(367-374)Task(377-380)Task(383-395)Task(398-401)Task(404-407)Task(410-413)Task(416-419)Task(422-425)Task(480-532)Task(538-555)Task(565-601)
tests/Testcontainers.Platform.Linux.Tests/FileTestBase.cs (3)
tests/Testcontainers.Commons/TestSession.cs (1)
TestSession(10-13)tests/Testcontainers.Commons/DockerCli.cs (1)
ToString(121-124)tests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs (1)
DisposeManagedResources(13-18)
src/Testcontainers/Containers/DockerContainer.cs (5)
src/Testcontainers/Images/FutureDockerImage.cs (1)
Exists(137-140)src/Testcontainers/Resource.cs (1)
Exists(37-37)src/Testcontainers/Networks/DockerNetwork.cs (1)
Exists(82-85)src/Testcontainers/Volumes/DockerVolume.cs (1)
Exists(82-85)src/Testcontainers/Clients/TestcontainersClient.cs (16)
Task(96-99)Task(102-121)Task(124-132)Task(135-143)Task(146-154)Task(157-165)Task(168-188)Task(191-194)Task(197-200)Task(203-232)Task(235-248)Task(251-264)Task(267-271)Task(274-314)Task(317-370)Task(373-416)
tests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs (2)
tests/Testcontainers.Platform.Linux.Tests/FileTestBase.cs (6)
FileTestBase(3-47)FileTestBase(9-15)FileTestBase(17-20)DisposeManagedResources(28-31)Dispose(22-26)Dispose(33-46)src/Testcontainers/Containers/TarOutputMemoryStream.cs (3)
TarOutputMemoryStream(15-172)TarOutputMemoryStream(28-32)TarOutputMemoryStream(38-43)
tests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs (2)
tests/Testcontainers.Platform.Linux.Tests/FileTestBase.cs (3)
FileTestBase(3-47)FileTestBase(9-15)FileTestBase(17-20)tests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs (3)
Fact(20-37)Fact(99-153)Fact(155-194)
src/Testcontainers/Builders/ContainerBuilder`3.cs (2)
src/Testcontainers/Configurations/Containers/TarArchiveMapping.cs (1)
TarArchiveMapping(7-11)src/Testcontainers/Configurations/Containers/ContainerConfiguration.cs (4)
ContainerConfiguration(47-97)ContainerConfiguration(103-106)ContainerConfiguration(112-115)ContainerConfiguration(122-149)
⏰ 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). (7)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: analyze (csharp)
🔇 Additional comments (12)
src/Testcontainers/Configurations/Containers/IContainerConfiguration.cs (2)
91-95: LGTM!The
TarArchiveMappingsproperty follows the established pattern ofResourceMappings(line 89) and is appropriately documented. The use ofIEnumerable<TarArchiveMapping>is consistent with other collection properties in this interface.
4-13: Remove unusedSystem.IOusing directive.The added
usingdirective forSystem.IO(line 11) is not used in this interface. However,System,System.Threading, andSystem.Threading.Tasksare required—SystemforFunc<>,System.ThreadingforCancellationToken, andSystem.Threading.TasksforTaskused in theStartupCallbackproperty signature.Likely an incorrect or invalid review comment.
src/Testcontainers/Clients/ITestcontainersClient.cs (1)
158-167: LGTM!The new
CopyTarArchiveAsyncmethod is well-documented and follows the established patterns of other copy methods in this interface. The defaultcontainerPath = "/"is appropriate since Docker's tar extraction operates relative to this path.src/Testcontainers/Containers/IContainer.cs (1)
303-311: LGTM!The new
CopyTarArchiveAsyncmethod extends the publicIContainerinterface appropriately. The signature omits the container ID (unlikeITestcontainersClient) sinceIContainerrepresents a specific container instance. Documentation is complete and consistent with otherCopyAsyncoverloads.src/Testcontainers/Clients/TestcontainersClient.cs (1)
266-271: LGTM!The implementation correctly delegates to
Container.ExtractArchiveToContainerAsync. The method is appropriately async and follows the established pattern of other copy methods.src/Testcontainers/Containers/DockerContainer.cs (2)
155-176: LGTM - Improved Hostname robustness.The refactored
npipe/unixcase now properly guards against accessing_container.NetworkSettingswhen the container hasn't been created yet. The early return oflocalhostwhen!Exists()or!_client.IsRunningInsideDockerpreventsNullReferenceExceptionand provides sensible defaults.
409-413: LGTM!The
CopyTarArchiveAsyncimplementation correctly delegates to the underlying client. The use ofIdproperty ensuresThrowIfResourceNotFound()is called, maintaining consistency with other copy methods that require the container to exist.src/Testcontainers/Builders/ContainerBuilder`3.cs (1)
275-280: LGTM!The implementation follows the established fluent builder pattern used by other
WithResourceMappingmethods. The defaultcontainerPathof"/"is a sensible default for extracting tar archives at the root.src/Testcontainers/Clients/IDockerContainerOperations.cs (1)
28-28: LGTM!The interface signature update to accept
Streaminstead ofTarOutputMemoryStreamenables more flexible stream sources, directly supporting the PR objective of avoiding the 4GBMemoryStreamlimit.tests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs (1)
13-18: LGTM!The disposal pattern correctly overrides
DisposeManagedResources(), calls the base implementation first, then disposes the_tarOutputMemoryStream. This properly integrates with theFileTestBaselifecycle management.src/Testcontainers/Configurations/Containers/ContainerConfiguration.cs (2)
46-96: LGTM!The
TarArchiveMappingsparameter and property follow the established pattern used by other configuration properties likeResourceMappings. The[JsonIgnore]attribute is appropriate sinceStreamobjects cannot be serialized.
137-137: LGTM!The merge logic correctly uses
BuildConfiguration.Combineto concatenate tar archive mappings from old and new configurations, consistent with howResourceMappingsand other collections are handled.
tests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/Testcontainers/Builders/IContainerBuilder`2.cs:
- Around line 326-335: The XML docs for IContainerBuilder<TBuilderEntity,
TContainer> method WithCopyTarArchive(Stream tarArchive, string containerPath =
"/") lack ownership/disposal and timing semantics for the provided Stream;
update the method's XML comments (summary/remarks) to state clearly whether the
caller retains ownership or the builder takes ownership, whether the stream must
remain open until container startup or is consumed immediately, and whether the
stream may be reused after the call (e.g., "The caller retains ownership and
must keep the stream open until container startup, and must dispose it
afterwards" OR "The builder takes ownership and will dispose the stream; do not
reuse or dispose it"); mention synchronous vs asynchronous consumption if
applicable and include explicit guidance about setting Stream.Position before
use.
- Line 335: The new interface method WithCopyTarArchive(Stream tarArchive,
string containerPath = "/") is missing the JetBrains.Annotations [PublicAPI]
attribute; add the [PublicAPI] attribute above the method declaration in
IContainerBuilder`2 so it matches the other methods and imports or references
JetBrains.Annotations if not already present.
🧹 Nitpick comments (2)
src/Testcontainers/Containers/IContainer.cs (1)
303-313: Clarify stream ownership and capability requirements in documentation.The XML documentation should specify:
- Ownership: Whether the caller is responsible for disposing
tarArchiveafter the method completes (following typical .NET conventions, the caller should retain ownership, but explicit documentation prevents ambiguity).- Stream capabilities: Whether
tarArchivemust be seekable or if it only needs to be readable, as this affects which stream types callers can use.These clarifications help prevent resource leaks and incorrect stream usage.
📝 Suggested documentation enhancement
/// <summary> /// Copies a tar archive contents to the container. /// </summary> /// <remarks> -/// Set the <paramref name="tarArchive"/> property <see cref="Stream.Position"/> to 0 before calling this method. +/// Set the <paramref name="tarArchive"/> property <see cref="Stream.Position"/> to 0 before calling this method. +/// The caller retains ownership of <paramref name="tarArchive"/> and is responsible for disposing it. +/// The stream must be readable; seekability is not required. /// </remarks> /// <param name="tarArchive">The <see cref="Stream"/> with the tar archive contents.</param> /// <param name="containerPath">The path where tar archive contents should be placed.</param> /// <param name="ct">Cancellation token.</param> /// <returns>A task that completes when the tar archive has been copied.</returns> Task CopyTarArchiveAsync(Stream tarArchive, string containerPath = "/", CancellationToken ct = default);src/Testcontainers/Builders/IContainerBuilder`2.cs (1)
330-330: Consider handling Stream.Position internally instead of requiring caller action.Requiring callers to manually set
Stream.Positionto 0 is error-prone and creates fragile API usage. The implementation could reset the position automatically if the stream is seekable, or document that non-seekable streams are acceptable.If the stream must be seekable for this operation, consider either:
- Automatically resetting the position in the implementation:
if (tarArchive.CanSeek) tarArchive.Position = 0;- Updating the remarks to state that non-seekable streams are acceptable and will be read from the current position
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Testcontainers/Builders/IContainerBuilder2.cs`src/Testcontainers/Clients/ITestcontainersClient.cssrc/Testcontainers/Containers/IContainer.cstests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Testcontainers/Clients/ITestcontainersClient.cs
- tests/Testcontainers.Platform.Linux.Tests/CopyTarArchiveTests.cs
⏰ 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). (7)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: analyze (csharp)
🔇 Additional comments (1)
src/Testcontainers/Builders/IContainerBuilder`2.cs (1)
326-335: No duplicate method declarations exist.Verification of the interface file confirms only one
WithCopyTarArchivemethod declaration is present. The AI-generated summary indicating duplicate declarations was inaccurate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Testcontainers/Clients/ITestcontainersClient.cs (1)
158-175: Consider validating stream position in the implementation.The documentation correctly warns that
tarArchive.Positionshould be 0, but the implementation does not validate this requirement. Other methods in the same file (e.g.,CopyFileToContainerAsync) explicitly callSeek(0, SeekOrigin.Begin)before callingExtractArchiveToContainerAsync. If callers forget to reset the position, the copy operation may be incomplete or fail silently.Consider adding a runtime position check or explicitly resetting the stream position to 0 in the
CopyTarArchiveAsyncimplementation to align with the documented requirement and match the pattern used elsewhere in the codebase.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Testcontainers/Builders/IContainerBuilder2.cs`src/Testcontainers/Clients/ITestcontainersClient.cssrc/Testcontainers/Containers/IContainer.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Testcontainers/Containers/IContainer.cs
⏰ 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). (7)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: analyze (csharp)
🔇 Additional comments (2)
src/Testcontainers/Builders/IContainerBuilder`2.cs (2)
37-37: LGTM: PublicAPI attribute additions improve API surface consistency.The addition of
[PublicAPI]attributes to these existing methods improves API documentation consistency across the builder interface.Also applies to: 325-325
328-344: Stream position and lifetime requirements match client interface.The documentation correctly mirrors the stream handling requirements from
ITestcontainersClient.CopyTarArchiveAsync. The same consideration about runtime validation of stream position applies here (see comment on ITestcontainersClient.cs).
1 participant
What does this PR do?
This PR adds a new method to
ContainerBuilderwhich allows to copy aStreamcontaining tar archive into container, leveraging already available methodIDockerContainerOperations.ExtractArchiveToContainerAsync. I changedIDockerContainerOperations.ExtractArchiveToContainerAsyncsignature to match underlyingDocker.DotnetAPI which takes a baseStreaminstead ofTarOutputmemoryStream.The catch is that the user must build tar stream on it's own.
At first I was thinking about adding some class/methods to allow users build tar archive stream using TC library, but then I decided not to because:
SharpZipLiblibrary in TC internals, but it looks like stopped being maintained. Probably not a good idea to create more functionality on top of it. Not using this library further may also allow us to drop it as a dependency later.SharpZipLibAPI is not that trivial to build custom functionality on top of.SharpZipLiblibrary and built-inSystem.IO(starting from NET 7). We may add this tests as a documentation to show users some guidance.Should close #1180
Also may help in cases like this: #1486
Why is it important?
This change will allow uses to add arbitrary files and directories to container. The existing API
WithResourceMappingcurrently have some limitations:targetis directory path, other assume it's file path).Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.