Skip to content

test(conformance): migrate general-conformance e2e tests - #4246

Open
politerealism wants to merge 7 commits into
NVIDIA:mainfrom
politerealism:test/3954-migrate-conformance-tests/politerealism
Open

test(conformance): migrate general-conformance e2e tests#4246
politerealism wants to merge 7 commits into
NVIDIA:mainfrom
politerealism:test/3954-migrate-conformance-tests/politerealism

Conversation

@politerealism

@politerealism politerealism commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrate five general-conformance e2e test files out of the Podman-specific
e2e/rust harness into tests/suites/conformance/, following the
OpenShellRunner/Scenario pattern RFC 0016 (#3460) establishes. These were
identified as general-conformance candidates — driver-independent, CLI/API-only,
no engine shell-outs — in a research pass on #3460. Coordinated with @elezar
before doing this partial migration; he's independently migrating a different
subset of files from the same candidate list (provider_refresh_handles.rs,
websocket_conformance.rs, sandbox_labels.rs needs no change).

Related Issue

Contributes to #3954

Changes

  • workspace_lifecycle.rs → workspace-lifecycle/workspace-terminating scenarios
  • upload_create.rs → file-transfer/create-upload scenario, grouped with the
    existing file-transfer family rather than left scattered
  • provider_auto_create.rs → provider-auto-create scenario
  • sandbox_templates.rs → 4 scenarios (lifecycle, get-after-delete,
    duplicate-name, missing-template)
  • settings_management.rs → settings-management scenario
  • Removed the corresponding old e2e/rust/tests/*.rs files and their
    Cargo.toml/tests/artifacts.nix entries
  • Added CommandResult::output_contains/output_contains_ignore_case/
    require_outcome to the shared openshell-conformance crate, consolidating
    several duplicate helper implementations the migration introduced across
    the new scenario files

Not included: port_forward.rs doesn't fit the conformance harness's CLI
request/response model (needs a long-lived background SSH-tunnel process plus
raw socket I/O) — held back pending a design question for @elezar on whether
that pattern belongs on OpenShellRunner or as a scenario-local bypass.

Found but not fixed here (pre-existing, out of scope): two already-landed
file_transfer.rs scenarios (round-trip, git-filtering) currently fail
against main on code this PR doesn't touch — flagged to @elezar separately.

Testing

  • All five migrated scenarios (9 tests total) verified passing against an
    ephemeral Podman gateway, across two rounds of code review and fixes
  • cargo fmt --all -- --check passes
  • cargo check on the affected crates and the full e2e/rust test crate
    (with e2e, e2e-podman, e2e-host-gateway, e2e-local-container-driver
    features) passes clean, no dangling references to removed helpers
  • Unit tests added/updated — this is the test migration itself
  • E2E tests added/updated — N/A, this moves existing e2e-equivalent
    coverage into the conformance suite rather than adding new coverage

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable) — not applicable, no architecture change

@politerealism
test(conformance): migrate workspace_lifecycle to the conformance suite
b3b16f0 
Port the workspace and workspace-scoped provider CRUD/isolation/deletion-guard
coverage from e2e/rust/tests/workspace_lifecycle.rs into two driver-agnostic
scenarios (workspace-lifecycle, workspace-terminating) under
tests/suites/conformance/, following the OpenShellRunner/Scenario pattern
established for sandbox_lifecycle. Remove the old per-driver test file and its
Cargo.toml and tests/artifacts.nix follow-up-exclusion entries.

Normalizes miette's line-wrapped diagnostic text before substring-matching
error messages; the original test's exact-match assertions were fragile to
the terminal width the CLI happened to detect when rendering errors.

Verified against an ephemeral Podman gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
test(conformance): migrate upload_create into file-transfer conformance
398dfe3 
Port sandbox create --upload coverage from e2e/rust/tests/upload_create.rs
into a new file-transfer/create-upload scenario in
crates/openshell-conformance/src/scenarios/file_transfer.rs, grouped
alongside the other file/template-transfer conformance scenarios rather than
left scattered in the Podman-specific e2e suite. Wire it into the
FILE_TRANSFER_SCENARIO chain and the conformance CLI test crate, and remove
the old per-driver test file (picked up by Cargo's default test
auto-discovery; it had no Cargo.toml or artifacts.nix entries to clean up).

Verified against an ephemeral Podman gateway: the new create-upload scenario
and the existing path-safety scenario pass. round-trip and git-filtering
already fail against current main on code this change does not touch (the
CLI doesn't warn/reject non-Git-tree uploads the way those scenarios assert)
-- pre-existing, out of scope here.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
test(conformance): migrate provider_auto_create to the conformance suite
ca17121 
Port --provider claude-code --auto-providers credential-placeholder-injection
coverage from e2e/rust/tests/provider_auto_create.rs into a new
provider-auto-create scenario under tests/suites/conformance/, following the
OpenShellRunner/Scenario pattern. Preserve the original's existence-check
skip and defensive pre-cleanup, since "claude-code" is a recognized
auto-provider type name rather than a value this scenario can scope per run.

Remove the old per-driver test file (no Cargo.toml entry; picked up by
Cargo's default test auto-discovery) and its tests/artifacts.nix follow-up
exclusion entry.

Verified against an ephemeral Podman gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
test(conformance): migrate sandbox_templates to the conformance suite
617b8bc 
Port reusable sandbox workload template CRUD and template-backed sandbox
creation coverage from e2e/rust/tests/sandbox_templates.rs into four
driver-agnostic scenarios (sandbox-templates/lifecycle, get-after-delete,
duplicate-name, missing-template) under tests/suites/conformance/, following
the OpenShellRunner/Scenario pattern. Remove the old per-driver test file and
its Cargo.toml entry.

Verified against an ephemeral Podman gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
test(conformance): migrate settings_management to the conformance suite
c49bcaa 
Port sandbox/global settings precedence, override, and delete-semantics
coverage from e2e/rust/tests/settings_management.rs into a new
settings-management scenario under tests/suites/conformance/, following the
OpenShellRunner/Scenario pattern. Matches the RFC's own "Settings" conformance
area (read/change/remove settings through public APIs; global overrides take
precedence and prevent conflicting sandbox changes).

"ocsf_json_enabled" is a fixed, recognized settings key rather than a name
this scenario can scope per run, so it keeps the original's hard-asserted
pre-cleanup and best-effort post-cleanup around it instead of per-run tracked
resource names.

Remove the old per-driver test file (no Cargo.toml or artifacts.nix entries;
picked up by Cargo's default test auto-discovery).

Verified against an ephemeral Podman gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
fix(conformance): address code-review findings on the migrated scenarios
f29ff75 
Code review of the five migrated scenarios found several issues:

- provider_auto_create.rs dropped the original e2e test's --policy fixture,
  which avoids the default image's network rules conflicting with the
  claude-code credential provider's startup validation. Restored it, minus
  the original's run_as_user/run_as_group section (that required a pinned
  test image the conformance suite has no equivalent for, and isn't needed
  without it).
- Discovered along the way: provider deletion can briefly fail with
  "attached to sandbox(es)" right after the sandbox delete command returns,
  since sandbox deletion isn't synchronous with detaching its providers.
  Replaced generic tracked-resource cleanup with explicit ordered cleanup
  (delete sandbox, then poll-retry the provider delete).
- settings_management.rs's "is managed" assertion lacked the miette
  line-wrap normalization added for workspace_lifecycle.rs's equivalent
  check, and had a dead, already-enforced success check after a
  settings_delete(expect_success: false) call. Fixed both.
- Consolidated four inconsistent copies of output_contains (plain
  case-sensitive, lowercased, miette-wrap-normalized) across the new
  scenario files into two shared CommandResult methods (output_contains,
  output_contains_ignore_case) on the shared crate, so the wrap-normalization
  fix applies uniformly instead of needing to be re-added per file.
- provider_auto_create.rs hardcoded "claude-code" in one assertion instead
  of interpolating the PROVIDER_NAME constant used everywhere else.

One review-flagged claim was investigated and refuted: that the migrated
file-transfer create-upload scenario weakened an upload-before-entrypoint
ordering guarantee. Verified against the actual deleted harness source
(e2e/rust/src/harness/sandbox.rs's create_with_uploads) that the original
never embedded the verification command as the sandbox's entrypoint either
-- it always created detached, then execed separately, same as the
migrated version.

Re-verified all five scenarios (8 tests) against a fresh Podman gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
fix(conformance): address second-round code-review findings
b3a07e7 
- settings_management.rs's wait_for_ready dropped sandbox_lifecycle.rs's
  defensive name-mismatch check when its JSON polling loop was written.
  Added it back (and the missing `name` field on SandboxState) so a
  wrong-sandbox response fails fast with a clear diagnostic instead of
  silently comparing the wrong resource's phase.
- provider_auto_create.rs's cleanup poll used a 30s overall retry budget
  with a 2-minute per-attempt command timeout -- a single slow attempt
  could exhaust the whole budget before a retry got a chance. Widened the
  retry budget to comfortably exceed the per-attempt ceiling.
- provider_auto_create.rs's sandbox was no longer tracked via
  runner.track_sandbox() after switching to explicit ordered cleanup in the
  prior commit, losing the Drop-impl warning and finish() safety net if a
  panic skips the explicit cleanup. Re-added tracking (forgotten once
  explicit cleanup completes) alongside the explicit poll-retried delete,
  which is still what actually avoids the sandbox/provider detach race.
- Added CommandResult::require_outcome(expect_success) to the shared crate
  and switched workspace_lifecycle.rs's run_cli and settings_management.rs's
  settings_delete to use it instead of two independent, byte-for-byte
  identical inline implementations.
- Consolidated settings_management.rs's require_setting_line and
  require_setting_line_with_scope into one function taking an
  Option<&str> scope.

Re-verified all four affected scenarios (8 tests) against a fresh Podman
gateway.

Signed-off-by: politerealism <burdcat17@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@politerealism