Skip to content

Cache container images during tests to prevent unnecessary loading - #41353

Merged
ggarzia-MSFT merged 7 commits into
masterfrom
user/ggarzia/cache-container-images
Aug 20, 2026
Merged

Cache container images during tests to prevent unnecessary loading#41353
ggarzia-MSFT merged 7 commits into
masterfrom
user/ggarzia/cache-container-images

Conversation

@ggarzia-MSFT

@ggarzia-MSFT ggarzia-MSFT commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Adds a TestImageRegistry to the wslc E2E tests that tracks which test images are already
loaded in a session, so repeated setup across test classes stops reloading the same image
tarballs. Measured 43.6 s (10.1%) faster across the 475 affected tests.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Test-only change; no product code is touched.

Previously every test class independently ran EnsureImageIsLoaded in setup and
EnsureImageIsDeleted in cleanup. Because classes share a small set of base images
(debian, alpine, hello-world, python), each cleanup deleted an image the next class
immediately reloaded, costing ~1 s per reload.

TestImageRegistry is a per-process singleton that seeds itself once per session from a
live image list query, then answers EnsureLoaded from memory. Shared-image deletes were
removed from class cleanups; images that a test genuinely needs gone are still deleted
explicitly.

Keeping a cache honest is the hard part, so rather than trusting call sites to report
mutations, NoteCommand is hooked into the three RunWslc primitives and observes every
wslc invocation. Only removal verbs matter — image remove/delete/rm, image prune,
and system session terminate — because commands that add an image can at worst leave the
cache pessimistic, which costs one extra query rather than producing a wrong answer.

Invalidation is per-image where the command names its targets, and session-wide only when
the affected set can't be known (image prune, session teardown, digest or port-qualified
references). This distinction matters: an earlier revision invalidated session-wide on every
removal, which destroyed the seed and gave back 13.5 s of the win.

All image handling now lives on the registry; the free EnsureImageIsLoaded and
EnsureImageIsDeleted helpers are deleted. Delete deliberately queries image list
directly instead of consulting the cache, since built and imported images are created
outside the registry and a cached negative would silently leak them.

Validation Steps Performed

Full 475-test cacheable selection (27 classes), x64 Debug, 3 runs per arm against an
identical deployment:

Arm Runs Mean Spread
master 428.00 / 430.96 / 429.72 429.56 s 3.0 s
this change 385.55 / 384.40 / 388.04 386.00 s 3.6 s

43.56 s faster (10.1%), 475/475 passing on all six runs. Ranges are separated by a 40 s
gap with no overlap.

ggarzia-MSFT and others added 2 commits August 13, 2026 16:25
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 13, 2026 23:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes WSLC Windows E2E test execution by introducing a per-process TestImageRegistry that caches which base test images are already loaded per session, avoiding repeated image list + image load work across many test classes. It integrates registry invalidation by observing WSLC commands executed via the existing executor helpers.

Changes:

  • Added TestImageRegistry (singleton) to seed per-session image inventory once and serve EnsureLoaded from memory, plus a Delete helper that queries live inventory to remove images safely.
  • Hooked command observation (NoteCommand) into WSLC executor entry points so cache state is invalidated when removal-like commands are issued.
  • Updated WSLC E2E tests to use TestImageRegistry APIs and removed the legacy EnsureImageIsLoaded/EnsureImageIsDeleted helpers.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/windows/wslc/e2e/WSLCExecutor.cpp Hooks TestImageRegistry::NoteCommand() into WSLC execution paths.
