Conversation
Reviewer's GuideAdds standardized Testcontainers labels and HTTP headers across containers, networks, and the Docker client, plus tests to verify label and header behavior and a way to inject custom headers into DockerClientSwift. Sequence diagram for DockerClient requests with Testcontainers custom headerssequenceDiagram
actor TestCode
participant TestcontainersDockerClient
participant DockerClient
participant DockerDaemon
TestCode->>TestcontainersDockerClient: init(socketPath)
TestcontainersDockerClient->>TestcontainersDockerClient: testcontainersHeaders()
TestcontainersDockerClient->>DockerClient: init(daemonSocket, client, logger, customHeaders)
TestCode->>TestcontainersDockerClient: performDockerOperation()
TestcontainersDockerClient->>DockerClient: execute(endpoint, body)
DockerClient->>DockerClient: buildHeaders()
DockerClient->>DockerDaemon: HTTP request with base headers + customHeaders
DockerDaemon-->>DockerClient: HTTP response
DockerClient-->>TestcontainersDockerClient: decoded response
TestcontainersDockerClient-->>TestCode: result
Sequence diagram for applying standard Testcontainers labels to containers and networkssequenceDiagram
actor TestCode
participant ContainerBuilder
participant NetworkBuilder
participant TestcontainersLabels
participant DockerClient
participant DockerDaemon
TestCode->>ContainerBuilder: withLabel(key, value)
TestCode->>ContainerBuilder: buildAndStart()
ContainerBuilder->>ContainerBuilder: mergedLabels = labels
ContainerBuilder->>TestcontainersLabels: addDefaultLabels(to: mergedLabels)
TestcontainersLabels-->>ContainerBuilder: mergedLabels with defaults
ContainerBuilder->>DockerClient: createContainer(labels: mergedLabels)
DockerClient->>DockerDaemon: create container request
DockerDaemon-->>DockerClient: container created
TestCode->>NetworkBuilder: withLabel(key, value)
TestCode->>NetworkBuilder: build()
NetworkBuilder->>NetworkBuilder: mergedLabels = labels
NetworkBuilder->>TestcontainersLabels: addDefaultLabels(to: mergedLabels)
TestcontainersLabels-->>NetworkBuilder: mergedLabels with defaults
NetworkBuilder->>DockerClient: createNetwork(name, driver)
DockerClient->>DockerDaemon: create network request
DockerDaemon-->>DockerClient: network id
Class diagram for Testcontainers labels and Docker client header integrationclassDiagram
class NetworkBuilder {
-String name
-String driver
-Bool Internal
-Dictionary~String,String~ labels
-DockerClient client
+NetworkBuilder(name String, client DockerClient)
+withName(name String) NetworkBuilder
+withDriver(driver String) NetworkBuilder
+withInternal(_ Internal Bool) NetworkBuilder
+withLabel(_ key String, _ value String) NetworkBuilder
+withLabel(_ labels Dictionary~String,String~) NetworkBuilder
+build() DockerNetworkImpl
}
class ContainerBuilder {
-String image
-TestcontainersDockerClient client
-String name
-Dictionary~String,String~ labels
-Dictionary~String,String~ environment
+ContainerBuilder(image String, client TestcontainersDockerClient)
+withName(name String) ContainerBuilder
+withLabel(_ key String, _ value String) ContainerBuilder
+withLabels(_ labels Dictionary~String,String~) ContainerBuilder
+withEnv(key String, value String) ContainerBuilder
+build() Container
+buildAndStart() Container
}
class TestcontainersLabels {
<<enum>>
+static String labelBase
+static String labelLang
+static String labelVersion
+static String labelSessionId
+static String lang
+static String version
+static String sessionId
+static defaultLabels Dictionary~String,String~
+static addDefaultLabels(to labels inout Dictionary~String,String~) void
}
class DockerClient {
-String daemonSocket
-HTTPClient client
-HTTPHeaders customHeaders
-Logger logger
+DockerClient(daemonSocket String, client HTTPClient, logger Logger, customHeaders HTTPHeaders)
+shutdown() void
+execute(endpoint Endpoint, body Encodable) T~Response~
+executeString(endpoint Endpoint, body Encodable) String
+buildHeaders() HTTPHeaders
}
class TestcontainersDockerClient {
-DockerClient client
-Logger logger
+TestcontainersDockerClient(socketPath String)
+createContainerBuilder(image String) ContainerBuilder
+createNetworkBuilder(name String) NetworkBuilder
+shutdown() void
+static testcontainersHeaders() HTTPHeaders
-static detectSocketPath() String
}
NetworkBuilder --> DockerClient : uses
ContainerBuilder --> TestcontainersDockerClient : uses
ContainerBuilder ..> TestcontainersLabels : merges defaultLabels
NetworkBuilder ..> TestcontainersLabels : merges defaultLabels
TestcontainersDockerClient --> DockerClient : wraps
TestcontainersDockerClient ..> TestcontainersLabels : testcontainersHeaders
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds NIOHTTP1 dependency; introduces PackageVersion and TestcontainersLabels with a process-wide sessionId; merges default labels into container and network builds; extends Docker clients to accept custom HTTP headers (x-tc-sid and User-Agent) and to serialize network labels; adds tests for labels and headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
NetworkBuilder.build()you constructmergedLabelsand add default labels, butcreateNetworkis still called without any labels, so those values are never applied to the Docker network; either pass the labels intocreateNetwork(and adjust its API if needed) or remove the unused merge. - The
TestcontainersLabels.versionvalue is hard-coded and can drift from the actual package version; consider deriving this from a single source of truth (e.g. a generated value or centralized constant) so it stays in sync across releases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NetworkBuilder.build()` you construct `mergedLabels` and add default labels, but `createNetwork` is still called without any labels, so those values are never applied to the Docker network; either pass the labels into `createNetwork` (and adjust its API if needed) or remove the unused merge.
- The `TestcontainersLabels.version` value is hard-coded and can drift from the actual package version; consider deriving this from a single source of truth (e.g. a generated value or centralized constant) so it stays in sync across releases.
## Individual Comments
### Comment 1
<location path="Sources/Testcontainers/Network.swift" line_range="110-113" />
<code_context>
+ /// Standard Testcontainers labels are automatically applied to the network.
/// - Returns: A Docker network instance.
/// - Throws: An error if network creation fails.
public func build() async throws -> DockerNetworkImpl {
+ var mergedLabels = labels
+ TestcontainersLabels.addDefaultLabels(to: &mergedLabels)
let networkId = try await client.createNetwork(name: name, driver: driver)
return DockerNetworkImpl(id: networkId, name: name, driver: driver, client: client)
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The merged labels are computed but never used when creating the network.
`mergedLabels` is built and default labels are added, but it’s never used in `client.createNetwork`, so neither custom nor default labels are applied to the network. If the Docker API supports labels, pass `mergedLabels` through to `createNetwork`; otherwise, remove `mergedLabels` and the label logic to avoid misleading behavior and comments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Tests/TestcontainersTests.swift (1)
659-668:⚠️ Potential issue | 🟡 MinorTest expectation doesn't account for default labels.
This test expects exactly 3 labels but
build()now merges 4 default Testcontainers labels. The assertion on line 664 will fail: expected 3, but will get 7 (3 user + 4 default).🐛 Proposed fix
func testBuilderWithLabelsDictionary() { let builder = ContainerBuilder("nginx:latest") .withLabel(["app": "web", "env": "test", "tier": "frontend"]) let container = builder.build() - XCTAssertEqual(container.labels.count, 3) + XCTAssertEqual(container.labels.count, 7) // 3 user + 4 default Testcontainers labels XCTAssertEqual(container.labels["app"], "web") XCTAssertEqual(container.labels["env"], "test") XCTAssertEqual(container.labels["tier"], "frontend") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TestcontainersTests.swift` around lines 659 - 668, Test expects exactly 3 labels but ContainerBuilder.build() now merges four default Testcontainers labels; update testBuilderWithLabelsDictionary to stop asserting an exact total count and instead assert the three user labels exist (via container.labels["app"], ["env"], ["tier"]) and either assert container.labels.count >= 3 or explicitly check for the expected default label keys if you know them; locate this logic in testBuilderWithLabelsDictionary, ContainerBuilder.withLabel, and the build()/container.labels usage and change the XCTAssertEqual(container.labels.count, 3) to a presence-based assertion as described.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Testcontainers/Network.swift`:
- Around line 110-114: The mergedLabels created in build() are never used;
either pass them into the Docker API call or remove the dead code. Update
build() so that the mergedLabels (after TestcontainersLabels.addDefaultLabels)
are supplied to the client.createNetwork call (e.g., add a labels parameter) and
modify the client.createNetwork signature and its callers to accept labels, then
ensure DockerNetworkImpl is constructed with those labels (add a labels
property/initializer) so labels set via withLabel() persist; alternatively, if
labels are not yet supported, remove mergedLabels and
TestcontainersLabels.addDefaultLabels from build() and avoid adding a labels
property to DockerNetworkImpl until API support is added.
---
Outside diff comments:
In `@Tests/TestcontainersTests.swift`:
- Around line 659-668: Test expects exactly 3 labels but
ContainerBuilder.build() now merges four default Testcontainers labels; update
testBuilderWithLabelsDictionary to stop asserting an exact total count and
instead assert the three user labels exist (via container.labels["app"],
["env"], ["tier"]) and either assert container.labels.count >= 3 or explicitly
check for the expected default label keys if you know them; locate this logic in
testBuilderWithLabelsDictionary, ContainerBuilder.withLabel, and the
build()/container.labels usage and change the
XCTAssertEqual(container.labels.count, 3) to a presence-based assertion as
described.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4897471e-c438-423e-9661-8e1252495a1d
📒 Files selected for processing (7)
Package.swiftSources/DockerClientSwift/APIs/DockerClient.swiftSources/Testcontainers/Container.swiftSources/Testcontainers/DockerClient.swiftSources/Testcontainers/Labels.swiftSources/Testcontainers/Network.swiftTests/TestcontainersTests.swift
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Sources/Testcontainers/DockerClient.swift (1)
88-91: Drop the trailing comma in the header literal to silence static analysis (Line 90).Not functional, just a lint cleanliness fix.
✂️ Tiny cleanup
HTTPHeaders([ ("x-tc-sid", TestcontainersLabels.sessionId), - ("User-Agent", "tc-swift/\(TestcontainersLabels.version)"), + ("User-Agent", "tc-swift/\(TestcontainersLabels.version)") ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Testcontainers/DockerClient.swift` around lines 88 - 91, Remove the trailing comma from the HTTPHeaders array literal to satisfy the linter: update the HTTPHeaders([...]) call that constructs headers (the tuple entries ("x-tc-sid", TestcontainersLabels.sessionId) and ("User-Agent", "tc-swift/\(TestcontainersLabels.version)") inside HTTPHeaders) so there is no trailing comma after the last tuple.Sources/Testcontainers/Labels.swift (1)
65-71: Remove the trailing comma indefaultLabelsto satisfy static analysis (Line 70).Small style fix, but it keeps Codacy noise down.
✂️ Tiny cleanup
[ labelBase: "true", labelLang: lang, labelVersion: version, - labelSessionId: sessionId, + labelSessionId: sessionId ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Testcontainers/Labels.swift` around lines 65 - 71, The dictionary literal in the computed property defaultLabels contains a trailing comma after the last entry (labelSessionId: sessionId); remove that trailing comma so the returned [String: String] literal is formatted without a dangling comma. Update the body of defaultLabels (referencing labelBase, labelLang, labelVersion, labelSessionId and sessionId) to end the final line without a comma to satisfy static analysis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Testcontainers/DockerClient.swift`:
- Around line 88-91: Remove the trailing comma from the HTTPHeaders array
literal to satisfy the linter: update the HTTPHeaders([...]) call that
constructs headers (the tuple entries ("x-tc-sid",
TestcontainersLabels.sessionId) and ("User-Agent",
"tc-swift/\(TestcontainersLabels.version)") inside HTTPHeaders) so there is no
trailing comma after the last tuple.
In `@Sources/Testcontainers/Labels.swift`:
- Around line 65-71: The dictionary literal in the computed property
defaultLabels contains a trailing comma after the last entry (labelSessionId:
sessionId); remove that trailing comma so the returned [String: String] literal
is formatted without a dangling comma. Update the body of defaultLabels
(referencing labelBase, labelLang, labelVersion, labelSessionId and sessionId)
to end the final line without a comma to satisfy static analysis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b60a251-269f-433a-8d5e-4d2c048fb656
📒 Files selected for processing (5)
Sources/Testcontainers/DockerClient.swiftSources/Testcontainers/Labels.swiftSources/Testcontainers/Network.swiftSources/Testcontainers/RawEndpoints.swiftSources/Testcontainers/Version.swift
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Tests/TestcontainersTests.swift`:
- Around line 807-810: Remove the trailing commas from collection literals
flagged by static analysis in TestcontainersTests.swift: locate the dictionary
assigned to the variable labels (the var labels: [String: String] = [...] block)
and any other collection literals around the same test (the occurrences near the
second block around lines 919-922) and delete the extra comma after the last
element so the literals end without a trailing comma; update the labels
assignment and the other identified array/dictionary literals accordingly to
satisfy the linter.
- Around line 724-866: Update the new test methods in the
TestcontainersLabelsTests class to use the repository test convention by
changing their signatures from synchronous to async throws; e.g., modify
functions such as testLabelBaseValue, testLabelLangValue, testLabelVersionValue,
testLabelSessionIdValue, testLangIsSwift, testVersionIsNotEmpty,
testSessionIdIsNonEmpty, testSessionIdIsValidUUID,
testSessionIdIsStableAcrossCalls, testDefaultLabelsContainsAllFourKeys,
testDefaultLabelsValues, testAddDefaultLabelsToEmptyDictionary,
testAddDefaultLabelsPreservesUserLabels,
testAddDefaultLabelsDoesNotOverwriteUserLabels,
testBuilderBuildIncludesStandardLabels,
testBuilderBuildWithUserLabelsIncludesBoth,
testBuilderBuildUserLabelsNotOverwritten, and
testAllContainersShareSameSessionId to have signatures like `func <name>() async
throws`; no behavioral changes needed besides updating the signature (and add
awaits where required by async calls).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf594971-698a-442e-be2a-7787e86aea80
📒 Files selected for processing (1)
Tests/TestcontainersTests.swift
Comment on lines
+724
to
+866
| final class TestcontainersLabelsTests: XCTestCase { | ||
| // MARK: - Label Constants | ||
|
|
||
| func testLabelBaseValue() { | ||
| XCTAssertEqual(TestcontainersLabels.labelBase, "org.testcontainers") | ||
| } | ||
|
|
||
| func testLabelLangValue() { | ||
| XCTAssertEqual(TestcontainersLabels.labelLang, "org.testcontainers.lang") | ||
| } | ||
|
|
||
| func testLabelVersionValue() { | ||
| XCTAssertEqual(TestcontainersLabels.labelVersion, "org.testcontainers.version") | ||
| } | ||
|
|
||
| func testLabelSessionIdValue() { | ||
| XCTAssertEqual(TestcontainersLabels.labelSessionId, "org.testcontainers.sessionId") | ||
| } | ||
|
|
||
| func testLangIsSwift() { | ||
| XCTAssertEqual(TestcontainersLabels.lang, "swift") | ||
| } | ||
|
|
||
| func testVersionIsNotEmpty() { | ||
| XCTAssertFalse(TestcontainersLabels.version.isEmpty) | ||
| } | ||
|
|
||
| // MARK: - Session ID | ||
|
|
||
| func testSessionIdIsNonEmpty() { | ||
| XCTAssertFalse(TestcontainersLabels.sessionId.isEmpty) | ||
| } | ||
|
|
||
| func testSessionIdIsValidUUID() { | ||
| // Session ID should be a valid UUID string | ||
| XCTAssertNotNil(UUID(uuidString: TestcontainersLabels.sessionId)) | ||
| } | ||
|
|
||
| func testSessionIdIsStableAcrossCalls() { | ||
| let firstCall = TestcontainersLabels.sessionId | ||
| let secondCall = TestcontainersLabels.sessionId | ||
| XCTAssertEqual(firstCall, secondCall, "Session ID should be stable within the same process") | ||
| } | ||
|
|
||
| // MARK: - Default Labels | ||
|
|
||
| func testDefaultLabelsContainsAllFourKeys() { | ||
| let labels = TestcontainersLabels.defaultLabels | ||
| XCTAssertEqual(labels.count, 4) | ||
| XCTAssertNotNil(labels[TestcontainersLabels.labelBase]) | ||
| XCTAssertNotNil(labels[TestcontainersLabels.labelLang]) | ||
| XCTAssertNotNil(labels[TestcontainersLabels.labelVersion]) | ||
| XCTAssertNotNil(labels[TestcontainersLabels.labelSessionId]) | ||
| } | ||
|
|
||
| func testDefaultLabelsValues() { | ||
| let labels = TestcontainersLabels.defaultLabels | ||
| XCTAssertEqual(labels[TestcontainersLabels.labelBase], "true") | ||
| XCTAssertEqual(labels[TestcontainersLabels.labelLang], "swift") | ||
| XCTAssertEqual(labels[TestcontainersLabels.labelVersion], TestcontainersLabels.version) | ||
| XCTAssertEqual(labels[TestcontainersLabels.labelSessionId], TestcontainersLabels.sessionId) | ||
| } | ||
|
|
||
| // MARK: - addDefaultLabels | ||
|
|
||
| func testAddDefaultLabelsToEmptyDictionary() { | ||
| var labels: [String: String] = [:] | ||
| TestcontainersLabels.addDefaultLabels(to: &labels) | ||
| XCTAssertEqual(labels.count, 4) | ||
| XCTAssertEqual(labels["org.testcontainers"], "true") | ||
| XCTAssertEqual(labels["org.testcontainers.lang"], "swift") | ||
| } | ||
|
|
||
| func testAddDefaultLabelsPreservesUserLabels() { | ||
| var labels: [String: String] = ["app": "myapp", "env": "test"] | ||
| TestcontainersLabels.addDefaultLabels(to: &labels) | ||
| XCTAssertEqual(labels["app"], "myapp") | ||
| XCTAssertEqual(labels["env"], "test") | ||
| XCTAssertEqual(labels["org.testcontainers"], "true") | ||
| XCTAssertEqual(labels.count, 6) // 2 user + 4 standard | ||
| } | ||
|
|
||
| func testAddDefaultLabelsDoesNotOverwriteUserLabels() { | ||
| var labels: [String: String] = [ | ||
| "org.testcontainers": "custom", | ||
| "org.testcontainers.lang": "custom-lang", | ||
| ] | ||
| TestcontainersLabels.addDefaultLabels(to: &labels) | ||
| XCTAssertEqual(labels["org.testcontainers"], "custom", | ||
| "User-defined label should not be overwritten") | ||
| XCTAssertEqual(labels["org.testcontainers.lang"], "custom-lang", | ||
| "User-defined label should not be overwritten") | ||
| // But the missing keys should still be added | ||
| XCTAssertNotNil(labels["org.testcontainers.version"]) | ||
| XCTAssertNotNil(labels["org.testcontainers.sessionId"]) | ||
| } | ||
|
|
||
| // MARK: - ContainerBuilder Integration | ||
|
|
||
| func testBuilderBuildIncludesStandardLabels() { | ||
| let container = ContainerBuilder("alpine:latest").build() | ||
|
|
||
| XCTAssertEqual(container.labels["org.testcontainers"], "true") | ||
| XCTAssertEqual(container.labels["org.testcontainers.lang"], "swift") | ||
| XCTAssertEqual(container.labels["org.testcontainers.version"], TestcontainersLabels.version) | ||
| XCTAssertEqual(container.labels["org.testcontainers.sessionId"], TestcontainersLabels.sessionId) | ||
| } | ||
|
|
||
| func testBuilderBuildWithUserLabelsIncludesBoth() { | ||
| let container = ContainerBuilder("alpine:latest") | ||
| .withLabel("app", "myapp") | ||
| .withLabel("env", "test") | ||
| .build() | ||
|
|
||
| // User labels present | ||
| XCTAssertEqual(container.labels["app"], "myapp") | ||
| XCTAssertEqual(container.labels["env"], "test") | ||
|
|
||
| // Standard labels present | ||
| XCTAssertEqual(container.labels["org.testcontainers"], "true") | ||
| XCTAssertEqual(container.labels["org.testcontainers.lang"], "swift") | ||
| } | ||
|
|
||
| func testBuilderBuildUserLabelsNotOverwritten() { | ||
| let container = ContainerBuilder("alpine:latest") | ||
| .withLabel("org.testcontainers", "custom-value") | ||
| .build() | ||
|
|
||
| XCTAssertEqual(container.labels["org.testcontainers"], "custom-value", | ||
| "User-defined standard label should not be overwritten by defaults") | ||
| } | ||
|
|
||
| func testAllContainersShareSameSessionId() { | ||
| let container1 = ContainerBuilder("alpine:latest").build() | ||
| let container2 = ContainerBuilder("nginx:latest").build() | ||
|
|
||
| XCTAssertEqual( | ||
| container1.labels["org.testcontainers.sessionId"], | ||
| container2.labels["org.testcontainers.sessionId"], | ||
| "All containers in the same process should have the same session ID" | ||
| ) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Convert new test methods to async throws to match test-suite convention.
All newly added test methods in these suites are synchronous. Please switch signatures to async throws for consistency with repository test rules.
Suggested pattern
- func testLabelBaseValue() {
+ func testLabelBaseValue() async throws {
XCTAssertEqual(TestcontainersLabels.labelBase, "org.testcontainers")
}
- func testHeadersContainSessionId() {
+ func testHeadersContainSessionId() async throws {
let headers = TestcontainersDockerClient.testcontainersHeaders()
let sid = headers["x-tc-sid"].first
XCTAssertNotNil(sid, "x-tc-sid header must be present")
XCTAssertEqual(sid, TestcontainersLabels.sessionId)
}As per coding guidelines, "Tests must use XCTest with async throws methods."
Also applies to: 870-948
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 809-809: Tests/TestcontainersTests.swift#L809
Collection literals should not have trailing commas
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/TestcontainersTests.swift` around lines 724 - 866, Update the new test
methods in the TestcontainersLabelsTests class to use the repository test
convention by changing their signatures from synchronous to async throws; e.g.,
modify functions such as testLabelBaseValue, testLabelLangValue,
testLabelVersionValue, testLabelSessionIdValue, testLangIsSwift,
testVersionIsNotEmpty, testSessionIdIsNonEmpty, testSessionIdIsValidUUID,
testSessionIdIsStableAcrossCalls, testDefaultLabelsContainsAllFourKeys,
testDefaultLabelsValues, testAddDefaultLabelsToEmptyDictionary,
testAddDefaultLabelsPreservesUserLabels,
testAddDefaultLabelsDoesNotOverwriteUserLabels,
testBuilderBuildIncludesStandardLabels,
testBuilderBuildWithUserLabelsIncludesBoth,
testBuilderBuildUserLabelsNotOverwritten, and
testAllContainersShareSameSessionId to have signatures like `func <name>() async
throws`; no behavioral changes needed besides updating the signature (and add
awaits where required by async calls).
Comment on lines
+807
to
+810
| var labels: [String: String] = [ | ||
| "org.testcontainers": "custom", | ||
| "org.testcontainers.lang": "custom-lang", | ||
| ] |
Contributor
There was a problem hiding this comment.
Remove trailing commas flagged by static analysis.
Line 809 and Line 921 use trailing commas in collection literals; Codacy is flagging these. If this check is gating, these should be removed.
Quick fix
var labels: [String: String] = [
"org.testcontainers": "custom",
- "org.testcontainers.lang": "custom-lang",
+ "org.testcontainers.lang": "custom-lang"
]
let custom = HTTPHeaders([
("x-tc-sid", "test-session-id"),
- ("User-Agent", "tc-swift/0.1.0"),
+ ("User-Agent", "tc-swift/0.1.0")
])Also applies to: 919-922
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 809-809: Tests/TestcontainersTests.swift#L809
Collection literals should not have trailing commas
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/TestcontainersTests.swift` around lines 807 - 810, Remove the trailing
commas from collection literals flagged by static analysis in
TestcontainersTests.swift: locate the dictionary assigned to the variable labels
(the var labels: [String: String] = [...] block) and any other collection
literals around the same test (the occurrences near the second block around
lines 919-922) and delete the extra comma after the last element so the literals
end without a trailing comma; update the labels assignment and the other
identified array/dictionary literals accordingly to satisfy the linter.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Tests/TestcontainersTests.swift (2)
808-811:⚠️ Potential issue | 🟡 MinorRemove trailing commas in collection literals to satisfy active lint checks.
Line 810 and Line 922 still have trailing commas in literals flagged by Codacy.
Quick fix
var labels: [String: String] = [ "org.testcontainers": "custom", - "org.testcontainers.lang": "custom-lang", + "org.testcontainers.lang": "custom-lang" ] let custom = HTTPHeaders([ ("x-tc-sid", "test-session-id"), - ("User-Agent", "tc-swift/0.1.0"), + ("User-Agent", "tc-swift/0.1.0") ])Also applies to: 920-923
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TestcontainersTests.swift` around lines 808 - 811, Remove trailing commas from Swift collection literals flagged by the linter: update the dictionary assigned to labels (the var labels: [String: String] = [...] in Tests/TestcontainersTests.swift) to omit the comma after the last key/value pair, and likewise remove trailing commas from the other collection literal reported (the similar literal near the end of the test around the second occurrence). Ensure each collection literal ends with the closing bracket immediately after the final element.
728-908:⚠️ Potential issue | 🟡 MinorConvert new test methods to
async throwsto match test-suite rules.The newly added methods in
TestcontainersLabelsTestsandTestcontainersHeadersTestsare still synchronous. Please update signatures (Line 728 through Line 908) toasync throwsfor consistency with repo policy.Suggested pattern
- func testLabelBaseValue() { + func testLabelBaseValue() async throws { XCTAssertEqual(TestcontainersLabels.labelBase, "org.testcontainers") } - func testHeadersContainSessionId() { + func testHeadersContainSessionId() async throws { let headers = TestcontainersDockerClient.testcontainersHeaders() let sid = headers["x-tc-sid"].first XCTAssertNotNil(sid, "x-tc-sid header must be present") XCTAssertEqual(sid, TestcontainersLabels.sessionId) }As per coding guidelines, "Tests must use XCTest with
async throwsmethods".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TestcontainersTests.swift` around lines 728 - 908, Several test methods in TestcontainersLabelsTests and TestcontainersHeadersTests (e.g., testLabelBaseValue, testLangIsSwift, testSessionIdIsValidUUID, testBuilderBuildIncludesStandardLabels, testHeadersContainSessionId, testHeadersUserAgentIncludesVersion, etc.) are synchronous; update each of these test function signatures to be async throws (change "func testXxx()" to "func testXxx() async throws") to comply with the test-suite rule. Locate and edit each test method in the shown range that currently has a plain "func test..." signature and add "async throws" while keeping the body unchanged so XCTest recognizes them as async tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Tests/TestcontainersTests.swift`:
- Line 668: Remove the trailing whitespace and the extra blank line at the end
of the TestcontainersTests scope in Tests/TestcontainersTests.swift (the file's
ending after the TestcontainersTests class/closure); trim the final line so the
file ends with the last token of the scope and no additional blank line or
spaces to satisfy the whitespace checker.
---
Duplicate comments:
In `@Tests/TestcontainersTests.swift`:
- Around line 808-811: Remove trailing commas from Swift collection literals
flagged by the linter: update the dictionary assigned to labels (the var labels:
[String: String] = [...] in Tests/TestcontainersTests.swift) to omit the comma
after the last key/value pair, and likewise remove trailing commas from the
other collection literal reported (the similar literal near the end of the test
around the second occurrence). Ensure each collection literal ends with the
closing bracket immediately after the final element.
- Around line 728-908: Several test methods in TestcontainersLabelsTests and
TestcontainersHeadersTests (e.g., testLabelBaseValue, testLangIsSwift,
testSessionIdIsValidUUID, testBuilderBuildIncludesStandardLabels,
testHeadersContainSessionId, testHeadersUserAgentIncludesVersion, etc.) are
synchronous; update each of these test function signatures to be async throws
(change "func testXxx()" to "func testXxx() async throws") to comply with the
test-suite rule. Locate and edit each test method in the shown range that
currently has a plain "func test..." signature and add "async throws" while
keeping the body unchanged so XCTest recognizes them as async tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1abac4a5-2e35-4c49-a7af-972b58287ff5
📒 Files selected for processing (1)
Tests/TestcontainersTests.swift
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 68.90% 69.29% +0.38%
==========================================
Files 40 41 +1
Lines 1534 1573 +39
==========================================
+ Hits 1057 1090 +33
- Misses 477 483 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Tests/TestcontainersTests.swift (2)
807-810:⚠️ Potential issue | 🟡 MinorRemove trailing commas in collection literals at Line 810 and Line 922.
These are still present and were already flagged by static analysis.
Quick diff
var labels: [String: String] = [ "org.testcontainers": "custom", - "org.testcontainers.lang": "custom-lang", + "org.testcontainers.lang": "custom-lang" ] let custom = HTTPHeaders([ ("x-tc-sid", "test-session-id"), - ("User-Agent", "tc-swift/0.1.0"), + ("User-Agent", "tc-swift/0.1.0") ])Also applies to: 919-922
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TestcontainersTests.swift` around lines 807 - 810, The dictionary literal assigned to variable labels contains trailing commas that cause static-analysis failures; remove the trailing comma after the last entry in the labels literal (the one with "org.testcontainers.lang") and also remove the trailing comma in the other collection literal around lines 919–922 so both collection literals end without a trailing comma; look for the variable name labels in Tests/TestcontainersTests.swift and the sibling collection literal near that block to update their bracketed initializers accordingly.
727-948:⚠️ Potential issue | 🟡 MinorConvert these new test methods to
async throws.From Line 727 through Line 947, the newly added test methods are synchronous. In this repo, test methods should use
async throws.#!/bin/bash python - <<'PY' import re from pathlib import Path lines = Path("Tests/TestcontainersTests.swift").read_text().splitlines() pattern = re.compile(r'^\s*func\s+test\w+\(\)\s*\{') for i in range(724, 949): line = lines[i - 1] if pattern.match(line): print(f"{i}: {line.strip()}") PYAs per coding guidelines, "Tests must use XCTest with
async throwsmethods."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TestcontainersTests.swift` around lines 727 - 948, Several newly added test methods are synchronous but must be declared async throws; update each test function in the block (e.g., testSessionIdIsNonEmpty, testSessionIdIsValidUUID, testSessionIdIsStableAcrossCalls, testDefaultLabelsContainsAllFourKeys, testDefaultLabelsValues, testAddDefaultLabelsToEmptyDictionary, testAddDefaultLabelsPreservesUserLabels, testAddDefaultLabelsDoesNotOverwriteUserLabels, testBuilderBuildIncludesStandardLabels, testBuilderBuildWithUserLabelsIncludesBoth, testBuilderBuildUserLabelsNotOverwritten, testAllContainersShareSameSessionId, testHeadersContainSessionId, testHeadersSessionIdIsValidUUID, testHeadersContainUserAgent, testHeadersUserAgentIncludesVersion, testHeadersAreStableAcrossCalls, testBuildHeadersIncludesBaseHeaders, testBuildHeadersWithCustomHeaders, testBuildHeadersWithNoCustomHeaders) by changing their signatures from "func NAME()" to "func NAME() async throws" and update any synchronous calls inside to use await/try where the called helpers are async (for example when invoking TestcontainersDockerClient.testcontainersHeaders() or ContainerBuilder.build() if they are async), so the tests compile and follow the async throws test convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Tests/TestcontainersTests.swift`:
- Around line 807-810: The dictionary literal assigned to variable labels
contains trailing commas that cause static-analysis failures; remove the
trailing comma after the last entry in the labels literal (the one with
"org.testcontainers.lang") and also remove the trailing comma in the other
collection literal around lines 919–922 so both collection literals end without
a trailing comma; look for the variable name labels in
Tests/TestcontainersTests.swift and the sibling collection literal near that
block to update their bracketed initializers accordingly.
- Around line 727-948: Several newly added test methods are synchronous but must
be declared async throws; update each test function in the block (e.g.,
testSessionIdIsNonEmpty, testSessionIdIsValidUUID,
testSessionIdIsStableAcrossCalls, testDefaultLabelsContainsAllFourKeys,
testDefaultLabelsValues, testAddDefaultLabelsToEmptyDictionary,
testAddDefaultLabelsPreservesUserLabels,
testAddDefaultLabelsDoesNotOverwriteUserLabels,
testBuilderBuildIncludesStandardLabels,
testBuilderBuildWithUserLabelsIncludesBoth,
testBuilderBuildUserLabelsNotOverwritten, testAllContainersShareSameSessionId,
testHeadersContainSessionId, testHeadersSessionIdIsValidUUID,
testHeadersContainUserAgent, testHeadersUserAgentIncludesVersion,
testHeadersAreStableAcrossCalls, testBuildHeadersIncludesBaseHeaders,
testBuildHeadersWithCustomHeaders, testBuildHeadersWithNoCustomHeaders) by
changing their signatures from "func NAME()" to "func NAME() async throws" and
update any synchronous calls inside to use await/try where the called helpers
are async (for example when invoking
TestcontainersDockerClient.testcontainersHeaders() or ContainerBuilder.build()
if they are async), so the tests compile and follow the async throws test
convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43367695-a44c-412d-bdf3-c3a08d533886
📒 Files selected for processing (1)
Tests/TestcontainersTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce standard Testcontainers labels and HTTP headers, and propagate them through container, network, and Docker client integrations.
New Features:
Enhancements:
Build:
Tests:
Continue Tasks: ❌ 3 failed — View all