fix(localstack): correct gofumpt formatting in s3 test#3
Open
aausch wants to merge 18 commits intothaJeztah:migrate_mobyfrom
Open
fix(localstack): correct gofumpt formatting in s3 test#3aausch wants to merge 18 commits intothaJeztah:migrate_mobyfrom
aausch wants to merge 18 commits intothaJeztah:migrate_mobyfrom
Conversation
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The IsZero() branch was inverted during the moby migration, causing the HTTP wait strategy to inspect the container when a specific port was requested and vice versa. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The jsonmessage.DisplayJSONMessagesStream and term.GetFdInfo calls were commented out during the moby migration. The equivalent APIs exist at github.com/moby/moby/client/pkg/jsonmessage and github.com/moby/term. Without this, build errors are silently swallowed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
stdcopy.NewStdWriter is no longer exported from moby. Add a local stdWriter that produces the same Docker-multiplexed frame format (8-byte header + payload), so that stdcopy.StdCopy can still demux stdout/stderr when reading logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The old nat.Port type (a plain string) accepted empty strings:
SplitProtoPort("") returns ("", ""), so Port() == "" and no container
port matches, yielding "not found". The empty-string guard preserves
that behavior while avoiding a ParsePort error.
Add a test to verify MappedPort("") returns ErrNotFound.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
During the moby migration, two test mocks were incorrectly updated:
1. TestHttpStrategyFailsWhileGettingPortDueToUnexpectedContainerStatus:
Status was changed from "dead" to container.StateRunning. The test
intent is to simulate a dead container — fixed to container.StateDead.
2. TestHttpStrategyFailsWhileRequestSendingDueToUnexpectedContainerStatus:
Status was correctly set to container.StateDead, but the expected
error message was changed from "dead" to "running" — reverted.
Both tests originally used Status: "dead" (bare string). The old
docker/docker container.State.Status was a plain string field; the new
moby type uses the ContainerState enum. The migration should have used
container.StateDead ("dead"), not container.StateRunning ("running").
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The moby migration changed client.NetworkList() return type from []network.Inspect (a slice) to network.List (a struct). The test called require.Len() on the struct, which fails because len() can't be applied to a struct. The .Items field (already used on the next line) holds the actual slice. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MergeCustomLabels previously validated and merged in a single pass. If a reserved-prefix key was encountered partway through iteration, some keys would already be merged while others would not. Since Go map iteration order is non-deterministic, this produced flaky test results — the test assumed "B" was always merged before the invalid LabelLang key was encountered. Split into two passes: validate all keys first, then merge with maps.Copy. On error, dst is now guaranteed to be unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After container termination, Docker may not have fully disconnected endpoints from the network yet. The moby client surfaces this as "has active endpoints" on NetworkRemove. This race was latent with the old docker/docker client but is now consistently triggered. Retry up to 3 times with a 500ms delay when the error indicates active endpoints remain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three issues from the moby migration in the ollama local process: 1. Inspect() populated Networks with a "bridge" entry, but the old code used an empty map (bridge info was in the deprecated NetworkSettingsBase.Bridge field). Restored empty map. 2. Tests compared port.Proto() (network.IPProtocol) to "tcp" (string). The old nat.Port.Proto() returned string; the new network.Port.Proto() returns network.IPProtocol. Fixed to compare against network.TCP. 3. Tests compared port[0].HostIP (netip.Addr) to "127.0.0.1" (string). The old PortBinding.HostIP was string; the new one is netip.Addr. Fixed to compare against netip.MustParseAddr(testHost). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SkipIfDockerDesktop only checked for Docker Desktop by OS name, but colima also runs Docker inside a Linux VM where host networking doesn't work. Detect colima via DOCKER_HOST containing "colima" and skip early. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Alex Ausch <alex@ausch.name> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
thaJeztah
force-pushed
the
migrate_moby
branch
5 times, most recently
from
d6c4a2d to
bb76dfe
Compare
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
2 participants
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
gcilinting failure inmodules/localstack/v1/s3_test.goby adding spaces around+operator in string concatenation (gofumpt formatting)Test plan
golangci-lint run ./...passes inmodules/localstack🤖 Generated with Claude Code