atelet: add successful RPC boundary tests#469
Conversation
089f487 to
711e3bb
Compare
|
Dmitry Berkovich (@dberkov) ptal, thanks! |
| // and sandbox record handling all use the production paths. | ||
| func TestRPCBoundariesAccept(t *testing.T) { | ||
| origBasePath, origStaticFilesDir := ateompath.BasePath, ateompath.StaticFilesDir | ||
| ateompath.BasePath = t.TempDir() |
There was a problem hiding this comment.
Mutable derived path vars are a footgun (main concern). StaticFilesDir is derived from BasePath at package init, so a test that overrides BasePath must remember to also override every derived var — this test does, but the invariant lives in the test, not the package. Concretely: the in-flight PR #467 adds ImageCacheDir = filepath.Join(BasePath, "image-cache") to that same var block. Once both merge, this test would silently leave ImageCacheDir pointing at
/var/lib/ateom-gvisor/image-cache while BasePath points at a tempdir.
There was a problem hiding this comment.
Updated the derived paths to functions of BasePath and added a regression test covering StaticFilesDir, ImageCacheDir, and RunSCBinaryPath.
| @@ -458,6 +461,156 @@ func TestRPCBoundariesReject(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Global mutation rules out t.Parallel. Fine today (no parallel tests in the package), but worth a one-line comment on the test so nobody adds t.Parallel() to a sibling test that touches ateompath paths.
There was a problem hiding this comment.
Added a comment that the test must not run in parallel because it redirects the package-wide BasePath.
| Fetch(context.Context, string) (io.ReadCloser, *v1.Config, error) | ||
| } | ||
|
|
||
| func prepareOCIDirectory(ctx context.Context, pullCache imagePuller, actorUID, containerName, ref string, command, args []string, env []string, annotations map[string]string, netns string, identityDir string, durableDirVolumeMounts []*ateletpb.VolumeMount) error { |
|
|
||
| client := &fakeAteomClient{} | ||
| dialer := &fakeAteomDialer{client: client} | ||
| s := &AteomHerder{ateomDialer: dialer, pullCache: fakeImagePuller{}} |
There was a problem hiding this comment.
Nil GCS clients are safe only implicitly. The AteomHerder fixture omits anonGCSClient/gcsClient; that's sound because the runsc binary is pre-seeded (so ensureSandboxAssets cache-hits) and checkpoint type is LOCAL — but a future edit that causes a cache miss would nil-panic rather than fail with a message. A one-line comment stating why nil is OK would save the next person a debugging round.
// GCS clients stay nil: the runsc asset is pre-seeded above (so
// ensureSandboxAssets never downloads) and the checkpoint/restore legs
// use CHECKPOINT_TYPE_LOCAL, so no code path touches object storage.
There was a problem hiding this comment.
Added the requested comment explaining why the nil GCS clients are safe in this fixture.
711e3bb to
6f40c26
Compare
6f40c26 to
02ab98b
Compare
2 participants
Summary
Testing
go test ./cmd/atelet ./internal/ateompathgo test ./...Closes #217