🧪 test_fields: assert structure, not repr - #7453
Closed
GeigerJ2 wants to merge 1 commit into
Closed
Conversation
The field regression tests compared `repr()` of each `QbField` against a
YAML reference file, which pinned the formatting of `__repr__` rather
than the field schema.
`__repr__` renders `backend_key`, which folds `is_attribute` into an
`attributes.` prefix, so the flag was never recorded and the encoding is
not injective: a field with key `exception` and `is_attribute=True` and
one with key `attributes.exception` and `is_attribute=False` render
identically. The reference files could not tell them apart.
It was also version-sensitive, since `repr()` of `typing` constructs is
not stable across releases, which is what forced the `skipif < 3.14`
guard. And it tied the files to a presentation detail: changing
`__repr__` rewrites all 46 references, which discourages fixing it.
Build the description in the test instead. The field key is dropped from
it, being the mapping key already, and `backend_key`, `is_attribute` and
`root` are emitted only where they carry information, which keeps the
files roughly the size they were:
levelname: QbStrField(dtype=str, doc='The name of the log level')
exception: QbStrField(backend_key=attributes.exception,
is_attribute=True, dtype=str | None, root=str, doc='...')
`backend_key` differs from the key exactly when `is_attribute` is set,
since a non-attribute field may not be aliased, so both appear together
or not at all.
Dtypes go through `format_dtype`, stable across 3.10-3.14 by
construction rather than by whatever `typing` does this release. Its
`get_origin` guard is load-bearing: `isinstance(dict[str, Any], type)`
is `True` on 3.10 alone, which would render a bare `dict` there and
`dict[str, Any]` from 3.11 on.
`root` pins `extract_root_type`, previously covered only through the
`QbField` subclass it selects. Breaking its union branch now reports
`root=int -> root=Union` beside the class change rather than leaving the
cause to be inferred.
Regenerate all 46 reference files. The diff is large but mechanical: no
field changed, only how it is described.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7453 +/- ##
==========================================
- Coverage 80.61% 80.60% -0.01%
==========================================
Files 580 580
Lines 46722 46722
==========================================
- Hits 37662 37657 -5
- Misses 9060 9065 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
Closing this for now, in favor of work on #7461. |
Member
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
2 participants
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.
Possible follow-up to #7448.
#7448 re-enabled the node field fixtures, which a filter had been silently excluding from
test_all_node_fields, and regenerated the eight files it had orphaned. Doing that showed they had drifted, and that__repr__lost some field information along the way:is_attributeis no longer in it, and it printsbackend_keywhere__init__takeskey, soQbStrField('exception', is_attribute=True)andQbStrField('attributes.exception', is_attribute=False)come out identical. Pinningrepr()oftypingconstructs is also what forced theskipif < 3.14guard.