Repository navigation
test(conformance): migrate create-upload tests to testkit - #4606
Open
politerealism wants to merge 2 commits into
Open
politerealism wants to merge 2 commits into
politerealism wants to merge 2 commits into
Conversation
politerealism
marked this pull request as ready for review
October 15, 2026 16:36
4 of 7 tasks
6 tasks done
|
This pull request has had no activity for 14 days and is now marked stale. |
Give `ExecOutput` the same `success()` check that `CliOutput` has, and record the command so the failure message names it. Add `Sandbox::sh` to run a shell script with `sh -c`. Suites hand-write the exit-code check and the `sh -c` wrapper after every `exec`; these let them drop that boilerplate. Signed-off-by: Evan Lezar <elezar@nvidia.com>
Move the three `sandbox create --upload` tests from the legacy e2e crate into the testkit conformance suite. The behavior is not driver-specific, so it 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 test, and that the directory is not flattened into the destination. - Check each destination in the multiple-uploads test 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. - Drop the claim that files arrive before the command runs. The CLI rejects `--upload` together with a command, so each test 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 NVIDIA#3954 Co-authored-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: politerealism <burdcat17@gmail.com>
elezar
force-pushed
the
test/3954-create-upload-stories/politerealism
branch
from
November 3, 2026 17:11
7d181e5 to
613b95b
Compare
elezar
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
November 3, 2026 17:11
Maintainer list changeGains approval rights: Confirm every change is intended. Anyone listed here can single-handedly satisfy |
Member
|
/ok to test 613b95b |
|
Label |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Move the three
sandbox create --uploadtests from the legacy e2e crate into the testkit conformance suite. The behavior isn't driver-specific, so it runs against any prepared gateway. AddExecOutput::successandSandbox::shto testkit so the tests don't hand-write the exit-code check and thesh -cwrapper.Related Issue
Contributes to #3954. No separate issue: this migrates existing coverage.
Changes
ExecOutput::success(), matchingCliOutput::success(), and a publiccommandfield so the failure message names the command. AddSandbox::sh(script), which runssh -c.tests/suites/conformance/tests/create_upload.rswith three tests, keeping the original names so filters keep working:create_with_upload_directory_preserves_source_basename,create_with_multiple_uploadsandcreate_with_upload_single_file.Sandbox::shand comparing exact content. The originals matched the printed output ofsandbox create.--no-git-ignore, which isn't needed outside a Git work tree and hid the default path.--uploadtogether with a command, so each test checks the files oncecreatereturns, and the module doc notes the limitation.e2e/rust/tests/upload_create.rsand the now-unusedSandboxGuard::create_with_uploadandcreate_with_uploadsfrom the legacy harness.create-uploadrow to the conformance README.mainhas no end-to-end coverage ofsandbox create --uploadoutside the legacy test this PR removes.crates/openshell-clihas integration tests for the planning and error paths, but they stub the gateway and SSH transport.tests/suites/conformance/tests/file_transfer.rscovers the separatesandbox uploadcommand. None of them check directory basenames, multiple uploads or a single-file upload on a real sandbox.Testing
mise run pre-commitpasses on both commitscargo clippy --testsis clean intests/suites/conformancetest:e2e). Not run yet.Checklist