docker-tools icon indicating copy to clipboard operation
docker-tools copied to clipboard

Remove the need for ManifestClient wrapper for mocking extension methods

Open lbussell opened this issue 1 year ago • 0 comments

https://github.com/dotnet/docker-tools/pull/1293 introduced the InnerManifestClient/ManifestClient pairing of classes. ManifestClient wraps the InnerManifestClient (which was formerly just ManifestClient) so that we can mock its extension methods for testing.

The core issue is that GetImageDigestAsync makes a call to the Docker CLI, so mocking the InnerManifestClient by itself is not sufficient.

https://github.com/dotnet/docker-tools/blob/354896e65e7893aa79cfef70751e66db666077a6/src/Microsoft.DotNet.ImageBuilder/src/ManifestServiceExtensions.cs#L38-L65

We should re-architect this code so that we can mock only the InnerManifestClient's methods directly, and then remove the wrapper and rename.

I was thinking that we could separate out GetImageDigestAsync into ManifestServiceExtensions.GetImageDigestAsync and DockerService.GetLocalImageDigest. Then, callers (like the build command) could define their own method for checking local images first or reaching out to the ACR when a local image isn't found (or define a static method for that, taking both services as arguments, to provide a shared implementation for all of the callers).

lbussell avatar May 20 '24 21:05 lbussell