driver/kubernetes/context: don't use ResolveDefaultContext in test#3444
driver/kubernetes/context: don't use ResolveDefaultContext in test#3444crazy-max merged 1 commit intodocker:masterfrom
Conversation
The ResolveDefaultContext function is only used internally by the CLI, and has no known external users, except for this test in buildx. It was exported in [cli@f820766] to allow (unit) testing, but did not document that it was only exported for this purpose. This patch rewrites the test to allow deprecating / removing the function in the CLI. [cli@f820766]: docker/cli@f820766 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
Not related but macos-14 runner looks broken: https://github.com/docker/buildx/actions/runs/18158118072/job/51682545037?pr=3444 
|
|
Yeah; github is having an incident with macOS runners. Also worth noting (but not sure if used here) that macOS 13 is being deprecated, and they added |
| // FIXME(thaJeztah): the context-store is not initialized until "Initialize" is called. | ||
| err = dockerCLI.Initialize(cliflags.NewClientOptions()) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
I want to have a look at this; the whole CLI (and CLI-plugin) bootstrapping is really involved. IMO, the CLI should be in a usable state after NewDockerCLI, but need to check if everything CAN be initialised in that constructor due to some of the oddities with flag parsing and top-level flags;
If we can't initialise in the NewDockerCli constructor, perhaps we can add some lazy constructor for the ContextStore() to at least prevent it returning nil (and panic in this test 😂)
|
🎉 OK, all happy now; should be ready to merge ❤️ |
2 participants
The ResolveDefaultContext function is only used internally by the CLI, and has no known external users, except for this test in buildx. It was exported in cli@f820766 to allow (unit) testing, but did not document that it was only exported for this purpose.
This patch rewrites the test to allow deprecating / removing the function in the CLI.