From 7d181e546f65bf7904638dd8f58eac9851f2ea5d Mon Sep 17 00:00:00 2001 From: politerealism Date: Wed, 14 Oct 2026 13:28:47 -0400 Subject: [PATCH] test(conformance): migrate create-upload tests to file_transfer stories Move the three `sandbox create --upload` tests from the legacy Podman e2e crate into the file_transfer conformance area as story modules. The behavior is not Podman-specific, so it belongs with the other file transfer stories and runs against any prepared gateway. Test names are unchanged so filters keep working: - create_with_upload_directory_preserves_source_basename - create_with_multiple_uploads - create_with_upload_single_file Changes from the originals: - Verify uploads by reading the files back with `sandbox exec` and comparing exact content, instead of matching the printed output of `sandbox create`. - Check the nested file in the directory story, and that the directory is not flattened into the destination. - Check each destination in the multiple-uploads story separately. - Use the default upload path. The originals passed `--no-git-ignore`, which is unnecessary outside a Git work tree and hid the default path. - Create the sandbox from the target's default image. The originals created it from the e2e test image. - Drop the claim that files arrive before the command runs. The CLI rejects `--upload` together with a command, and the docs say uploads finish after the main process starts, so each story checks the files once `create` returns and documents the limitation. Remove the now-unused `SandboxGuard::create_with_upload` and `create_with_uploads` from the legacy harness, and the old test file. Contributes to #3954 Signed-off-by: politerealism --- e2e/rust/src/harness/sandbox.rs | 81 --------- e2e/rust/tests/upload_create.rs | 113 ------------ .../cli/tests/file_transfer/create_upload.rs | 166 ++++++++++++++++++ .../cli/tests/file_transfer/helpers.rs | 65 +++++++ .../cli/tests/file_transfer/main.rs | 1 + 5 files changed, 232 insertions(+), 194 deletions(-) delete mode 100644 e2e/rust/tests/upload_create.rs create mode 100644 e2e/suites/conformance/cli/tests/file_transfer/create_upload.rs diff --git a/e2e/rust/src/harness/sandbox.rs b/e2e/rust/src/harness/sandbox.rs index 0ebfde9081..20270a483c 100644 --- a/e2e/rust/src/harness/sandbox.rs +++ b/e2e/rust/src/harness/sandbox.rs @@ -390,87 +390,6 @@ impl SandboxGuard { }) } - /// Create a detached scratch sandbox with pre-loaded files, then exec a - /// command in it. - /// - /// Equivalent to: - /// ```text - /// openshell sandbox create --detach --upload : [extra_args...] - /// openshell sandbox exec -- - /// ``` - /// - /// The `--no-git-ignore` flag is passed to avoid needing a git repository. - /// - /// # Errors - /// - /// Returns an error if the CLI exits with a non-zero status or the sandbox - /// name cannot be parsed. - pub async fn create_with_upload( - upload_local: &str, - upload_dest: &str, - command: &[&str], - ) -> Result { - Self::create_with_uploads(&[(upload_local, upload_dest)], command).await - } - - /// Create a sandbox with multiple `--upload` specs. - /// - /// Each element of `uploads` is a `(local_path, sandbox_dest)` pair. - /// `--no-git-ignore` is applied to all uploads. - pub async fn create_with_uploads( - uploads: &[(&str, &str)], - command: &[&str], - ) -> Result { - let mut cmd = openshell_cmd(); - cmd.arg("sandbox").arg("create").arg("--detach"); - add_unique_name_if_missing(&mut cmd, &[]); - add_test_image_if_missing(&mut cmd, &[]); - for (local, dest) in uploads { - cmd.arg("--upload").arg(format!("{local}:{dest}")); - } - cmd.arg("--no-git-ignore"); - cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); - - let output = timeout(SANDBOX_READY_TIMEOUT, cmd.output()) - .await - .map_err(|_| { - format!("sandbox create --upload timed out after {SANDBOX_READY_TIMEOUT:?}") - })? - .map_err(|e| format!("failed to spawn openshell: {e}"))?; - - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - let combined = format!("{stdout}{stderr}"); - - if !output.status.success() { - return Err(format!( - "sandbox create --upload failed (exit {:?}):\n{combined}", - output.status.code() - )); - } - - let name = extract_sandbox_name(&combined).ok_or_else(|| { - format!("could not parse sandbox name from create output:\n{combined}") - })?; - - let mut guard = Self { - name, - create_output: combined, - child: None, - cleaned_up: false, - }; - - match guard.exec(command).await { - Ok(exec_output) => guard.create_output.push_str(&exec_output), - Err(err) => { - guard.cleanup().await; - return Err(err); - } - } - - Ok(guard) - } - /// Upload local files to the sandbox via `openshell sandbox upload`. /// /// # Arguments diff --git a/e2e/rust/tests/upload_create.rs b/e2e/rust/tests/upload_create.rs deleted file mode 100644 index 5cc3964841..0000000000 --- a/e2e/rust/tests/upload_create.rs +++ /dev/null @@ -1,113 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -#![cfg(feature = "e2e")] - -//! E2E test: `sandbox create --upload` pre-loads files before running a command. -//! -//! Validates that the `--upload :` flag on `sandbox create` -//! transfers files into the sandbox before the user command executes, -//! so the command can read the uploaded content. -//! -//! Prerequisites: -//! - A running openshell gateway (`mise run gateway:docker`) -//! - The `openshell` binary (built automatically from the workspace) - -use std::fs; - -use openshell_e2e::harness::output::strip_ansi; -use openshell_e2e::harness::sandbox::SandboxGuard; - -/// Create a sandbox with `--upload dir:/sandbox/data` and verify directory -/// uploads preserve the source basename at `/sandbox/data//...`. -#[tokio::test] -async fn create_with_upload_directory_preserves_source_basename() { - let tmpdir = tempfile::tempdir().expect("create tmpdir"); - - // Create a directory with files to upload. - let upload_dir = tmpdir.path().join("project"); - fs::create_dir_all(upload_dir.join("src")).expect("create project/src"); - fs::write(upload_dir.join("marker.txt"), "upload-create-marker").expect("write marker.txt"); - fs::write(upload_dir.join("src/main.py"), "print('hello')").expect("write main.py"); - - let upload_str = upload_dir.to_str().expect("upload path is UTF-8"); - let remote_marker = "/sandbox/data/project/marker.txt"; - - // The command reads the marker file — if upload worked, its content - // appears in the output. - let mut guard = - SandboxGuard::create_with_upload(upload_str, "/sandbox/data", &["cat", remote_marker]) - .await - .expect("sandbox create --upload"); - - let clean = strip_ansi(&guard.create_output); - assert!( - clean.contains("upload-create-marker"), - "expected uploaded marker content in sandbox output:\n{clean}" - ); - - guard.cleanup().await; -} - -/// Two `--upload` specs in a single `sandbox create` call should both land in -/// the sandbox before the command runs. -#[tokio::test] -async fn create_with_multiple_uploads() { - let tmpdir = tempfile::tempdir().expect("create tmpdir"); - - let dir_a = tmpdir.path().join("alpha"); - let dir_b = tmpdir.path().join("beta"); - fs::create_dir_all(&dir_a).expect("create alpha"); - fs::create_dir_all(&dir_b).expect("create beta"); - fs::write(dir_a.join("a.txt"), "content-alpha").expect("write a.txt"); - fs::write(dir_b.join("b.txt"), "content-beta").expect("write b.txt"); - - let spec_a = dir_a.to_str().expect("alpha path is UTF-8"); - let spec_b = dir_b.to_str().expect("beta path is UTF-8"); - - let mut guard = SandboxGuard::create_with_uploads( - &[(spec_a, "/sandbox/alpha"), (spec_b, "/sandbox/beta")], - &[ - "sh", - "-c", - "cat /sandbox/alpha/alpha/a.txt /sandbox/beta/beta/b.txt", - ], - ) - .await - .expect("sandbox create with multiple --upload flags"); - - let clean = strip_ansi(&guard.create_output); - assert!( - clean.contains("content-alpha"), - "expected alpha content in output:\n{clean}" - ); - assert!( - clean.contains("content-beta"), - "expected beta content in output:\n{clean}" - ); - - guard.cleanup().await; -} - -/// `--upload` with a single file (not a directory) should work. -#[tokio::test] -async fn create_with_upload_single_file() { - let tmpdir = tempfile::tempdir().expect("create tmpdir"); - let file_path = tmpdir.path().join("config.txt"); - fs::write(&file_path, "single-file-upload-test").expect("write config.txt"); - - let file_str = file_path.to_str().expect("file path is UTF-8"); - - let mut guard = - SandboxGuard::create_with_upload(file_str, "/sandbox", &["cat", "/sandbox/config.txt"]) - .await - .expect("sandbox create --upload single file"); - - let clean = strip_ansi(&guard.create_output); - assert!( - clean.contains("single-file-upload-test"), - "expected single-file content in sandbox output:\n{clean}" - ); - - guard.cleanup().await; -} diff --git a/e2e/suites/conformance/cli/tests/file_transfer/create_upload.rs b/e2e/suites/conformance/cli/tests/file_transfer/create_upload.rs new file mode 100644 index 0000000000..3bf007d02c --- /dev/null +++ b/e2e/suites/conformance/cli/tests/file_transfer/create_upload.rs @@ -0,0 +1,166 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Stories for `sandbox create --upload`. +//! +//! Preconditions: the target supplies the default sandbox image. Each story +//! writes its upload sources to a local temporary directory. +//! +//! Permissions: an identity allowed to create, execute in, and delete sandboxes. +//! +//! Side effects: each story creates one sandbox named from the run id and +//! deletes it. +//! +//! Known limitation: `--upload` cannot be combined with a command, so stories +//! check the files once `create` returns, not that they arrived before a main +//! process started. + +use super::helpers::{create_with_uploads, delete_sandbox, expect_stdout, fs_error}; +use openshell_e2e_support::OpenShellRunner; +use std::fs; + +/// Verify `sandbox create --upload :` keeps the +/// directory's basename under the destination. +#[tokio::test] +async fn create_with_upload_directory_preserves_source_basename() { + let mut runner = OpenShellRunner::from_env("file-transfer/create-upload-directory") + .expect("candidate openshell CLI is available"); + let result = async { + runner.check_gateway_status().await?; + let runner = &mut runner; + let local = tempfile::tempdir().map_err(fs_error("create temporary directory"))?; + let source = local.path().join("project"); + fs::create_dir_all(source.join("src")).map_err(fs_error("create project/src"))?; + fs::write(source.join("marker.txt"), "upload-create-marker") + .map_err(fs_error("write marker.txt"))?; + fs::write(source.join("src/main.py"), "print('hello')") + .map_err(fs_error("write main.py"))?; + + let sandbox_name = format!("ct-{}-cu1", runner.id()); + create_with_uploads( + runner, + "create-upload-directory/create", + &sandbox_name, + &[(&source, "/sandbox/data")], + ) + .await?; + + expect_stdout( + runner, + &sandbox_name, + "create-upload-directory/marker", + "cat /sandbox/data/project/marker.txt", + "upload-create-marker", + ) + .await?; + expect_stdout( + runner, + &sandbox_name, + "create-upload-directory/nested", + "cat /sandbox/data/project/src/main.py", + "print('hello')", + ) + .await?; + expect_stdout( + runner, + &sandbox_name, + "create-upload-directory/not-flattened", + "test ! -e /sandbox/data/marker.txt && echo absent", + "absent\n", + ) + .await?; + delete_sandbox(runner, &sandbox_name).await + } + .await; + if let Err(error) = runner.finish(result).await { + panic!("file-transfer/create-upload-directory conformance story failed:\n{error}"); + } +} + +/// Verify several `--upload` flags on one `sandbox create` all place their +/// files at their own destinations. +#[tokio::test] +async fn create_with_multiple_uploads() { + let mut runner = OpenShellRunner::from_env("file-transfer/create-upload-multiple") + .expect("candidate openshell CLI is available"); + let result = async { + runner.check_gateway_status().await?; + let runner = &mut runner; + let local = tempfile::tempdir().map_err(fs_error("create temporary directory"))?; + let alpha = local.path().join("alpha"); + let beta = local.path().join("beta"); + fs::create_dir_all(&alpha).map_err(fs_error("create alpha"))?; + fs::create_dir_all(&beta).map_err(fs_error("create beta"))?; + fs::write(alpha.join("a.txt"), "content-alpha").map_err(fs_error("write a.txt"))?; + fs::write(beta.join("b.txt"), "content-beta").map_err(fs_error("write b.txt"))?; + + let sandbox_name = format!("ct-{}-cu2", runner.id()); + create_with_uploads( + runner, + "create-upload-multiple/create", + &sandbox_name, + &[(&alpha, "/sandbox/alpha"), (&beta, "/sandbox/beta")], + ) + .await?; + + expect_stdout( + runner, + &sandbox_name, + "create-upload-multiple/alpha", + "cat /sandbox/alpha/alpha/a.txt", + "content-alpha", + ) + .await?; + expect_stdout( + runner, + &sandbox_name, + "create-upload-multiple/beta", + "cat /sandbox/beta/beta/b.txt", + "content-beta", + ) + .await?; + delete_sandbox(runner, &sandbox_name).await + } + .await; + if let Err(error) = runner.finish(result).await { + panic!("file-transfer/create-upload-multiple conformance story failed:\n{error}"); + } +} + +/// Verify `sandbox create --upload :` places a single file +/// at the destination. +#[tokio::test] +async fn create_with_upload_single_file() { + let mut runner = OpenShellRunner::from_env("file-transfer/create-upload-file") + .expect("candidate openshell CLI is available"); + let result = async { + runner.check_gateway_status().await?; + let runner = &mut runner; + let local = tempfile::tempdir().map_err(fs_error("create temporary directory"))?; + let source = local.path().join("config.txt"); + fs::write(&source, "single-file-upload-test").map_err(fs_error("write config.txt"))?; + + let sandbox_name = format!("ct-{}-cu3", runner.id()); + create_with_uploads( + runner, + "create-upload-file/create", + &sandbox_name, + &[(&source, "/sandbox")], + ) + .await?; + + expect_stdout( + runner, + &sandbox_name, + "create-upload-file/read", + "cat /sandbox/config.txt", + "single-file-upload-test", + ) + .await?; + delete_sandbox(runner, &sandbox_name).await + } + .await; + if let Err(error) = runner.finish(result).await { + panic!("file-transfer/create-upload-file conformance story failed:\n{error}"); + } +} diff --git a/e2e/suites/conformance/cli/tests/file_transfer/helpers.rs b/e2e/suites/conformance/cli/tests/file_transfer/helpers.rs index 2bffcf5da1..86b6534b6f 100644 --- a/e2e/suites/conformance/cli/tests/file_transfer/helpers.rs +++ b/e2e/suites/conformance/cli/tests/file_transfer/helpers.rs @@ -155,6 +155,71 @@ pub async fn exec( result.require_success() } +/// Create a detached sandbox with one `--upload :` flag +/// per entry. `--upload` cannot be combined with a command, so stories verify +/// the files with `sandbox exec` once the sandbox is created. +pub async fn create_with_uploads( + runner: &mut OpenShellRunner, + step: &str, + sandbox_name: &str, + uploads: &[(&Path, &str)], +) -> Result<(), String> { + runner.track_sandbox(sandbox_name); + let mut args = vec![ + "sandbox".to_string(), + "create".to_string(), + "--name".to_string(), + sandbox_name.to_string(), + "--detach".to_string(), + ]; + for (source, destination) in uploads { + let source = source + .to_str() + .ok_or_else(|| format!("local upload path is not UTF-8: {}", source.display()))?; + args.push("--upload".to_string()); + args.push(format!("{source}:{destination}")); + } + let args: Vec<&str> = args.iter().map(String::as_str).collect(); + + let result = runner + .step(step) + .description(format!("sandbox '{sandbox_name}' is created with uploads")) + .with_timeout(CREATE_TIMEOUT) + .run(&args) + .await + .map_err(|error| error.to_string())?; + result.require_success() +} + +/// Run `script` in the sandbox and require its standard output to equal +/// `expected` exactly. +pub async fn expect_stdout( + runner: &OpenShellRunner, + sandbox: &str, + step: &str, + script: &str, + expected: &str, +) -> Result<(), String> { + let result = runner + .step(step) + .description(format!("sandbox '{sandbox}' prints the expected output")) + .with_timeout(COMMAND_TIMEOUT) + .run(&[ + "sandbox", "exec", "--name", sandbox, "--no-tty", "--", "sh", "-c", script, + ]) + .await + .map_err(|error| error.to_string())?; + result.require_success()?; + if result.stdout() == expected { + Ok(()) + } else { + Err(result.failure_diagnostic(&format!( + "standard output equals {expected:?}, received {:?}", + result.stdout() + ))) + } +} + pub async fn git_init(repository: &Path) -> Result<(), String> { git(repository, &["init", "--quiet"]).await } diff --git a/e2e/suites/conformance/cli/tests/file_transfer/main.rs b/e2e/suites/conformance/cli/tests/file_transfer/main.rs index ccf0da9468..0539398787 100644 --- a/e2e/suites/conformance/cli/tests/file_transfer/main.rs +++ b/e2e/suites/conformance/cli/tests/file_transfer/main.rs @@ -3,6 +3,7 @@ //! File transfer conformance stories. +mod create_upload; mod git_filtering; mod helpers; mod path_safety;