Skip to content

test(conformance): run conformance suites from test plans in tmachine environments - #4629

Draft
elezar wants to merge 18 commits into
mainfrom
test/3954-conformance-test-plans/elezar
Draft

elezar wants to merge 18 commits into
mainfrom
test/3954-conformance-test-plans/elezar

Conversation

@elezar

@elezar elezar commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds a test plan runner that runs SDK-driven e2e suites inside isolated tmachine VMs:

nix run .#test-runner -- test --plan tests/plans/cpu-tests.yaml

It starts as a vertical slice and now replaces the tmachine CLI conformance run: every test in tests/suites/conformance/cli is ported to testkit (12 tests in three suites), the plan covers ten environments (Docker, k3s, HA k3s, rootful and rootless Podman, on amd64 and arm64), and CI's conformance-integration lanes run through the test runner.

Related Issue

Part of #3954. Related to RFC 0016 (#3460). Draft for discussion; it does not close an issue.

Changes

testkit (tests/testkit/, a separate workspace shared by every e2e family)

  • testkit: black-box test support over the Rust SDK.
    • Tests read the system under test from a target file named by OPENSHELL_TEST_TARGET.
    • mTLS targets get a tonic channel that testkit builds and passes to OpenShellClient::from_parts, since the SDK does not speak mTLS.
    • Context tracks every resource a test creates (named test-<run>-<suffix>) and cleans it up on pass, failure, or panic.
    • Target::wait_ready retries a health check until the gateway reports healthy; the testkit-ready binary wraps it. Tests themselves connect once and fail fast.
    • Errors include their sources, so a connect failure says why instead of only "transport error".
    • cx.cli([...]) drives the CLI the environment installed, against the target's gateway, for behavior that exists only in the CLI (file upload and download). The target's cli: section names it; the CLI is part of every target, not a capability, and testkit-ready checks that openshell status reports the gateway connected.
    • Sandbox helpers: stop, start, wait_phase, wait_finished, get, plus helpers over the generated gRPC client for API the SDK doesn't wrap: create with a policy, sandbox settings, effective config, draft policy.
  • #[testkit::test] (testkit-macros):
    • Every test has a required, stable ID (id = "t-xxxxxxxx", random). It never changes or gets reused, including across renames and moves between suites and families. A missing ID's compile error suggests a fresh one.
    • The function name is the nextest test name, used for selection. The doc comment is the required description.
    • requires = [...] names target capabilities. A test whose requirement is not met fails rather than passing.
  • testkit-spec: the single parser for #[testkit::test], shared by the macro and the runner's catalog.

Conformance suites (tests/suites/conformance/)

  • Conformance covers every public client interface: the API through the SDK, and the installed CLI where a promise exists only there. Tests assert only what the public contract (proto comments and published docs) promises, using phase and exit code rather than condition reasons.
  • sandbox-lifecycle: long_running_exec_delete, run_to_completion_exit_delete, default_command_sandbox_is_listed_ready, restart_stopped_sandbox, stopped_sandbox_keeps_workspace (requires = [persistent_workspace]), stopped_sandbox_can_be_deleted.
  • file-transfer (through the CLI): upload_download_round_trip, upload_respects_gitignore, download_stays_inside_workspace.
  • policy-advisor: policy_local_accepts_proposals, denied_address_produces_draft_rule, denied_hostname_produces_draft_rule.
  • These replace the nine tests in tests/suites/conformance/cli for the tmachine and CI path. That CLI suite and crates/openshell-conformance stay for the developer mise run e2e scripts until those move.

tmachine (tests/tmachine/)

  • A library target exposing Session: up, exec, copy_in, copy_out, collect_diagnostics, shell, and down, over password-only SSH.
  • A dropped session cannot leak a VM.
  • tmachine test is rebuilt on Session; the CLI is unchanged.
  • Firmware paths can be overridden at runtime.

Installers

  • The openshell_client role writes /etc/openshell-test/gateway.yaml: the gateway endpoint (plus mTLS paths when present) and the installed CLI with the gateway name it registered. Every installer registers its gateway through this role.
  • Every tmachine environment installs git during setup: the CLI's upload and the file-transfer tests need it, and Fedora cloud images lack it.

Runner (tests/runner/, nix run .#test-runner)

  • Plans (tests/plans/) and environment definitions (tests/environments/) are validated before anything is built or booted. cpu-tests runs every conformance suite on ten environments: Ubuntu Docker (deb), k3s and HA k3s, and Fedora rootful and rootless Podman (rpm), each on amd64 and arm64.
  • The catalog is read from source; the runner never executes test binaries on the host.
  • Jobs are grouped by environment, so each environment is booted once.
  • Test IDs must be unique across all families, and each JUnit test case gets a test_id property.
  • matrix prints every test against every environment: where it runs, or why not (missing capability, waiver, or with --host, this machine). A test is expected wherever its required capabilities are met; nobody lists combinations, and plans only select.
  • matrix and test fail when a test runs in no defined environment, or when tests require a capability no environment declares.
  • tests/waivers.yaml skips a test where a known bug breaks it: test ID, environment selector, tracking issue, and reason. Waived tests are skipped in every plan and listed in the run summary; --ignore-waivers runs them anyway.
  • Tests whose required capabilities are missing, and environments the host cannot boot, are reported as unsupported.
  • Each family is cross-built into a nextest archive bundle, checked against the catalog, and shipped with testkit-ready. Suites are selected by package and binary (binary_id). In the VM, the runner waits up to five minutes for the gateway to report healthy and for the CLI to reach it, then runs the suites against a target combining the installer's sections with the environment's capabilities. Either check failing is an environment error, not a test failure.
  • JUnit, the nextest log, and gateway diagnostics on failure are written to artifacts/test-results/<plan>/<env>/<family>/ (or --results).
  • Flags: --dry-run, --env, --suite, --keep, --results, and --ignore-waivers. A catalog subcommand prints every suite and test.

CI

  • e2e-suites.yml: a reusable matrix job over {plan, runner} entries, each optionally narrowed to one environment, uploading e2e-results-<plan>-<environment>.
  • conformance-integration in Branch E2E, Release Dev, and Release Qualification now runs cpu-tests through e2e-suites.yml, one job per amd64 environment, replacing the tmachine CLI conformance run. The job keeps its name, so core-e2e-result and the required OpenShell / E2E status still depend on it.
  • Branch Checks and the mise tasks now format, lint, and test tests/testkit, tests/runner, and tests/tmachine. Nothing ran their unit tests before.

Lockfiles

  • check_cargo_lockfile_alignment.py requires listed workspaces to lock crates they share with the root lockfile at root versions. --locked only compares a lockfile with its own manifest, so it cannot catch this.
  • tests/suites/conformance and tests/testkit are enrolled and locked from the root lockfile. This fixes an aws-lc-sys 0.46 pickup that does not link against Nix's AWS-LC.
  • The check runs in rust:lockfiles:check and the Cargo lockfiles CI job.
  • Test archives and the runner build with --locked.

Layout: everything new lives under tests/ (suites, testkit, runner, environments, plans, waivers, provisioning). e2e/ is the existing harness, being migrated into tests/; it only takes fixes until it is empty. TESTING.md and AGENTS.md state the rule. This departs from approved RFC 0016, which put suites in e2e/suites/; an amendment to #3460 follows.

Docs: TESTING.md covers test plans, the expected matrix and waivers, testkit (including the CLI driver and the CLI in the target), authoring conformance tests (IDs and what a test may assert), the readiness wait, the tests/ layout rule, and lockfile alignment. CI.md describes the conformance lanes. AGENTS.md points new end-to-end tests at tests/suites/.

Testing

  • Checks appropriate to the affected code and behavior pass:
    • fmt, clippy, and nextest (--profile ci) for testkit, the runner, and tmachine in the Nix dev shell
    • pre-commit rust:lint and rust:lockfiles:check
    • ruff for the alignment script; actionlint and zizmor for the workflows
    • x86_64 musl cross-build of testkit, testkit-ready, and the conformance archive in the Nix dev shell
    • nix build .#tmachine .#test-runner
  • Unit tests added/updated:
    • testkit, macros, and spec: 30 tests
    • runner: 31 tests
    • tmachine: 3 tests
    • lockfile alignment: 4 tests
  • E2E tests added/updated: nix run .#test-runner -- test --plan tests/plans/cpu-tests.yaml on an arm64 macOS host, 12/12 in each of:
    • tmachine.ubuntu2404-arm64.docker-rootful.deb (packaged mTLS gateway, including a run booted from a cached install layer; test_id in the JUnit report)
    • tmachine.ubuntu2404-arm64.k3s.k3s and tmachine.ubuntu2404-arm64.k3s.k3s-ha
    • tmachine.fedora44-arm64.podman-rootful.rpm and tmachine.fedora44-arm64.podman-rootless.rpm, with RPMs from the local RPM build in feat(tmachine): build local RPM artifacts #4651 (not part of this PR)
    • A temporary waiver on run_to_completion_exit_delete for arm64: excluded from the nextest filter, counted as waived, and listed with its issue; the other test passed.
    • amd64: reported unsupported on this host. The five amd64 environments run in CI's conformance lanes.
  • Existing nix run .#tmachine -- test ubuntu-docker-rootful deb conformance still passes with the rebuilt tmachine CLI.

Not covered yet:

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

Add a testkit for black-box conformance tests driven through the Rust SDK,
and a first sandbox-lifecycle suite built on it.

- testkit loads the system under test from a target file named by
  OPENSHELL_TEST_TARGET (gateway endpoint, TLS or mTLS material, bearer
  token, capabilities). mTLS targets get a tonic channel built by testkit
  and passed to OpenShellClient::from_parts, since the SDK does not speak
  mTLS.
- Context tracks every sandbox a test creates and deletes them after the
  test, including on failure or panic; cleanup failures are reported
  separately.
- #[testkit::test] wraps an async fn in a same-named #[tokio::test]. The
  doc comment is the required description, the function name is the test
  ID, and requires = [...] names target capabilities. A test whose
  requirement the target lacks fails rather than passes.
- sandbox-lifecycle adds long_running_exec_delete: create, wait for Ready,
  exec, delete, and confirm the sandbox is gone from get and list.

Pin aws-lc-rs 1.18.0 and aws-lc-sys 0.44.0 to the root workspace versions
so the musl test archive links against the Nix-provided AWS-LC.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
…tions

Give the conformance test runner a way to act on a running test machine,
and describe the environments it can run in.

tmachine:
- Add a library target exposing Session: up (set up and install from
  cached disks, boot a throwaway overlay, wait for SSH), exec, copy_in,
  copy_out, collect_diagnostics, shell, run_playbooks, and down. QEMU stays
  a child of the session, so a dropped session cannot leak a VM.
- Rebuild `tmachine test` on Session; the CLI is unchanged.
- Use password-only SSH so keys from the caller's agent or ~/.ssh cannot
  exhaust the guest's authentication attempts.
- Let TMACHINE_FIRMWARE_CODE/VARS be set at runtime, falling back to the
  compiled-in paths, so library users built outside Nix can boot ARM guests.

Ansible:
- openshell_client writes /etc/openshell-test/gateway.yaml describing the
  registered gateway (endpoint, plus mTLS paths when client credentials
  exist). The runner combines it with environment capabilities into the
  target file tests read.

Runner:
- Add tests/runner with the environment definition loader and
  tests/environments/docker.yaml (Ubuntu 24.04 Docker rootful deb, amd64
  and arm64). IDs must match their fields, tmachine environments must name
  their tmachine environment and installer, and an architecture the host
  cannot boot is reported as unsupported.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Add the openshell-test runner, which runs a test plan end to end:

  nix run .#test-runner -- test --plan tests/plans/cpu-tests.yaml

- Plans (tests/plans/*.yaml) select families, suites, and environments.
  Unknown families, suites, environments, and fields fail before anything
  is built or booted.
- The catalog is read from source: each tests/*.rs file of a family
  workspace containing #[testkit::test] functions is a suite. Parsing moves
  into testkit-spec, shared by the macro and the runner, so they cannot
  disagree about a test's ID, description, or requirements.
- Jobs are grouped by environment so each one is provisioned once. Tests
  whose requirements the environment lacks are excluded by exact name and
  reported as unsupported, as are environments the host cannot boot.
- Each family is cross-built into a nextest archive bundle for the guest,
  checked against the catalog, run in a tmachine session against the target
  formed from the installer's gateway section and the environment's
  capabilities, and its JUnit report, log, and (on failure) gateway
  diagnostics are copied to artifacts/test-results/<plan>/<env>/<family>/.
- --dry-run prints resolved jobs and filtersets; --env and --suite narrow a
  plan; --keep leaves each VM up after its tests; `catalog` prints every
  suite and test as JSON.
- The test-runner Nix package provides the cross toolchains, QEMU, Ansible,
  firmware, and tmachine configuration.

Document test plans and conformance suites in TESTING.md.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
`cargo --locked` only checks a lockfile against its own manifest. The
conformance suites build openshell-sdk from a separate workspace with its
own Cargo.lock, so the two lockfiles could resolve shared crates to
different versions without any check failing. That is how aws-lc-sys 0.46
was picked up, which does not link against the AWS-LC Nix provides.

- Add check_cargo_lockfile_alignment.py: aligned workspaces must lock every
  registry crate they share with the root at a version the root lockfile
  has. `--fix` re-locks a workspace from the root lockfile, which keeps
  coupled crate families such as futures-* consistent.
- Enroll tests/suites/conformance and re-lock it from the root lockfile.
  The other workspaces that path-depend on root crates also drift and can
  be enrolled once aligned.
- Run the check in rust:lockfiles:check and the Cargo lockfiles CI job,
  and its tests in `mise run test`.
- Build test archives and run the test runner with --locked so a stale
  lockfile fails instead of being re-resolved.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

@elezar

elezar commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test a37ebe9

firmware_path() was compiled only on aarch64, but command() calls it on
every architecture, so tmachine and the test runner failed to build on
x86. Drop the architecture guard; the runtime override is harmless there.

Resolve a relative --results directory before load_tmachine() changes
into tests/, so all of a run's artifacts land in one place.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
testkit is not specific to conformance: the target file, gateway
connection, sandbox cleanup and the #[testkit::test] metadata apply to
every test family, and the runner already reached into the conformance
workspace for testkit-spec.

- Move testkit, testkit-macros and testkit-spec to tests/testkit, a
  separate workspace like tests/runner and tests/tmachine. Suites and the
  runner depend on it by path.
- Enroll tests/testkit in the lockfile alignment check and seed its
  lockfile from the root lockfile.
- Make the wording family-neutral and name sandboxes test-<run>-<suffix>
  instead of ct-<run>-<suffix>.
- Describe testkit and its location in TESTING.md.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
A tmachine session booted from a cached install layer restarts the
gateway, and the runner started nextest as soon as SSH answered. The
first test then failed to connect before the gateway listener was bound.

- Add Target::wait_ready to testkit, which retries a health check until
  the gateway reports healthy, and a testkit-ready binary that runs it.
- Build testkit-ready for the guest, ship it in each bundle, and run it
  once per environment before nextest. A gateway that never becomes
  healthy is an environment error, not a test failure.
- Include error sources in testkit errors, so a connect failure says why
  instead of only "transport error".

Signed-off-by: Evan Lezar <elezar@nvidia.com>
- Add e2e-suites.yml, a reusable matrix job that runs the e2e suites a
  plan in tests/plans/ selects, on each runner, and uploads the JUnit
  reports and diagnostics. Branch E2E runs it for cpu-tests on
  ubuntu-24.04 with the other integration jobs. It is advisory and not
  part of core-e2e-result until it has a passing history on amd64.
- Format, lint and test tests/testkit, tests/runner and tests/tmachine in
  Branch Checks and the matching mise tasks. Nothing ran their unit tests
  before.
- Describe the job in CI.md.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar

elezar commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test cf72f11

@elezar elezar changed the title test(conformance): run SDK-driven conformance test plans in tmachine environments test(conformance): run SDK-driven e2e suites from test plans in tmachine environments Oct 16, 2026
@elezar elezar added the test:e2e Requires end-to-end coverage label Oct 16, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for cf72f11. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

Cover a sandbox whose main command exits 0 under restart policy Never:
it reaches Completed with exit code 0, is not restarted, and is gone
from get and list after delete. Assertions use the phase and exit code
from the public contract, not condition reasons.

Add Sandbox::wait_finished and Sandbox::get to testkit.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Test names change as the contract is better understood, which breaks
result history and anything that refers to a test. Give every test a
required ID, `t-` plus eight random lowercase letters or digits, that
never changes or is reused, including across renames and moves between
suites and families. Random IDs avoid allocation conflicts between
parallel changes.

- testkit-spec parses and validates `id`; a missing or malformed ID is
  a compile error that suggests a fresh one.
- The function name becomes `name` and stays the nextest test name used
  for selection.
- The runner's catalog rejects duplicate IDs across all families and
  adds each test's ID to its JUnit test case as a `test_id` property.
- Assign IDs to the existing sandbox-lifecycle tests and document the
  rule in TESTING.md.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Conformance tests assert only what the proto comments and published
docs promise, use the phase and exit code rather than condition reasons,
check negatives without sleeping, and gate driver-dependent behavior on
a capability. Also correct which selectors use the test name.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
A test is expected in every environment that provides the capabilities
it requires; nobody lists combinations, and plans only select from the
result.

- `matrix` prints every test against every environment: where it runs,
  or why not (missing capability, waiver, or with --host, this machine).
- `matrix` and `test` fail when a test runs in no defined environment,
  or when tests require a capability no environment declares.
- tests/waivers.yaml skips a test where a known bug breaks it. Each
  waiver names the test ID, an environment selector, the tracking issue
  and a reason. Waived tests are skipped in every plan and listed in the
  run summary; --ignore-waivers runs them anyway.
- Environment selectors match environments by their fields.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
tests/ is the new test surface: suites, their support library, the plan
runner, and provisioning. e2e/ holds the existing harness, which is
being migrated into tests/ and only takes fixes until it is empty.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Conformance covers every public client interface. Behavior that exists
only in the CLI, such as file upload and download, is tested through the
CLI the environment installed, alongside the SDK.

- testkit drives that CLI (`cx.cli`). The target gains a `cli` section,
  written by the openshell_client role, and testkit-ready checks that
  `openshell status` reports the gateway connected. The CLI is part of
  every target rather than a capability.
- Port the nine tests in tests/suites/conformance/cli: four lifecycle
  tests in sandbox-lifecycle, a CLI-driven file-transfer suite, and a
  policy-advisor suite using testkit helpers over the generated gRPC
  client for policies, settings, and draft rules.
- Add the persistent_workspace capability for workspace persistence
  across stop and start, which the docs leave to the driver.
- Filter suites by package and binary, since the CLI package has test
  binaries with the same names.
- Wait up to five minutes for the gateway, matching the installers.

The CLI suite stays for the developer e2e scripts.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Add Ubuntu K3s and HA K3s environments for both architectures, and
Fedora rootful and rootless Podman environments for amd64, where RPMs
are built. The cpu-tests plan runs every conformance suite on all of
them.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
conformance-integration in Branch E2E, Release Dev, and Release
Qualification now runs the cpu-tests plan through e2e-suites.yml, one
job per amd64 environment, replacing the tmachine CLI conformance run.
A matrix entry can name an environment to run on its own. The separate
advisory e2e-suites job is removed.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Release builds produce aarch64 RPMs, so Fedora with Podman is a supported arm64 combination. The environments run where arm64 RPMs are available; CI's conformance lanes stay on amd64.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar

elezar commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test bd8184a

The CLI's sandbox upload runs git to apply .gitignore rules, and the file-transfer conformance tests create Git work trees. The tmachine conformance playbook installed git before its tests; the test runner path has no such step, and Fedora cloud images lack git. Install it during environment setup so it is cached in the setup disk.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar changed the title test(conformance): run SDK-driven e2e suites from test plans in tmachine environments test(conformance): run conformance suites from test plans in tmachine environments Oct 16, 2026
@elezar

elezar commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test 6739288

@matthewgrossman matthewgrossman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. The direction is good: one shared parser for the macro and the catalog, a matrix where plans only select tests, capabilities that fail closed in testkit, and solid Context cleanup (sandboxes are registered before the create call, cleanup runs after catch_unwind, and a cleanup failure never hides the original failure).

My main concern is the one that hid Kubernetes coverage before: the runner validates declared metadata carefully, but nothing reconciles what actually ran against what was expected, and several paths exit 0 having run nothing. Inline comments cover the specifics. The points below don't anchor to changed lines.

Lane can pass with fewer tests (summary)

  1. Unsupported / NothingToRun / zero jobs exit 0. I reproduced --env tmachine.ubuntu2404-amd64.docker-rootful.deb on an arm64 host: UNSUPPORTED …, exit 0.
  2. Results aren't reconciled with expected IDs. nextest silently ignores a test(=name) that matches nothing, so a catalogued test missing from the binary just vanishes.
  3. The CI matrix hand-copies 5 of the plan's 10 environments, and nothing checks the two against each other. Planned arm64 coverage never runs and nothing says so.
  4. Removing a capability key from an environment drops tests silently. The test still runs somewhere, so matrix passes.

I'd suggest making a strict/CI mode, JUnit-vs-expected reconciliation in both directions, and either --emit-github-matrix or a plan-vs-workflow check prerequisites for the cutover rather than follow-ups. A checked-in golden expected test × environment matrix would make (4) visible in review.

Not anchorable inline

  • Coverage claims vs. reality. The old lane exercised the installed CLI for sandbox create --detach/--policy/--no-auto-providers -- cmd, get/list --output json (with paging), exec --no-tty, stop/start/delete, settings set/get, policy get --full, and rule get. Now only upload/download (plus status in readiness) go through the CLI. The underlying behaviour is still covered through the SDK, but release qualification no longer exercises the CLI artifact's parsing and JSON output. TESTING.md:259 still says the installed-artifact suite "runs the same tests". Either keep a CLI smoke test in the lane or call out the drop explicitly.
  • QemuVm::shutdown / wait (tests/tmachine/src/qemu/vm.rs:46-77) unwrap/assert! on the QMP socket and wait for ACPI power-off with no timeout. These were CLI-only before; now they sit on the runner's main path, so a dead or wedged QEMU panics or hangs the whole run, and later environments get no results. The same applies to unwrap/panic! in QemuImage::create, QemuVm::start, and firmware_path, which bypass the per-family Status::Error handling in execute.rs.
  • Signals. Error returns and panics are leak-free (kill_on_drop, correct field drop order). SIGTERM/SIGKILL (CI cancel, timeout) orphan QEMU: there's no PR_SET_PDEATHSIG, no own process group, and no signal handler in main.rs. Temp dirs and overlays leak in every signal case. "A dropped session cannot leak a VM" only holds when destructors run.
  • Lint gap. tests/suites/conformance/* isn't fmt- or clippy-checked in Branch Checks; only testkit, runner, and tmachine were added.
  • Drift. The old cli suite, crates/openshell-conformance, the conformance playbooks, the config.nix testsuites, and the integration-runner.yml default matrix no longer run in CI.

Possible review split

  1. Lockfile alignment script and the aws-lc-sys fix
  2. tmachine Session, the openshell_client target file, and the git install (no CI behaviour change)
  3. testkit, macros, spec, and sandbox-lifecycle, plus their Branch Checks wiring
  4. Runner, plans, environments, waivers, and docs
  5. The file-transfer and policy-advisor ports
  6. CI cutover, ideally run alongside the old lane for a few runs before the old suite is deleted

Thoughts on the open questions

  • Feature tests: e2e or integration? E2E. They're black-box tests against a deployed system that just need specific infra. I'd model that as capabilities on the same testkit and runner, and keep "integration" for in-process multi-component tests.
  • Are labels sufficient? For selection, yes. For guaranteeing coverage, no: they need typed values (compute_driver: kubernetes can't be required today), one schema shared by runner and testkit (it's free-form in environments.rs and a closed set in target.rs), a golden expected matrix, and result reconciliation.
  • Plan selection. One plan per trigger (PR / release-dev / release-qualification), with the GitHub matrix generated from the plan and CI running in strict mode. If arm64 stays planned but unrun, list it as an explicit, reviewed gap.
  • Name for conformance-integration. Keep the job ID (core-e2e-result and the required check depend on it) and change the display name, e.g. "Conformance (installed artifacts)".
  • Migration. Move one family at a time with a written parity checklist, overlap old and new lanes until green for N runs, then delete the old suite, crates/openshell-conformance, and the playbooks together. Amend RFC 0016 alongside this rather than after.

}

impl Status {
pub const fn is_failure(&self) -> bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsupported and NothingToRun aren't failures, so test exits 0 when it ran nothing. I reproduced this with --env tmachine.ubuntu2404-amd64.docker-rootful.deb on an arm64 host: UNSUPPORTED …, exit 0. In CI, an arm runner label, an arm64 env in the matrix, or an entry without environment would go green with zero tests. Could e2e-suites.yml pass a --strict/--ci flag that makes unsupported, nothing-to-run, and zero jobs errors?

};

Ok(match (output.status.success(), counts) {
(true, Some((tests, 0))) => Status::Passed { tests },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trusts nextest's exit code and JUnit totals but never compares them with the expected test list. nextest silently ignores a test(=name) that matches nothing (a filter with one typo'd name ran 1 test and exited 0), so a catalogued test missing from the binary, e.g. one behind a cfg, just disappears. I'd require a passing <testcase> for every expected ID and a catalog ID for every testcase.


const NEXTEST_CONFIG: &str = r#"# Written by the OpenShell test runner.
[profile.openshell]
fail-fast = false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things in this profile:

  • The old playbook pinned --test-threads 1 ("The guest has 4 GiB; keep sandbox creates from competing for it"). Without it, nextest runs up to 4 sandbox-creating tests at once on a 4G / -smp 4 guest, which looks like a flake source on k3s-ha and rootless Podman.
  • There's no slow-timeout/terminate-after, so one hung test uses up the job's 90-minute budget.

Suggest test-threads = 1 (or a test group), a slow-timeout, and an explicit no-tests = "fail".

run(&mut command).with_context(|| format!("build {} test archive", family.name))?;

let archived = archived_binaries(&manifest, &archive)?;
let missing: Vec<_> = binaries.difference(&archived).collect();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check runs in one direction only and compares binary names only. Comparing the archive's nextest list (no execution needed) with the catalog in both directions, keyed by package::binary, would catch tests the source parser misses. The old cli package also has file_transfer and policy_advisor binaries, so name-only matching can't tell them apart.

Comment thread tests/runner/src/jobs.rs
});
}
}
if family_jobs.is_empty() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty family jobs are dropped silently, so --env A --suite X, where X is only planned for B, produces no jobs and exits 0. Since --suite is validated against the whole plan, this passes validation. I'd fail if narrowing leaves nothing to run.

let uploaded = self.upload(&source, &remote, false).await?.success()?;
let warning = message(&uploaded);
testkit::ensure!(
warning.contains("outside a Git work tree")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching CLI message text ("outside a Git work tree" here, and at 313 and 339) ties conformance to wording the docs don't promise. The exit code and filesystem effects are the contract.


sandbox.stop().await?;
match sandbox.exec(["true"]).await {
Err(_) => {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err(_) => {} accepts any error, including a transport failure or the 2-minute exec timeout, as "rejected while stopped". Match the expected status code? Also, the second-stop check at 132 can't fail, because stop() already waits for Stopped, and the "not restarted" re-read at 72 happens immediately after Completed, so a delayed restart wouldn't be caught.

# github.workflow is the caller's name here, so scope the group to this job
# to avoid colliding with (and cancelling) the calling workflow.
concurrency:
group: ${{ github.workflow }}-e2e-suites-${{ github.ref }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old runner keyed concurrency on category. Without a discriminator, a second e2e-suites.yml lane in the same caller (GPU, features) would cancel this one.

uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0
with:
path: ~/.cache/tmachine
key: tmachine-${{ runner.os }}-${{ runner.arch }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integration-runner.yml deliberately skipped this cache for ubuntu-k3s (#3848), and now every environment shares one key. Intentional? Also, each job now cross-compiles the testkit/SDK workspace with no cargo cache, where prepared archives came from prepare-integration before, so expect a CI time increase.

{"environment":"fedora-podman-rootful","installer":"rpm","testsuite":"conformance"},
{"environment":"fedora-podman-rootless","installer":"rpm","testsuite":"conformance"}
{"plan":"cpu-tests","runner":"ubuntu-24.04","environment":"tmachine.ubuntu2404-amd64.docker-rootful.deb"},
{"plan":"cpu-tests","runner":"ubuntu-24.04","environment":"tmachine.ubuntu2404-amd64.k3s.k3s"},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list is copied by hand from cpu-tests.yaml (5 of its 10 environments) and repeated in release-dev and release-qualification, with no check between them. The arm64 environments are planned but never run, and nothing reports it, which is the failure mode that hid Kubernetes coverage. I'd generate this from the plan, or check it against the plan, before cutover.

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

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants