dotnet / docker-tools

This is a repo to house some common tools for our various docker repos.
MIT License
124 stars 46 forks source link

Remove the need for ManifestClient wrapper for mocking extension methods #1296

Closed lbussell closed 1 month ago

lbussell commented 5 months ago

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).