From c805fb367bd06881e36962708e3b3d77012ed476 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Thu, 28 Jul 2022 14:58:05 -0400 Subject: [PATCH] PR: doc, simplified signitures Added deprecated warning to `layers.LayerFolders`, which relies on docker. Added doc comment to functional tests to clarify overlap with other tests. Removed unnecessary parameter in `WaitForError`. Signed-off-by: Hamza El-Saawy --- test/functional/lcow_container_bench_test.go | 5 ++--- test/functional/lcow_container_test.go | 12 ++++++------ test/functional/main_test.go | 5 +++++ test/internal/containerd/containerd.go | 11 ----------- test/internal/layers/layerfolders.go | 1 + test/internal/timeout/timeout.go | 10 +++++----- test/internal/uvm/uvm.go | 4 ++-- 7 files changed, 21 insertions(+), 27 deletions(-) diff --git a/test/functional/lcow_container_bench_test.go b/test/functional/lcow_container_bench_test.go index 88bddc71ab..335c3be161 100644 --- a/test/functional/lcow_container_bench_test.go +++ b/test/functional/lcow_container_bench_test.go @@ -30,8 +30,8 @@ func BenchmarkLCOW_Container(b *testing.B) { require.Build(b, osversion.RS5) ctx, _, client := newContainerdClient(context.Background(), b) - cid := containerd.PullImage(ctx, b, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) - ls := layers.FromChainID(ctx, b, client, cid, constants.SnapshotterLinux) + chainID := containerd.PullImage(ctx, b, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) + ls := layers.FromChainID(ctx, b, client, chainID, constants.SnapshotterLinux) // Create a new uvm per benchmark in case any left over state lingers @@ -225,5 +225,4 @@ func BenchmarkLCOW_Container(b *testing.B) { cleanup() } }) - } diff --git a/test/functional/lcow_container_test.go b/test/functional/lcow_container_test.go index 6a12705c8d..11bace59dd 100644 --- a/test/functional/lcow_container_test.go +++ b/test/functional/lcow_container_test.go @@ -27,8 +27,8 @@ func TestLCOW_ContainerLifecycle(t *testing.T) { require.Build(t, osversion.RS5) ctx, _, client := newContainerdClient(context.Background(), t) - cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) - ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux) + chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) + ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux) opts := defaultLCOWOptions(t, t.Name()) vm := uvm.CreateAndStartLCOWFromOpts(ctx, t, opts) @@ -84,8 +84,8 @@ func TestLCOW_ContainerIO(t *testing.T) { require.Build(t, osversion.RS5) ctx, _, client := newContainerdClient(context.Background(), t) - cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) - ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux) + chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) + ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux) opts := defaultLCOWOptions(t, t.Name()) cache := layers.CacheFile(ctx, t, "") @@ -129,8 +129,8 @@ func TestLCOW_ContainerExec(t *testing.T) { require.Build(t, osversion.RS5) ctx, _, client := newContainerdClient(context.Background(), t) - cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) - ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux) + chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux) + ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux) opts := defaultLCOWOptions(t, t.Name()) vm := uvm.CreateAndStartLCOWFromOpts(ctx, t, opts) diff --git a/test/functional/main_test.go b/test/functional/main_test.go index 820418c641..49261758af 100644 --- a/test/functional/main_test.go +++ b/test/functional/main_test.go @@ -1,5 +1,10 @@ //go:build windows && functional +// This package tests the internals of hcsshim, independent of the OCI interfaces it exposes +// and the container runtime (or CRI API) that normally would be communicating with the shim. +// +// While these tests may overlap with CRI/containerd or shim tests, they exercise `internal/*` +// code paths and primitives directly. package functional import ( diff --git a/test/internal/containerd/containerd.go b/test/internal/containerd/containerd.go index 0c368a1b0c..313a82914e 100644 --- a/test/internal/containerd/containerd.go +++ b/test/internal/containerd/containerd.go @@ -184,17 +184,6 @@ func PullImage(ctx context.Context, t testing.TB, client *containerd.Client, ref t.Fatalf("could not pull image %q: %v", ref, err) } - // img, err := client.Fetch(ctx, ref, opts...) - // if err != nil { - // t.Fatalf("could not fetch image %q: %v", ref, err) - // } - // t.Logf("fetched image %q", img.Name) - - // i := containerd.NewImageWithPlatform(client, img, p) - // if err := i.Unpack(ctx, ss); err != nil { - // t.Fatalf("could not unpack image %q: %v", i.Name(), err) - // } - name := img.Name() diffIDs, err := img.RootFS(ctx) if err != nil { diff --git a/test/internal/layers/layerfolders.go b/test/internal/layers/layerfolders.go index 3f63492970..0ced953717 100644 --- a/test/internal/layers/layerfolders.go +++ b/test/internal/layers/layerfolders.go @@ -48,6 +48,7 @@ func FromMount(_ context.Context, t testing.TB, m mount.Mount) (layers []string) return layers } +// Deprecated: This relies on docker. Use [FromChainID] or [FromMount] instead. func LayerFolders(t testing.TB, imageName string) []string { if _, ok := imageLayers[imageName]; !ok { imageLayers[imageName] = getLayers(t, imageName) diff --git a/test/internal/timeout/timeout.go b/test/internal/timeout/timeout.go index 756652a07e..38f8212d39 100644 --- a/test/internal/timeout/timeout.go +++ b/test/internal/timeout/timeout.go @@ -13,11 +13,11 @@ const ConnectTimeout = time.Second * 10 type ErrorFunc func(error) error // WaitForError waits until f returns or the context is done. -// The returned error will be passed to the error processing functions fe or fec, respectively. -// If processed error is non-nil, it is passed to t.Fatal(). +// The returned error will be passed to the error processing functions fe, and (if non-nil), +// then passed to to [testing.Fatal]. // -// Both error processing functions should expect nil errors. -func WaitForError(ctx context.Context, t testing.TB, f func() error, fe, fec ErrorFunc) { +// fe should expect nil errors. +func WaitForError(ctx context.Context, t testing.TB, f func() error, fe ErrorFunc) { var err error ch := make(chan struct{}) @@ -30,7 +30,7 @@ func WaitForError(ctx context.Context, t testing.TB, f func() error, fe, fec Err case <-ch: fatalOnError(t, fe, err) case <-ctx.Done(): - fatalOnError(t, fec, ctx.Err()) + fatalOnError(t, fe, ctx.Err()) } } diff --git a/test/internal/uvm/uvm.go b/test/internal/uvm/uvm.go index 74bc9c5181..02bf301b2d 100644 --- a/test/internal/uvm/uvm.go +++ b/test/internal/uvm/uvm.go @@ -36,7 +36,7 @@ func Wait(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) { return err } - timeout.WaitForError(ctx, t, vm.Wait, fe, fe) + timeout.WaitForError(ctx, t, vm.Wait, fe) } func Kill(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) { @@ -55,5 +55,5 @@ func Close(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) { return err } - timeout.WaitForError(ctx, t, vm.Close, fe, fe) + timeout.WaitForError(ctx, t, vm.Close, fe) }