fix(test): grouped keyword unions schema into objects of smaller size to allow CPython compilation - #3125
Open
carloscasellas wants to merge 1 commit into
Open
Conversation
… to allow CPython compilation
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.
Description
Restructures the
test/inputs/schema/keyword-unions.schemafixture (and the generator that produces it,test/keywords.py) so that its ~275 top-level union properties are split into 5 nestedobjNobjects of at most 64 properties each, instead of one flat object with all 275 properties. Updates thekeyword-unions.2.jsonsample to nest its value underobj1to match the new shape, and removes the Python-specificskipSchemaentry that previously excluded this fixture from the Python test matrix.Related Issue
Fixes #3119
Motivation and Context
keyword-unions.schemagenerates one Python class per top-level object, with a positional-argument constructor call (from_dict) that takes one argument per property. With all 275 properties on a single flat object, quicktype generated a 276-argument constructor call, which exceeds CPython's hard limit of 255 arguments per call — the generated Python module fails to compile/import with aSyntaxError.Because of this, the fixture was permanently skipped for Python (
test/languages.ts), which meant there was no test coverage catching this class of bug: any real-world schema wide enough to trip the same 255-argument ceiling would silently produce broken, unusable Python from quicktype, with nothing in CI to catch it.Grouping the schema's properties into smaller nested objects keeps the widest generated constructor call well under the 255-argument limit while still exercising every keyword/property the fixture is meant to cover, so the fixture can now run — and stay covered — for Python.
Previous Behaviour / Output
test/languages.tsexplicitly skipped this schema for Python:Generating Python from the (unskipped) flat schema produced a 276-argument call site, e.g.:
which raises
SyntaxError: more than 255 argumentsat import/call time — the generated code was never actually valid Python.New Behaviour / Output
The schema now groups its properties into 5 nested objects (
obj1..obj5) of up to 64 properties each. The generated Python's widest constructor call site is now 65 arguments (Obj1), well under the 255-argument ceiling:test/languages.ts'sskipSchemano longer excludeskeyword-unions.schemafor Python (skipSchema: []), so the fixture now runs as part of the normal Python test matrix.How Has This Been Tested?
test/keywords.py --unionsand diffed it against the committedtest/inputs/schema/keyword-unions.schema— byte-for-byte match, confirming the generator and the checked-in fixture are in sync.npm run build) and rannode dist/index.js --lang python --src-lang schema test/inputs/schema/keyword-unions.schemadirectly; confirmed the widest generated constructor call is 65 arguments (Obj1), and thatpython3 -m py_compilesucceeds on the output.mypylocally (not present by default in this environment) and ran the actual fixture test end-to-end:FIXTURE=schema-python QUICKTEST=true npm run test:fixtures -- test/inputs/schema/keyword-unions.schema— passes, exercising both the.1.json({}) and.2.json({ "obj1": { "constructor": 123 } }) samples through the full generate → compile/typecheck → round-trip pipeline, not just a static check.keyword-unions.2.json's new nested shape ({ "obj1": { "constructor": 123 } }) is still valid against the schema, and thatkeyword-unions.1.json's{}remains valid since the schema has norequiredproperties at any level.test/languages.tsfor other languages referencingkeyword-unions.schema: all other languages that skip it do so for unrelated, pre-existing reasons (naming collisions, memory limits, etc.) untouched by this change, so no other language's test configuration was affected.Screenshots (if appropriate):
N/A — this is a test-fixture/schema change with no visual output.