test(xtest): characterize the cli.sh supports matrix before replacing it (DSPX-4792) - #597
Draft
dmihalcik-virtru wants to merge 3 commits into
Draft
dmihalcik-virtru wants to merge 3 commits into
dmihalcik-virtru wants to merge 3 commits into
Conversation
native sdk adapters
Rewrite the scaffold into the seven-heading template with the numbers
counted out of the shell rather than estimated.
846 lines across sdk/{go,java,js}/cli.sh, of which 388 (45.9%) are the
supports case statement: 60 feature gates over 78 possible (SDK, feature)
pairs, plus 13 pairs answered ahead of the shell by a 9-arm Python match
in tdfs._uncached_supports. Full per-SDK gate tables map every arm to the
declarative kind it becomes, including an AllOf(HelpContains, MinVersion)
composite that ecwrap needs on all three SDKs.
Records two comment/code divergences found while tabulating:
- go hexless gates on >= 4.2.0 under a comment claiming 4.3.0; java and
js both gate on 4.3.0 exactly. Confirmed, not refuted.
- go better-messages-2024 gates on >= 0.3.18 under a comment claiming
0.3.28. Same shape, previously unreported.
Also records that the awk gates fail open: POSIX specifies a string
comparison for non-numeric fields, so 'main', 'dev', and 'v0.12.0'
satisfy every '$1 > 0' gate -- the opposite of what the FORCED_SUPPORTS
docstring claims the shims do for unreleased builds.
DSPX-4792 replaces the three cli.sh shims with native Python adapters.
Nothing today pins what the shims currently answer, so a port that gets a
gate subtly wrong shows up as a changed skip count on a green run rather
than as a failure.
supports_matrix.json records all 3 SDKs x 26 features = 78 gates: the
kind (help_contains / min_version / always / delegate / unprobeable /
all_of / absent), the source line, the exact probe argv, the verbatim awk
program, and -- the part that makes this a characterization test rather
than a transcription -- the answer each version gate yields at
representative versions. It also records the 4 help-text probes that live
outside the supports blocks, inline in the encrypt/decrypt argv paths,
where an adapter porting only the case statement would miss them.
test_supports_matrix.py verifies the snapshot offline:
- the recorded line numbers, arms, and probes still match cli.sh
- each min_version gate's Python model agrees with real awk over a
6x24x32 version grid
- the Python match arms in tdfs._uncached_supports are each classified
against the shell they shadow (5 redundant, 1 redundant with the
wildcard default, 1 contradicting, 6 supplying a missing answer)
- non-numeric versions satisfy every version gate, pinning the awk
string-comparison behaviour so A2 cannot change it by accident
No platform and no installed SDK builds are needed; the one test that
wants a real build skips per SDK when sdk/<sdk>/dist/*/cli.sh is absent.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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
1 participant
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.
Workstream A of DSPX-4640: replace
xtest/sdk/{go,java,js}/cli.shwith nativePython SDK adapters, one SDK at a time, then delete the shell.
Stacked on DSPX-4791 — read this first
The native adapters depend on the
otdf-adapterpackage (theSdkAdapterprotocol, the declarative gate kinds,
SubprocessCliAdapter), which isDSPX-4791 and is not on
mainyet. The adapters land only after it merges.So this PR deliberately contains no adapters and deletes no
cli.sh. Itcontains the plan, and the safety net the plan needs.
What's here
1.
spec/DSPX-4792.md— the strangler-migration plan for A2–A6, rewrittenfrom the Jira scaffold into the house template, with numbers counted out of the
shell rather than estimated:
supportscase statement (the parent spec guessed ~360).
through
*toexit 2.and the declarative kind it becomes. That table is the migration.
AllOf(HelpContains, MinVersion)composite thatecwrapneeds on allthree SDKs, which the parent spec's kind list doesn't have.
matchintdfs._uncached_supportscovering 13(SDK, feature) pairs (the parent spec says "10 hardcoded answers"), each row
classified against the shell arm it shadows.
=-form, js camelCase).green before any shell is deleted.
2.
xtest/supports_matrix.json+xtest/test_supports_matrix.py— acharacterization snapshot of the whole
supportsmatrix, and an offline testmodule holding the snapshot and the shell to each other.
Why this slice
The obvious first slice is "port the go adapter behind a flag" — but that's A2,
and A2 needs
otdf-adapter. The next-most-obvious is "extract the gate tables",which is the same blocker.
What is independently landable, and is the thing A2–A6 actually need, is an
oracle. Right now nothing pins what the shims answer. Port a gate slightly
wrong and it doesn't fail — it changes a skip count on a still-green run, which
nobody reads. So this lands the snapshot first, on
main, where it startsguarding immediately and A2 can diff against it from day one.
samplesis the part that makes it a characterization test rather than atranscription of the source: each version gate records the answer it yields at
representative versions, so an adapter that faithfully copies a declared
min_versionbut computes differently is still caught.The tests are fully offline — they read
cli.shas text, modelawkin Python(cross-checked against real
awkover a 6x24x32 version grid), and drive_uncached_supportsagainst stub shells. The one test that wants a real buildskips per SDK when
sdk/<sdk>/dist/*/cli.shis absent.Three findings from tabulating the gates
1. The reported
hexlessbug is CONFIRMED.sdk/go/cli.sh:83compares($1 > 4 || ($1 == 4 && $2 >= 2))— i.e. >= 4.2.0 — directly under thecomment
# Schema version 4.3.0 introduced hexless.java:109andjs:72bothuse
($1 > 4 || ($1 == 4 && $2 > 2) || ($1 == 4 && $2 == 3 && $3 >= 0)), whichis exactly >= 4.3.0. go alone reports hexless support for a 4.2.x build.
Fixed in A3, not here, so the skip-count delta lands in a PR that says so.
2. A second divergence of the same shape, previously unreported.
sdk/go/cli.sh:117comments "These appeared in go sdk 0.3.28" and gates on$3 >= 18— >= 0.3.18. Found by the same table, which is the argument forthe table.
3. The awk gates fail open, and the docstring claims the opposite. POSIX
specifies a string comparison when a field is not a numeric string. Verified
against real awk:
main,dev, andv0.12.0satisfy every$1 > 0gatein the suite.
FORCED_SUPPORTS's docstring says the shims "say no for preciselythe unreleased builds a fix needs to be evaluated against" — for any SDK whose
version probe doesn't emit a bare numeric triple, they say yes to everything.
Pinned by a test so A2 can't change it silently.
Also recorded, not fixed (each is a behaviour change and wants its own ticket
plus a functional test):
tdfs.decrypt_commandwritesXT_WITH_KAS_ALLOWLISTwhile all three shims read
XT_WITH_KAS_ALLOW_LIST— dead today because nofunctional test passes those kwargs, and the one unit test that does asserts the
Python-side spelling, so it can never catch the mismatch.
What was run
The 78 skips are
TestInstalledBuildAgreement, 26 features x 3 SDKs, skippedbecause no SDK builds are installed in this environment. No platform was
available and no platform-dependent test was run.
Out of lane, on purpose
No
otdf-adapter/package (DSPX-4791), nosrc/xtest/restructure(DSPX-4793), no entry-point registry (DSPX-4794), and no
cli.shdeleted.