test: Add 20 function-library fixtures incl. repr_* round-trips - #159
test: Add 20 function-library fixtures incl. repr_* round-trips#159leongdl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.1 bool() string forms (1/true/on/yes, 0/false/off/no, case-insensitive). All 8 forms match the spec note exactly; lowercase output matches suite convention.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.1 int(float) exactness + §1.2.1 int64. Reviewer-verified: 9e18 is exactly representable (mantissa < 2^53) and int(9e18)=9000000000000000000 < int64 max. Header math checks out.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.1 len = unicode codepoints; §1.1.5 escapes. All five expectations computed and confirmed (日本語=3, 👍=1, e+combining=2, \U0001D54F=1).
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.1 note, verbatim: range_expr of a whitespace-only string is an error. Uses the spec's own example input.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (3 reviewers, 2 distinct nits) — spec: EL §2.2.2 abs + §1.2.1 int64 (abs(-2^63)=2^63 unrepresentable). Nit 1 (adversarial): a parser treating -9223372036854775808 as unary-minus applied to a 2^63 literal rejects before abs ever runs — the test then passes without any abs overflow check existing; abs(0 - 9223372036854775807 - 1) would isolate the named rule. Nit 2 (literalist): the explicit overflow-is-error text was dropped from the published spec (it lives in RFC 0005); the mandate is inferred from the type's range — worth restoring in the spec.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (3 reviewers same nit) — spec: EL §2.2.2 floor/ceil→int + §1.2.1 int64. Two behaviors bundled in one abort-on-first-error fixture: ceil is never independently pinned (the header admits it). Split into two fixtures. Same dropped-overflow-text caveat as the abs sibling.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS — spec: EL §2.2.2 sum + §1.2.1 (max + 1 overflows; both elements individually valid, isolating the overflow to sum). Same caveat: overflow-is-error rests on the type table since the RFC 0005 normative text isn't in the published spec.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.4 upper/lower. All six unicode expectations computed and confirmed; the deliberate exclusion of ß (full vs simple case mapping) and Turkish i (locale) is exactly right — the spec picks neither.
There was a problem hiding this comment.
Quorum verdict: GOOD (4/5) — spec: EL §2.2.4 ljust/rjust/center/zfill + len-in-codepoints. Expectations verified; odd-split center correctly deferred as spec-unspecified (even split only). Nit: width-measured-in-codepoints is an inference from len's definition — the spec never states it for padding; a spec sentence would close it.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.4 predicate empty/uncased rules (isupper/islower need a cased char; isdigit/isalpha/isalnum/isspace need non-empty; isascii true on empty). Each of the 9 expectations follows verbatim from a table clause.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (4/5; 1 dissent NOT-GOOD) — spec: EL §2.2.4 isascii (explicit U+0000–U+007F — solid) and isalpha. Dissent: ALPHA_CJK:true / ALPHA_ACCENT:true pin Unicode-alphabetic semantics while the spec wording ("all characters are alphabetic") is exactly parallel to isdigit's, which this same PR rules ambiguous and routes to proposed/. Both implementations happen to agree here (Python str.isalpha and Rust char::is_alphabetic are both Unicode), which is why the majority keeps it — but consider a spec sentence defining "alphabetic", or move the two ALPHA lines alongside the isdigit question for consistency.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, one wording nit) — spec: EL §2.2.5 (syntax = Python re ∩ Rust regex; backrefs explicitly "Not supported"). Raw string correctly delivers backslash-one to the engine. Nit: the spec says "not supported" rather than "is an error" for pattern features — must-reject is the only intersection-consistent reading, but tightening the spec wording would help.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.5 (lookahead in the not-supported list). Verbatim not-supported construct; same "not supported" vs "error" spec-wording nit as the backreference sibling.
There was a problem hiding this comment.
Quorum verdict: GOOD-WITH-NITS (3 reviewers same nit) — spec: EL §2.2.5 (re_sub repl is literal; group refs "are errors" — spec-explicit). Nit: both dialect forms (\1 and $1) are bundled into one abort-on-first-error fixture, so the $1 form is never independently pinned; split.
There was a problem hiding this comment.
Quorum verdict: NOT-GOOD (2 independent reviewers converged; others GOOD/nits) — spec: EL §2.2.6 repr_cmd. The P03 caret payload expects a ^ b to round-trip through cmd /c, but cmd.exe does not process ^ as an escape inside double-quoted spans: the spec's own documented output for repr_cmd("a ^ b") is "a ^^ b", which delivers a literal doubled caret to the child argv — so a spec-conforming implementation FAILS this fixture. This is inconsistent with the fixture's own exclusion of the % and ! payloads, and it's flagged WINDOWS-UNVERIFIED. Fix: drop the ^ payload (like % and !) or verify on a real Windows runner first — if real cmd behavior differs from the spec's inside-quotes escaping table, that's a spec bug to file. The other 11 payloads check out.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.6 repr_json (all overloads). Parse-back via json.loads with value AND type comparison is the right spec-literal oracle (any valid JSON encoding passes); all 15 escape/overload branches verified.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5, craft nits) — spec: EL §2.2.6 repr_pwsh (single-quote doubling, $true/$false, @() lists). Genuine roundtrip through the real PowerShell parser; correctly defeats PowerShell's string-coercing -eq with -isnot [string]. Nits: twelve copy-pasted verification blocks; WINDOWS-UNVERIFIED (honestly flagged) — value is contingent on a Windows runner.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.6 repr_py = Python repr. ast.literal_eval round-trip with type checks; argv-per-value avoids any shell layer; the embedded-newline exclusion is honestly documented and correctly deferred to the proposed/ fixture in PR #165.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.6 repr_sh = shlex semantics. All 15 payloads computed against shlex.quote; bash re-parse plus forbidden INJ lines correctly detect substitution leaks. A genuine re-invocation of bash, not an echo.
There was a problem hiding this comment.
Quorum verdict: GOOD (5/5) — spec: EL §2.2.6 repr_sh string/path/list overloads. set -- re-parse asserting LIST_ARGC:3 plus per-word brackets is the right proof the list overload joins into 3 shell words. Uniquely covers the list/path overloads vs the adversarial sibling (4 trivial payloads overlap — acceptable split).
|
Quorum review (5 independent agents). 20 fixtures; per-fixture verdicts posted as file comments. Net: 13 GOOD, 6 GOOD-WITH-NITS, 1 NOT-GOOD. The roundtrip fixtures are the highlight — all five genuinely feed repr output back through the real target parser (bash, json.loads, ast.literal_eval, cmd→argv, PowerShell) rather than echoing quoted strings, and the deliberate exclusions (upper(ß), isdigit(٣), repr_py newline) correctly avoid pinning spec-ambiguous behavior. One blocker: the repr_cmd roundtrip's caret payload asserts behavior that contradicts the spec's own example. Note: the ~160-case EXPR arity-error gap remains unaddressed by this PR (by design). |
e95c435 to
c69fcf2
Compare
|
Quorum-review fixes applied and pushed (rebased onto mainline 3069673):
Verified: all 68 expr2.2* jobs fixtures pass against openjd-rs upstream/main |
c69fcf2 to
bd2c93a
Compare
|
Follow-up push (bd2c93a): the Windows CI failure was a runner encoding bug, not a fixture defect — |
bd2c93a to
3c2a2f7
Compare
20 fixtures from the RFC 0006 coverage audit: repr_sh/repr_py/repr_json adversarial round-trips executed through the real target interpreter, repr_cmd/repr_pwsh round-trips (windows-gated), unicode codepoint semantics for len/case/predicates/padding, int64 overflow inside abs/sum/floor/ceil, predicate and conversion edges, and three regex-dialect pins (lookahead, backreference, group-reference rejection per spec 2.2.5). Suite: 350 -> 368 passing. Spec-question fixtures that fail today are split to conformance-func-lib-expected-failures. Review: quorum-review fixes — dropped the caret payload from the repr_cmd round-trip (the spec's own example output "a ^^ b" cannot round-trip through cmd /c under the documented quoting model; header explains); abs overflow operand rebuilt as (0 - max - 1) so a unary-minus lexing quirk cannot mask a missing abs check; floor/ceil and the two re_sub group-reference dialects split into separate fixtures so abort-on-first-error cannot leave the second case unpinned; documented the isalpha-parallel-to-isdigit ambiguity and the padding width-unit inference. Also: fix runner file encoding for Windows CI — the runner opened fixture files and decoded subprocess output with the platform default encoding, so Windows (cp1252) crashed with UnicodeDecodeError on this PR's non-ASCII unicode fixtures. All reads/writes and subprocess decoding now pin UTF-8 (errors='replace' for child output). Also set PYTHONUTF8=1 for child processes: on Windows a child python writing to a pipe uses cp1252 by default, mangling non-ASCII sentinel output before it reaches the session log (the remaining unicode-fixture failures on the Rust windows job). Parked expr2.2.6--repr-cmd-roundtrip in jobs/proposed/ after its first real Windows CI run: the cmd.exe child emits no stdout at all (siblings repr_pwsh/repr_py/repr_json pass on the same runner), so the cmd /c action path needs hands-on Windows investigation before this fixture can be live. Header records the observed behavior and promotion criteria. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
|
Second follow-up (head 9e0d13c): the PYTHONUTF8=1 runner fix cleared all unicode fixtures on the Rust windows job (case-transforms, padding, string-classification, len). The one remaining Rust-windows failure was |
3c2a2f7 to
9e0d13c
Compare
|
Final CI state for head 9e0d13c: Conformance Tests (Rust) pass on all three platforms; Python ubuntu passes. The Python windows job reports 16 failures: the 13 pre-existing mainline failures (that job has been red on mainline since July — it installs the unpinned released CLI), plus 3 of this PR's round-trip fixtures (repr_json/repr_pwsh/repr_py adversarial) failing on Windows only. Those 3 are the same defect class as the pre-existing mainline repr_pwsh/repr_py/repr_sh Windows failures — the released Python CLI's repr functions misbehave on Windows — and they pass on the other four jobs, so they are kept as additional pins on that known defect rather than parked. The job's red/green state is unchanged by this PR. |
Summary
20 function-library fixtures from a coverage audit of RFC 0006. Suite: EXPR 350 → 368 passing, 0 failing (2 new fixtures are windows-gated and skip on POSIX).
Areas covered
repr_*round-trips — the security-critical surface. The audit found all five pre-existingrepr_*fixtures assert the quoted text as a substring; none executed it. Quoting is only correct if the target interpreter agrees, so these fixtures pipe the quoted output through the real target:repr_shthroughbash(15 adversarial payloads: metacharacters,$(...), backtick, nested quotes, backslash, glob,.., leading-, unicode, empty, whitespace-only),repr_pythroughast.literal_eval(16 payloads),repr_jsonthroughjson.loads(15 payloads) — each asserted byte-identical after the round trip.repr_cmd/repr_pwshround-trips: written and windows-gated (runOn: [windows]); they skip on POSIX and need a Windows CI run to count.%VAR%/!VAR!payloads excluded from the cmd battery per the spec's own.bat-context scoping.lenon CJK/emoji/combining-mark/astral strings; case transforms restricted to 1:1 mappings;isascii/isalphaclassification; padding to codepoint widths.abs(-2^63)rejected (negation overflow),sumoverflow rejected,floor(1e300)/ceil(1e300)rejected,int()at the exact-double 9e18 boundary.isupper("123")/islower("123")false (the "no cased characters" clause), predicates on"",bool("TRUE")/bool("1")case-insensitive forms,range_expr(" ")rejected.re_subreplacements are all rejected by openjd-rs exactly as §2.2.5 requires — but Python'srenatively supports all three, so a naive Python-backed implementation would accept them. These three.invalidfixtures now pin the conservative dialect for every implementation.Deliberately excluded
~160 arity/type error cases (indistinguishable under the current runner, which asserts only a non-zero exit) and ~78 null-argument cases (the spec defines null behaviour for only 6 of 84 functions — a spec decision is needed first). Writing these now would manufacture confidence.
Companion PR
Three parked fixtures with dual-implementation observations (both-implementation
repr_pynewline bug;center()/isdigit()spec gaps where both implementations agree) are in the expected-failures PR (branchconformance-func-lib-expected-failures).