Skip to content

Only run test_recursive_call() in CI #12568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Viicos wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Only run test_recursive_call() in CI #12568

Viicos wants to merge 2 commits into from
+5 −1

Conversation

@Viicos
Copy link
Member

@Viicos Viicos commented Nov 24, 2025

Change Summary

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Nov 24, 2025
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 24, 2025 •
edited
Loading

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: fe7b52e
Status: ✅  Deploy successful!
Preview URL: https://a1102f94.pydantic-docs.pages.dev
Branch Preview URL: https://vp-debug-recursive-test.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 24, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #12568 will improve performances by 5.8%

Comparing vp/debug-recursive-test (fe7b52e) with main (4890db6)

Summary

⚡ 2 improvements
✅ 209 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
⚡ test_list_of_ints_core_json 757.3 µs 720.4 µs +5.13%
⚡ test_list_of_strs_json_uncached 409.4 µs 386.9 µs +5.8%

@Viicos Viicos changed the title Debug recursive test Only run test_recursive_call() in CI Nov 24, 2025
@Viicos Viicos added relnotes-ignore Omit this PR from the release notes. and removed relnotes-fix Used for bugfixes. labels Nov 24, 2025
@Viicos Viicos requested a review from davidhewitt November 24, 2025 14:46
@github-actions
Copy link
Contributor

github-actions bot commented Nov 24, 2025

Coverage report

This PR does not seem to contain any modification to coverable code.

@davidhewitt
Copy link
Contributor

davidhewitt commented Nov 26, 2025

Can you explain again the conditions under which you get it to fail? Maybe I can fix this without needing to disable it from local testing (which IMO makes it easy for development to miss fixing the test).

@Viicos
Copy link
Member Author

Viicos commented Nov 26, 2025

Can you explain again the conditions under which you get it to fail? Maybe I can fix this without needing to disable it from local testing (which IMO makes it easy for development to miss fixing the test).

Setting the recursion limit to >= 750 on my end will create the segfault. I discussed this with Matthew, and he told me Rust doesn't have any guarantee of providing a correct "stack overflow" message (see rust-lang/rust#79935 (comment)), so my assumption is that on my end, it overflows on the Rust side with a lower limit (around 750) than the Python one I have (1000).

@Viicos Viicos mentioned this pull request Nov 28, 2025
Open
1 task
@davidhewitt davidhewitt mentioned this pull request Nov 30, 2025
Draft
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidhewitt davidhewitt Awaiting requested review from davidhewitt

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

relnotes-ignore Omit this PR from the release notes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Viicos @davidhewitt