tests: harden skip-unused json2 test against helper refactors - #27537
Closed
enghitalo wants to merge 1 commit into
Closed
tests: harden skip-unused json2 test against helper refactors#27537enghitalo wants to merge 1 commit into
enghitalo wants to merge 1 commit into
Conversation
…nused test `test_skip_unused_keeps_json2_embedded_struct_decode_helpers` hard-coded the generated symbol `x__json2__create_value_from_optional_T_time__Time`. That helper is a json2 implementation detail: vlang#27535 replaces it with an inlined `$zero(field.typ.payload_type)`, after which the symbol no longer exists and the test fails. Assert `x__json2__Decoder_decode_value_T_time__Time` instead -- the generic instantiation that actually decodes the `?time.Time` payload of the embedded `Meta` struct, reachable only through comptime `$for field` codegen. It is the symbol skip-unused must keep (the point of issue vlang#26928), is present whether or not the json2 helpers exist, and so the test no longer breaks when they are refactored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Folding this into #27535 instead — the test fix now travels with the change that removes the helper. Closing in favor of that single PR. |
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.
Background
On #27535, the
chatgpt-codex-connectorreview flagged thatvlib/v/tests/skip_unused/generic_fn_instantiation_pruning_test.vwould start failing. This PR fixes the underlying brittleness so the test no longer hard-codes a json2 implementation detail.The bug
test_skip_unused_keeps_json2_embedded_struct_decode_helpers(regression for #26928) asserts that-skip-unusedkeeps generic json2 helpers reachable only through comptime$for fieldcodegen. One assertion hard-coded the generated symbol of a specific helper:create_value_from_optionalis a json2 implementation detail. #27535 removes it (the option payload is now built inline with$zero(field.typ.payload_type)), so the symbol disappears and this assertion fails.Reproduction
Compile the test program to C and grep the symbols (this is exactly what the test does):
master:x__json2__create_value_from_optional_T_time__Timeis present → test passes.$zero/$newtype accessors #27535 branch: the symbol is gone →test_skip_unused_keeps_json2_embedded_struct_decode_helpersfails at that assertion.The fix
Assert the symbol that actually decodes the
?time.Timepayload of the embeddedMeta—x__json2__Decoder_decode_value_T_time__Time— instead of a specific helper name. In this programtime.Timeonly appears as that optional embedded field, so the instantiation is reachable only through comptime$for fieldcodegen — exactly the skip-unused guarantee #26928 is about. Crucially, it is emitted whether or not the json2 helpers exist, so the test no longer breaks when they are refactored.Testing
v -w test vlib/v/tests/skip_unused/generic_fn_instantiation_pruning_test.v→ passes both:master(json2 unchanged), and$zero/$newtype accessors #27535 branch (helper removed).This is independent of #27535 and green on
mastertoday; once it lands, #27535 just needs a rebase to pick it up.🤖 Generated with Claude Code