Skip to content

test(pydantic): remove stale xfail from field extension default test - #4787

Open
nikhillakra2007-tech wants to merge 3 commits into
strawberry-graphql:mainfrom
nikhillakra2007-tech:fix/829-unxfail-default-test
Open

nikhillakra2007-tech wants to merge 3 commits into
strawberry-graphql:mainfrom
nikhillakra2007-tech:fix/829-unxfail-default-test

Conversation

@nikhillakra2007-tech

@nikhillakra2007-tech nikhillakra2007-tech commented Nov 3, 2026 •

Copy link
Copy Markdown

Back in 2021 this test was marked as an expected failure because default values didn't work when extending a pydantic type (#829). That was fixed not long after and the issue was closed, but the marker stayed, and the test's field-order assertion (model fields first) no longer matches the behavior covered by the sibling nested-fields test, where extension fields come first.

This updates the assertion to the actual order and drops the marker, so the test now runs for real and confirms defaults work.

Test plan:

  • The updated test passes without the marker.
  • Pydantic suite: 158 passed, 11 skipped, 0 xfailed (previously 157 passed, 11 skipped, 1 xfailed).
  • Ruff format and checks pass.

No RELEASE.md since this is a test-only change - happy to add one if you'd like.

Summary by Sourcery

Tests:

  • Enable the Pydantic field-extension default test by removing its stale expected-failure marker and aligning its field-order assertions with current behavior.

Default values on fields declared in the class body of a pydantic type
have worked since strawberry-graphql#826, and strawberry-graphql#829 was closed as fixed in 2021. The test
kept its xfail marker and a stale field-order expectation (model fields
first) that contradicts both current behavior and the sibling
nested-fields test, which asserts extension fields come first.

Update the assertion to the order verified by the sibling test and
remove the marker, so the suite verifies the default behavior directly.
@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This test-only change appears safe to merge.

Summary

Removes the stale xfail marker from the Pydantic extension-field default test.

  • The Pydantic default-field test now runs with the expected field order.

Reviews (1) · Last reviewed commit: "test(pydantic): un-xfail field extension..." · Reviewed by Greptile

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/experimental/pydantic/test_basic.py" line_range="430" />
<code_context>
     assert empty_list == []


-@pytest.mark.xfail(
</code_context>
<issue_to_address>
**Release notes are missing**

`RELEASE.md` is absent from this PR, so the required release description is missing for the automated release process.

Add a `RELEASE.md` describing the change.
</issue_to_address>

Sourcery assessment

Approval pending. 1 finding to address first.

Blocking findings: tests/experimental/pydantic/test_basic.py:430


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for adding the RELEASE.md file!

Below is the changelog that will be used for the release.


This release fixes the Pydantic field-extension default test by removing its
stale expected-failure marker and updating its field-order assertion to the
current behaviour. Default values have worked since the original issue was
fixed; no library behaviour changes.

This release was contributed by @nikhillakra2007-tech in #4787

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant