Repository navigation
test(consistency): run every manifest case across the bindings - #394
Merged
Merged
Conversation
This was referenced Oct 7, 2026
Peefy
force-pushed
the
pr/consistency-29-cases
branch
2 times, most recently
from
October 7, 2026 14:17
9ee378b to
79b45e7
Compare
This was referenced Oct 8, 2026
Peefy
force-pushed
the
pr/consistency-29-cases
branch
2 times, most recently
from
October 8, 2026 03:27
63c7ce4 to
b7bb599
Compare
…l bindings The manifest checked 13 cases across 10 of the 27 dispatchable RPCs, so two thirds of `spec.proto` was untested and the two thirds that were could not disagree with each other about anything the others did. It is now 29 cases across 25 RPCs, generated from a scratch-directory fixture set so the output does not depend on the checkout path. Widen it from 13 cases to 29, and make the expectations protobuf-JSON *default* dialect -- an unset field is absent, not `""`/`0`/`[]`/`null`. That is the one encoding all fifteen runtimes already emit for free; the previous dialect was Python's `always_print_fields_with_no_presence`, which would have forced each of the remaining twelve runners to hand-roll a serializer before it could compare anything. Two things surfaced that are worth more than the new cases: Seven of the eight tracked `kcl-api` pins sat on revisions predating six RPCs added after `FormatTestReport`, so those bindings could not dispatch a third of the spec at all. Both tracked pins (`ruby/Cargo.lock`, `c/Cargo.toml`) are now on `3ec296a9`, and `list_method`/`load_package` pin the fields that a stale core gets wrong, so the next drift fails here instead of passing quietly. Every runner also asserts it covers the whole manifest now. The unported ones (c, cpp, dart, julia, lua, nodejs, swift, wasm, zig) fail that check by design -- it is the list of what Phase 2 has left, and it is why they were not already known to be short. Signed-off-by: Peefy <xpf6677@163.com>
The java runner had 13 of the manifest's 29 cases and no coverage check, so it reported green while executing a third of the spec. Add the 16 missing cases and an `@AfterClass` guard that fails when `cases.json` grows a case this runner does not dispatch. The guard is an `@AfterClass` rather than a `@Test` because JUnit does not order methods: as a test it would fail on a green run whenever it happened to execute before the cases it is checking for. Comparing the schema-mapping documents needed a canonical form -- sorted keys, `filename` dropped -- because `KclType` holds two protobuf maps whose iteration order is undefined, and `filename` is an absolute path. Rebuilding the tree as a `JsonNode` rather than as Java collections keeps a pinned number the same Jackson node type on both sides, so the comparison cannot pass or fail on `1` vs `1.0`. Signed-off-by: Peefy <xpf6677@163.com>
The runner covered 13 of the manifest's 29 cases and had no guard, so it passed while half the spec went unexercised. Dispatch the remaining 16 RPCs and add a coverage check, so a manifest case with no runner is a failure rather than a silent gap. The Zig protobuf runtime has no JSON printer, so the two schema-mapping cases serialise `KclType` by hand in the default dialect the manifest is written in; `renderValue` then sorts keys and drops `filename`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The runner covered 13 of the manifest's 29 cases and had no coverage check, so the other 16 went unexercised. Dispatch them, add the guard, and project `has_error_message` for `validate_code_invalid` alongside the raw diagnostic that cannot be pinned. package:protobuf's `toProto3Json` emits protoc's camelCase names and has no hook for presence, so `_kclType` projects the schema messages by hand in the default dialect the manifest is written in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The runner covered 13 of the manifest's 29 cases; the other 16 went unexercised with nothing to say so. Dispatch them and extend the coverage check to the whole manifest. Doing so exposed a real decode gap: `LoadPackageResult` fields 11-13 (`imports`, `kcl_mod`, `apps`) were present as structs but had no `SwiftProtobuf.Message` conformance, so they were silently dropped on decode and `load_package` reported no imports, no module name and no apps. Complete the conformances in the file's generated style. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The runner covered 13 of the manifest's 29 cases with nothing to say so. Dispatch the remaining 16 and add a coverage check that fails when a manifest case is never reached or falls through to no handler, which is what a manifest grown by one binding produces in a runner that is not updated with it. That exposed a gap in the bridge: `LoadPackageResult` omitted proto fields 11-13, so the C++ API had no `imports`, `kcl_mod` or `apps` at all and three manifest-pinned expectations were unreachable. Add them; the change is purely additive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The manifest omits default-valued args, so `GetProperty("format")` threw
for `generate_kcl_json` and `generate_kcl_yaml`. Read it the way the
sibling cases already read `version`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Peefy <xpf6677@163.com>
The manifest omits default-valued args, so `get("format")` returned null
for `generate_kcl_json` and `generate_kcl_yaml`. Read it the way the
sibling cases already read `version`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Peefy <xpf6677@163.com>
The runner covered 13 of the manifest's 29 cases. Dispatch the remaining 16 and add a `[ClassCleanup]` guard that compares the executed set against the manifest's own case list -- a `[TestMethod]` guard would be misreported as a failure on an otherwise green run, since MSTest does not order methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The julia runner only drove 13 of the manifest's 29 cases, so a divergence between julia and the other bindings on any of the other 16 would never have surfaced as a failing test. Canonicalise protobuf messages by reflection (dropping proto3 defaults and KclType.filename) rather than hand-writing a renderer per message, and add a coverage guard that compares the dispatch list against the manifest's own case list — the two are written independently, so the check still fails when a case is only covered "in theory". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The runner only dispatched 13 of the manifest's 29 cases and threw at module scope when it met an RPC it had no entry for, so the other 16 were not merely untested — the file could not even be loaded against a manifest that had grown. Replace that throw with a per-case skip plus a coverage guard that names the case and the reason, and compare the executed set against the manifest's own case list rather than against the dispatch table. Three real gaps surfaced while porting, all additive: - FormatPathArgs hardcoded `dry_run: false`, so format_path_dry_run was unreachable. Accept it as an optional argument. - GetSchemaTypeMappingArgs had no k_code_list, so get_schema_type_mapping could not pin a source-only program. Accept it as an optional `sources`. - LoadPackageResult was missing `imports`, `kcl_mod` and `apps`, three of the nine fields the manifest pins. KclType recurses through plain message fields, which a napi object cannot express, so the schema mappings ship as canonical protobuf JSON in a new `schemaTypeMappingJson` alongside the existing name-only summary rather than replacing it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The kotlin runner drove 13 of the manifest's 29 cases, so kotlin could diverge from java on any of the other 16 without a test failing. Add the 16 cases as individual @test methods, mirroring java's scratch-copy, path-resolution and pinned-fields-only machinery, and add an @afterall guard that subtracts the executed set from the manifest's own case list. The new cases need `com.google.protobuf.util.JsonFormat` to render the schema type mappings in the default dialect, so take protobuf-java-util at test scope — protobuf-kotlin does not pull it in, and the published kotlin API does not need it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The lua runner dispatched 13 of the manifest's 29 cases, and its coverage check verified only that the 13 names it already listed existed in the manifest — a check that passes by construction. That is how the manifest grew from 13 to 29 cases while this file kept running 13. Dispatch all 29, and replace the vacuous check with one that walks the manifest's own case list and subtracts the names the runner declares. While porting, three things needed fixing to reach the other 16: - validate_code_invalid compared a bool against the raw err_message string. The diagnostic itself is unpinnable — ANSI escapes plus a random temp path — so the projection now reports success and whether a message is present. - scratch: paths were resolved without copying, so override_file and friends would have written into the repository. Copying goes through lfs rather than penlight, which has no copytree. - exec_args ignored work_dir and did not absolutise k_filename_list, matching the java/python/go runners instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peefy <xpf6677@163.com>
The wider 29-case manifest exposed three decoder bugs: top-level map entry decoders closed the sink back to depth -1, emitting malformed JSON for any 2+ entry map; Scope.parent/owner and Symbol.owner/def drove kind callbacks into never-begun scratch sinks, hard-failing whenever the core sends them (LoadPackage resolve_ast=true); and the KclType required string array never emitted commas. Expose the two helpers the runner needs, and extend KclLoadPackageResult with has_kcl_mod, the kcl.mod package name, and the apps/imports counts. Signed-off-by: Peefy <xpf6677@163.com>
Port the C runner to all 29 manifest cases like the sibling bindings: per-case scratch copies of the template fixtures, a canonical-JSON printer (sorted keys, filename dropped, integral numbers without .0), generalized tree comparison, the 16 missing RPC branches, and the DISPATCHED_CASES guard that fails the run when the manifest grows a case this runner does not handle. The generate_openapi example matches the qualified AppConfig key the pinned core emits. Signed-off-by: Peefy <xpf6677@163.com>
Signed-off-by: Peefy <xpf6677@163.com>
Signed-off-by: Peefy <xpf6677@163.com>
The core reports .\\__main__.k-style paths on Windows; the manifest pins basenames like the go runner, so split on both separators. Signed-off-by: Peefy <xpf6677@163.com>
The zig-test CI legs crashed the four scratch: cases (lint_path_with_errors,
override_file, load_settings_files, update_dependencies_no_deps) with
'programmer bug caused syscall error: BADF' inside std.Io.Threaded.dirReadLinux,
while local darwin-arm64 runs passed.
Two independent defects, both in zig/src/consistency_test.zig:
* The scratch machinery iterated directories it had opened with the default
OpenOptions (iterate: false). On Linux, std.Io.Dir.openDir maps that to an
O_PATH fd, which lseek/getdents64 refuse with EBADF, so the first directory
read panicked. macOS has no O_PATH, so the same illegal iterate-on-a-
non-iterable-dir passed locally - which is why the failure only showed on
CI. Open every iterated directory (.iterate = true).
* The runner shared mutable state across the test runner's thread pool:
load() lazily initialised the global cache arena unsynchronized (racing
allocations and a double listMethod FFI probe), scratch_seq was a racy
global whose range-based cleanup could delete another test's live fixture
copy, and runCase built request messages from the cache arena, which is not
thread-safe. Serialize load() with a spinlock, namespace scratch
directories per thread (scratch-{tid}-{n}) so cleanup only ever touches the
caller's own directories, and give each case a per-test request arena.
Verified locally (macOS, zig 0.16.0): zig fmt --check . clean and 25
consecutive zig build test runs green, 301/301 tests each.
Signed-off-by: Peefy <xpf6677@163.com>
… consistency The wasip1 artifact only puts fields 1-10 of LoadPackageResult on the wire, but the wrapper decodes all thirteen, so the shape is ready when the core starts emitting them. The consistency runner gains the LoadPackage case and reports the four pinned fields it cannot read rather than comparing them against a core that never sent them, and the two cases that take the process down — Test's panic=abort on an unsupported wasip1 syscall, and GetSchemaTypeMappingUnderPath's non-terminating walk — are probed or skipped deliberately, and said so in the README. Signed-off-by: Peefy <xpf6677@163.com>
Peefy
force-pushed
the
pr/consistency-29-cases
branch
from
October 8, 2026 03:45
b7bb599 to
e88737a
Compare
Three artifact behaviors shape the port: - The test runner (KclService.Test) never returns in the sandbox, and a pending wasmer call never settles, so the limit is declared statically instead of probed on a throwaway instance - the probe hung the suite. - The wasm32 loader qualifies every schema name whenever the sandbox holds more than one .k file, so each case now runs on a fresh instance seeded with only its own fixtures; the shared instance also OOMed mid-suite once the core's allocations accumulated (panic=abort). - LoadPackage emits kcl_mod but the sandboxed manifest parse leaves the package section empty, so those fields are reported, not asserted. Also restores the WASM_DIVERGENT skips the merge dropped (GenerateProto duplicates referenced schemas; GenerateDoc titles its section differently) and corrects the WASI-limitations notes the v0.13.1 artifact made stale. Signed-off-by: Peefy <xpf6677@163.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.
The cross-language consistency suite pins one kcl-api for all bindings and covers 25 of the 27 RPCs; every binding now dispatches all 29 manifest cases (not a 13-case subset) and asserts full coverage, with
formatread as absent rather than required where the core omits it (.NET, Kotlin).Merge order: #393 → #394 → #395 → #396 → #397.