Add a server-parameterized scenario framework (Phase 0) - #341
Open
sibson wants to merge 1 commit into
Open
Conversation
Extract the shared round trip in tests/functional/vncservers.py into tests/functional/scenarios.py: a Scenario is data (name, description, required capabilities) wrapping a plain run(client, ctx) callable, so the same scenario list can drive unittest, the future vncdo record wrapper, and the Tier 3 checklist -- not just TestCase methods. VNCServer grows an explicit capabilities property (auth type, known_size, renders_desktop, input_reactive) replacing ad-hoc flag checks; register_server_tests() now generates the servers x scenarios cross product (test ids like TestServer_tigervnc.test_capture), and a server missing a required capability skips with a named reason rather than passing silently. Only connect and capture are ported in this PR; authenticate/keyboard/mouse/expect follow in 3b.
sibson
added a commit
that referenced
this pull request
Aug 14, 2026
* Add server testing framework design (Phase 0) Three legs: unit tests as the regression layer (byte-level protocol tests and per-encoding decoder goldens), the live server fleet for smoke and discovery via subprocess-run CLI with event-sink verification, and a capture kit (vncdolog --capture plus an in-repo replay tool) for servers we cannot host. Replay never runs in CI; bugs found live or via capture are distilled into unit tests. Supersedes the #341 scenario-registry approach, retires pexpect and the native libvncserver build, and demotes the generated compatibility.rst to a non-requirement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Reconcile compatibility plan with the testing framework design Phase 0 now points at docs/testing-framework-design.md and reflects its decisions: unit tests as the regression layer, no replay in CI, decoder golden unit tests instead of live golden PNGs, subprocess-run smoke with event-sink verification, the native libvncserver build retired into the compose fleet, and generated compatibility.rst demoted to a non-requirement. Tier 3 becomes capture-plus-distill (vncdolog --capture evidence on issues, replay tool for maintainers, distilled unit tests as the permanent floor) instead of in-repo replayable fingerprint fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
What changed
This is PR 3a of the Phase 0 stack, stacked on PR 2 (#340). It extracts the abstraction the plan's Phase 0 bullet asks for ("extract the common scenarios... into a server-parameterized base class") — framework only; the scenarios beyond
connect/captureland in a follow-up PR 3b.The problem with plain test methods
tests/functional/vncservers.pyhad one shared test body,test_connect_key_and_capture, generated per server byregister_server_tests(). Addingtest_mouse,test_expect, … as sibling methods is the obvious next step, but the plan needs the same scenario list to serve three consumers, only one of which is unittest:vncdo recordwrapper overloggingproxydrives a scenario against a server and writes a fixture directory; it needs a scenario callable outside aTestCase.So
tests/functional/scenarios.pymakes aScenarioaNamedTuple(name, description, required capabilities) wrapping a plainrun(client, ctx) -> Nonefunction, andSCENARIOSis the ordered registry both unittest and the future recorder consume.Capability model
VNCServer(invncservers.py) grows acapabilitiesproperty — the one obvious place a server declares what it can do — derived from its existing fields (auth:none/auth:vncpass/auth:ardfrom username/password,known_sizefromsize,renders_desktop, and an as-yet-unusedinput_reactive). AScenario.requiresa subset of that; a server missing a required capability skips with a reason naming the capability, never a silent pass. That distinction matters because a skip has to render differently from a pass in the compatibility matrix — otherwise macOS Screen Sharing's black framebuffer reads as green coverage instead of "not applicable here."register_server_tests()now generates the servers × scenarios cross product: oneTestCasesubclass per server, onetest_<scenario>method per scenario, so test ids read likeTestServer_tigervnc.test_capture— a per-server, per-scenario pass/fail/skip cell.Why
PIXELexists but is unusedscenarios.pydefines three assertion-level helpers for input scenarios:PROTOCOL(session survives, always available),CHANGE(framebuffer differs at all — honest that a blinking cursor would also pass this), andPIXEL(a specific region changed — the strongest level). No Tier 1 server declaresinput_reactiveyet:tests/servers/draw-content.shpaints static content, so nothing in the fleet reacts to input in a known region. That's deferred to its own spike (image work with real flakiness risk — font rendering, timing) rather than folded into the PR that establishes the abstraction.PIXELis implemented now so that spike only has to flip a capability flag, not invent an assertion. I added this as an explicit Tier 1 follow-up indocs/server-compatibility-plan.md.Scope: only
connectandcapturePorted only the existing round trip, split into its natural scenarios (
connect: handshake + negotiated version/security type recorded to the artifact dir;capture: the existing PNG assertions — valid PNG, size whenknown_size, not-flat whenrenders_desktop). Deliberately not addingauthenticate/keyboard/mouse/expecthere — that's PR 3b, once this abstraction has been reviewed with two users instead of six.Screenshot gallery
Left
tests/functional/capture_screenshots.pyas-is (flatscreenshots/<server>.png, one gallery). The unittest-generated tests now write into the finer-grainedscreenshots/<server>/<scenario>/the spec describes, but folding the standalone gallery script into that same per-scenario layout is a bigger change (it would need to walkSCENARIOSand call scenario bodies rather than justcapture_screenshot()) and is left for 3b alongside the new scenarios it would need to display.Verification
flake8 --count --statistics vncdotool tests— clean (0).make test(python -m unittest discover tests/unit) — 67 tests pass, unaffected (all changes are undertests/functional/).python -m unittest discover -v -s tests/functional -t . -p 'test_servers.py'with no servers running: all 6 generated tests (tigervnc/tigervnc-auth/x11vnc×connect/capture) skip with a clear "not reachable... start the servers first" message, and the process exits normally (no hang).TestLoader().discover(...):tests/functional/capture_screenshots.py dockerstill runs end-to-end (skips cleanly with no servers up, writes the galleryindex.html).Scope note
Per instructions,
docs/server-compatibility-plan.mdgets only the one new Tier 1 follow-up bullet (the deferred input-reactive spike) — no other restructuring.Generated by Claude Code