Skip to content

test(testkit): add the openshell-conformance test program - #5249

Open
elezar wants to merge 11 commits into
mainfrom
test/3954-testkit-test-program-restack/elezar
Open

test(testkit): add the openshell-conformance test program#5249
elezar wants to merge 11 commits into
mainfrom
test/3954-testkit-test-program-restack/elezar

Conversation

@elezar

@elezar elezar commented Nov 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

A restack of #5138 for review, not a new change. It has the same content, rebased onto the current main and reordered into smaller commits that each do one thing. Differences from #5138 are listed below.

Related Issue

Part of #3954. No issue closes with this PR; it is a draft for review of the commit structure.

Changes

Ten commits, in order:

  1. build(testkit): add dependencies and lockfiles (linkme, libtest-mimic, clap, quick-junit, libc).
  2. feat(testkit): register tests with #[testkit::test] and add a test program with list and libtest's interface.
  3. feat(testkit): add a ready subcommand, shared with testkit-ready.
  4. feat(testkit): add a run subcommand, one process per test with a timeout.
  5. feat(testkit): write a JUnit report from run and keep each test's output.
  6. docs(testkit): document the program and test registration.
  7. refactor(conformance): move the suites into a library (renames, plus the nextest name change).
  8. feat(conformance): add the openshell-conformance program and its drift check.
  9. test(e2e): run the conformance host lanes with the program.
  10. ci(e2e): build the program once per architecture in Branch E2E.

Authorship: the code is Matthew Grossman's work from #5138, so every commit lists him as the author. Evan Lezar split and reordered the commits, wrote the intermediate versions that let each one build on its own, and signed them off as committer.

Differences from #5138:

Testing

  • Each testkit commit compiles, passes clippy with -D warnings, and passes the testkit tests on its own
  • On the final tree: conformance clippy, check-conformance-program.sh, lockfile and alignment checks, and cargo check for support, features, podman and runner
  • Intermediate conformance and CI commits individually; the pre-commit hook was skipped while building the stack
  • E2E lanes; they run with this PR's test:e2e label

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

@elezar elezar added the test:e2e Requires end-to-end coverage label Nov 4, 2026
@copy-pr-bot

copy-pr-bot Bot commented Nov 4, 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

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/5249 does not exist yet. A maintainer needs to comment /ok to test 410aa2809f57b0c8c77debe17c0317c5298bbd1d to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pr-5249.docs.buildwithfern.com/openshell

@matthewgrossman @elezar
build(testkit): add dependencies for the test registry and program
a1a6440 
Add linkme for the test registry, libtest-mimic and clap for the test
program's command line, quick-junit for its report, and libc for process
control on Unix. Refresh the lockfiles of the workspaces that depend on
testkit.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
feat(testkit): register tests and add a test program with list
3655307 
`#[testkit::test]` now also registers each test, with its family, name,
description and `#[ignore]` reason, in a link-time registry. The hidden
function gets only `cfg` and lint attributes; `#[should_panic]`,
`#[cfg_attr]` and `#[expect]` are rejected with an error that says why.

`testkit::main()` turns a binary into a test program. It takes libtest's
arguments, so it also works as a `harness = false` target under cargo
test and nextest, and it has a `list` subcommand that prints each test's
name and description. A program that links no tests exits 2, since Rust
does not link a suite crate that is never named.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
feat(testkit): add a ready subcommand to the test program
76c4eae 
`ready [--timeout SECS]` waits for the gateway the program will test and
exits 2 if it is not ready in time. It shares its check with the
`testkit-ready` binary, which now calls it instead of keeping its own
copy.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
feat(testkit): add a run subcommand to the test program
72d50df 
`run [FILTER]... [--include-ignored] [--timeout SECS] [--grace SECS]`
runs the selected tests one at a time. Each test is a new process, the
program started as `<name> --exact --nocapture`, in its own process
group, so nothing a test starts outlives it. A test that runs past its
timeout (900s by default) gets SIGTERM, then SIGKILL after the grace
period (10s by default).

Interrupting `run` stops the current test and runs no more; a second
interrupt skips the grace period. `run` exits 1 when a test failed and 2
when tests could not run or the run was interrupted. It is Unix only; on
other platforms it says to use cargo test or nextest.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
feat(testkit): write a JUnit report from the test program's run
0e17fd4 
`run --junit PATH` writes a JUnit report with one test suite per family
and one test case per test, including skipped, failed and timed-out
tests. Each test's output is shown as it comes and the last 4 MiB of each
stream is kept for the report. The supervisor waits for a test's output
to close before it moves on, so a signal that arrives while it waits
still ends the run.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
docs(testkit): document the test program and test registration
39f5b91 
Describe how `#[testkit::test]` registers tests, the `list`, `ready` and
`run` subcommands, their exit codes, and the limits of `run`: Unix only,
and a test stopped on a timeout does not run its cleanup.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
refactor(conformance): move the conformance suites into a library
99ab04f 
Cargo builds each file in tests/ as its own test executable, which no other
program can link. Move the conformance suites into src/ as modules of a
library, so a test program can link them. Test code is unchanged; cargo test
and nextest run the same 15 tests as the library's unit tests.

Nextest now names them as binary `conformance`, test `<suite>::<fn>`, so
filters that selected a suite by binary select it by test name instead.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
@matthewgrossman @elezar
feat(conformance): add the openshell-conformance program
822eaed 
The conformance crate now also builds openshell-conformance, a binary whose
main calls testkit::main() and links the crate's library. It lists, waits
for the gateway, and runs the conformance tests without cargo or nextest.
It runs only the conformance suite and promises only the conformance
contract; feature and driver suites are not part of it.

The binary has test = false, so cargo test and nextest still see one
conformance test binary, the library's unit tests, and the e2e lanes and
the runner's filters are unchanged. The library is linked without
cfg(test), so the program gets the registrations and none of the tokio
test wrappers. tasks/scripts/check-conformance-program.sh checks that
openshell-conformance list names the same tests as nextest; Branch Checks
and mise test:rust run it.

A dependency that is never named is not linked, so a suite the program
forgets to name would silently go missing. testkit::main() now fails with
exit 2 when a program links no tests, under every command.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman @elezar
test(e2e): run the conformance host lanes with openshell-conformance
12238f7 
The Docker, Podman, VM and Kubernetes host lanes ran the conformance suite
with cargo nextest. They now run it with openshell-conformance: ready, then
run --junit, so CI exercises the program that gateway owners use.

The script uses OPENSHELL_CONFORMANCE_BIN when set and otherwise builds the
program from the checkout. The JUnit report goes to
OPENSHELL_CONFORMANCE_JUNIT or results/conformance-<gateway>.xml; the
Kubernetes runner names it after its report so credential-driver runs keep
both.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman @elezar
ci(e2e): build openshell-conformance once per architecture in Branch E2E
9d4f6e5 
Each Docker, Podman, VM and Kubernetes host lane built the conformance
suite itself. Branch E2E now builds openshell-conformance once per
architecture as a static musl binary, checks that it is static and lists
its tests, and uploads it. setup-e2e-cli downloads the one that matches the
runner when a lane sets prebuilt-conformance, and the lane runs it through
OPENSHELL_CONFORMANCE_BIN.

The job is separate from build-binaries.yml, which the release workflows
also call: the program has its own workspace and is not released. Lanes
called without prebuilt-conformance, such as the release qualification
lanes, still build the program from the checkout.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@elezar
elezar force-pushed the test/3954-testkit-test-program-restack/elezar branch from 410aa28 to 9d4f6e5 Compare November 4, 2026 13:56
@elezar

elezar commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test 9d4f6e5

@elezar elezar added area:build Related to CI/CD and builds topic:testing labels Nov 4, 2026
@elezar elezar mentioned this pull request Nov 4, 2026
Open
4 of 8 tasks
@matthewgrossman
matthewgrossman marked this pull request as ready for review November 4, 2026 17:25
This was referenced Nov 4, 2026
Open
Open
matthewgrossman
matthewgrossman previously approved these changes Nov 4, 2026
View reviewed changes

@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 restacking this. I checked it against #5138 (7531d8a21).

Content. Once main's movement is accounted for, #5249 + #5248 match #5138. The only differences are the gateway.rs test (now #5248), the README wording (#5247's change is already included here), and the refreshed tests/suites/drivers/podman/Cargo.lock. The conflict resolutions in branch-checks.yml, tasks/test.toml, TESTING.md and testkit/Cargo.toml keep both sides, as they should.

E2E (run 41843697095 at 9d4f6e57d). All 15 host lanes report openshell-conformance: 15 passed, 0 failed with the prebuilt program (OPENSHELL_CONFORMANCE_BIN set, no conformance compile). All 5 tmachine conformance rows are 15/0/0/0. The skipped jobs are the same opt-in HA, credential-driver and GPU jobs that #5138 skipped.

LGTM.

@matthewgrossman matthewgrossman mentioned this pull request Nov 4, 2026
Draft
3 of 6 tasks
@matthewgrossman
test(e2e): harden the conformance program helpers
7f5c39b 
Take the openshell-conformance path from cargo's build messages instead of
assuming <target>/debug, so CARGO_BUILD_TARGET and other profiles work. Make
the drift check fail clearly when it reads no test names from nextest.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman

matthewgrossman commented Nov 5, 2026

Copy link
Copy Markdown
Member

Reviewed the testkit program code (run, process supervision, output capture, JUnit), the registry, conformance.sh, the drift check, and the new Branch E2E build job. I did not find any bugs.

I pushed one small commit to this branch (7f5c39b):

  • conformance.sh now takes the binary path from cargo's build messages instead of assuming <target>/debug/, so CARGO_BUILD_TARGET and other profiles work.
  • check-conformance-program.sh fails with a clear message when it reads no test names from nextest, instead of printing a diff with every line removed.

Known limitations, not blocking:

  • A test stopped by run --timeout skips its cleanup hooks (test(testkit): add cleanup hooks that run after pass, failure, or panic #5219 has no signal handling), so its test-<run id>-* sandboxes can be left behind. The supervisor.rs header documents this. Worth a follow-up issue for a SIGTERM handler that runs the hooks within the grace period.
  • Each test's stdout and stderr are kept in memory (up to 4 MiB per stream) until the JUnit report is written. That is fine at the current suite size; writing output to disk per test would remove the growth if suites get chatty.

The E2E lanes still need to run with the test:e2e label.

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

@matthewgrossman matthewgrossman matthewgrossman approved these changes
@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

Assignees

No one assigned

Labels

area:build Related to CI/CD and builds test:e2e Requires end-to-end coverage topic:testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@elezar @matthewgrossman