test/windows/wslc/e2e/WSLCE2EWarningTests.cpp Switches setup to TestImageRegistry::EnsureLoaded; removes shared-image cleanup.
test/windows/wslc/e2e/WSLCE2EVolumeRemoveTests.cpp Uses registry-based image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EVolumePruneTests.cpp Uses registry-based image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2ETlsRegistryTests.cpp Uses registry-based image load for named sessions.
test/windows/wslc/e2e/WSLCE2ERegistryTests.cpp Uses registry-based image load for test image prerequisites.
test/windows/wslc/e2e/WSLCE2EPushPullTests.cpp Uses registry-based image load for push/pull prerequisites.
test/windows/wslc/e2e/WSLCE2ENetworkTests.cpp Uses registry-based image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2ENetworkPruneTests.cpp Uses registry-based image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EInspectTests.cpp Uses registry-based image load and registry Delete for test-created images.
test/windows/wslc/e2e/WSLCE2EImageTagTests.cpp Uses registry EnsureLoaded / Delete and reduces cleanup deletions to avoid thrash.
test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp Uses registry image load/delete for save/load flows and cleanup.
test/windows/wslc/e2e/WSLCE2EImagePruneTests.cpp Uses registry image load/delete in prune scenarios and cleanup.
test/windows/wslc/e2e/WSLCE2EImageListTests.cpp Uses registry image load and removes shared-image cleanup.
test/windows/wslc/e2e/WSLCE2EImageInspectTests.cpp Uses registry image load and registry Delete for built image cleanup.
test/windows/wslc/e2e/WSLCE2EImageImportTests.cpp Uses registry image load/delete for import prerequisites and cleanup.
test/windows/wslc/e2e/WSLCE2EImageDeleteTests.cpp Uses registry image load/delete for delete command test setup/cleanup.
test/windows/wslc/e2e/WSLCE2EImageBuildTests.cpp Uses registry image load for build prerequisites; removes shared-image cleanup.
test/windows/wslc/e2e/WSLCE2EHelpers.h Removes declarations of legacy image load/delete helpers.
test/windows/wslc/e2e/WSLCE2EHelpers.cpp Removes implementations of legacy image load/delete helpers; adds registry include.
test/windows/wslc/e2e/WSLCE2EGlobalTests.cpp Uses registry image load for session-targeted tests.
test/windows/wslc/e2e/WSLCE2EContainerStopTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerStatsTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerRemoveTests.cpp Uses registry image load and registry Delete for test-created images.
test/windows/wslc/e2e/WSLCE2EContainerPruneTests.cpp Uses registry image load in class setup.
test/windows/wslc/e2e/WSLCE2EContainerLogsTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerListTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerKillTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerInspectTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerExportTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerExecTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp Uses registry image load; uses registry Delete for test-created images.
test/windows/wslc/e2e/WSLCE2EContainerCpTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/WSLCE2EContainerAttachTests.cpp Uses registry image load; removes shared-image deletion from cleanup.
test/windows/wslc/e2e/TestImageRegistry.h Introduces the registry API and cached state structures.
test/windows/wslc/e2e/TestImageRegistry.cpp Implements seeding, load/delete behavior, and command-based invalidation logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/windows/wslc/e2e/TestImageRegistry.cpp
@ggarzia-MSFT
ggarzia-MSFT marked this pull request as ready for review August 17, 2026 16:28
@ggarzia-MSFT
ggarzia-MSFT requested review from a team as code owners August 17, 2026 16:28
@ggarzia-MSFT ggarzia-MSFT changed the title User/ggarzia/cache container images Cache container images during tests to prevent unnecessary loading Aug 17, 2026
Comment thread test/windows/wslc/e2e/TestImageRegistry.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 18, 2026 16:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/windows/wslc/e2e/TestImageRegistry.cpp:47

  • EnsureSeeded permanently short-circuits after the first image list per sessionName, but this PR description says the cache is kept honest by observing every wslc invocation (image delete/prune/session terminate). In the current implementation there’s no invalidation path, so any later mutation done via plain RunWslc(...) (e.g. image delete, image prune, system session terminate) can leave m_loaded stale and cause EnsureLoaded to incorrectly skip reloading.
void TestImageRegistry::EnsureSeeded(const std::wstring& sessionName)
{
    if (m_seededSessions.contains(sessionName))
    {
        return;

test/windows/wslc/e2e/WSLCE2EHelpers.cpp:430

  • PR description says the image cache stays honest by observing removal verbs including system session terminate, but EnsureSessionIsTerminated does not interact with TestImageRegistry at all. If tests terminate/recreate sessions within the same process, the registry’s per-session seed (m_seededSessions) can become stale unless termination triggers an explicit invalidation path.
void EnsureSessionIsTerminated(const std::wstring& sessionName)
{
    std::wstring targetSession = sessionName;

…container-images

# Conflicts:
#	test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp
Copilot AI review requested due to automatic review settings August 18, 2026 16:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/windows/wslc/e2e/WSLCE2EHelpers.cpp:22

  • TestImageRegistry.h is included here but not used anywhere in this file. Keeping unused includes increases compile time and can introduce accidental dependencies.
#include "WSLCE2EHelpers.h"
#include "TestImageRegistry.h"
#include <JsonUtils.h>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 18, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

test/windows/wslc/e2e/TestImageRegistry.h:27

  • PR description says the registry observes every wslc invocation via a NoteCommand hook into the RunWslc primitives to keep the cache honest, but this implementation has no NoteCommand/invalidation mechanism and instead relies on call sites to route removals through Delete() or to manually Restore(). That’s a significant behavior gap: any test that runs RunWslc("image delete/remove/rm/prune") directly can make m_loaded stale and cause later EnsureLoaded() calls to incorrectly skip a needed image load.

Either implement the command-observation invalidation described in the PR text, or update the PR description and ensure all test call sites comply with the stricter invariant.

// The cache is only correct as long as every removal is either routed through Delete or followed
// by Restore, so a test that removes images another way has to put the session back itself.
class TestImageRegistry

Comment thread test/windows/wslc/e2e/TestImageRegistry.cpp
…ng collisions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 18, 2026 23:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/windows/wslc/e2e/TestImageRegistry.cpp:103

  • This code parses image list --format json output as ImageInformation, but the list command’s JSON shape is ImageOutputInformation (string fields, including Size, and <none> sentinels). As written, this can throw during parsing or mis-detect presence. Parse ImageOutputInformation here (as other tests do) and compare Repository/Tag strings against the target name/tag (taking <none> into account).

Note: EnsureSeeded has the same parse-type issue and must be fixed at the same time.

    auto result = RunWslc(FormatCommand(sessionName, L"image list --format json"));
    result.Verify({.Stderr = L"", .ExitCode = 0});

    const auto images = ParseNdjsonOutputAs<wsl::windows::wslc::models::ImageInformation>(result);
    const auto name = wsl::shared::string::WideToMultiByte(image.Name);
    const auto tag = wsl::shared::string::WideToMultiByte(image.Tag);
    const bool present =
        std::ranges::any_of(images, [&](const auto& candidate) { return candidate.Repository == name && candidate.Tag == tag; });

test/windows/wslc/e2e/TestImageRegistry.cpp:54

  • image list --format json emits the ImageOutputInformation shape (string fields, <none> sentinels). Parsing that output as ImageInformation will fail type conversion (e.g., Size is a string in the output but an int64_t in ImageInformation) and can break seeding/caching. Use ImageOutputInformation here and only cache entries where Repository/Tag aren’t <none>.

Note: the same incorrect parse type is also used in TestImageRegistry::Delete below, so both call sites need to be updated together.

    auto result = RunWslc(FormatCommand(sessionName, L"image list --format json"));
    result.Verify({.Stderr = L"", .ExitCode = 0});

    const auto images = ParseNdjsonOutputAs<wsl::windows::wslc::models::ImageInformation>(result);

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 19, 2026 16:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/windows/wslc/e2e/TestImageRegistry.h:29

  • TestImageRegistry keeps per-session state in m_loaded/m_seededSessions but currently has no way to invalidate it when a session is recreated (e.g., system session terminate / EnsureSessionIsTerminated). After a terminate, EnsureLoaded() can incorrectly short-circuit because the cache still claims an image exists even though the new session is empty, which can break later test classes that rely on the base images being present.

Consider adding an explicit session invalidation API (e.g., InvalidateSession(sessionName) that clears all cached ImageKeys for that session and removes it from m_seededSessions) and calling it from the session-terminate helpers or by hooking the RunWslc wrappers so terminations are always observed.

// The cache is only correct as long as every removal is either routed through Delete or followed
// by Restore, so a test that removes images another way has to put the session back itself.
class TestImageRegistry
{
public:

@ggarzia-MSFT
ggarzia-MSFT merged commit 3ae2ead into master Aug 20, 2026
12 checks passed
@ggarzia-MSFT
ggarzia-MSFT deleted the user/ggarzia/cache-container-images branch August 20, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants