Skip to content
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.

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

Fix racy doctests #6103

Merged
merged 1 commit into from Jun 12, 2023
Merged

Fix racy doctests #6103

merged 1 commit into from Jun 12, 2023

Conversation

K900
Copy link

@K900 K900 commented Jun 12, 2023

On very fast machines (in my case, a 7950X3D with gen4 storage) it's possible that the script will start running the tests before importlib figures out the files are there, and get a spurious ModuleNotFoundError instead. Explicitly flushing the cache before importing things seems to help it work consistently, at the cost of an extra 0.1s or so of runtime.

Change Summary

Related issue number

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @PrettyWood

skip change file check

On very fast machines (in my case, a 7950X3D with gen4 storage) it's possible that the script will start running the tests before importlib figures out the files are there, and get a spurious `ModuleNotFoundError` instead. Explicitly flushing the cache before importing things seems to help it work consistently, at the cost of an extra 0.1s or so of runtime.
@K900
Copy link
Author

K900 commented Jun 12, 2023

Not sure if this is changelog worthy, being just a developer (and maybe distribution) facing change, so please review?

@adriangb adriangb enabled auto-merge (squash) June 12, 2023 21:02
@K900
Copy link
Author

K900 commented Jun 12, 2023

Also, I'm not sure if something like this is needed on 2.x, because I couldn't find where the doctest code moved to (if it wasn't removed entirely).

@K900
Copy link
Author

K900 commented Jun 12, 2023

Also, do I need to add a changelog entry for this or is there some way to skip the check?

@adriangb
Copy link
Member

I don't think this needs a changelog entry. You are editing the V2 files already :)

@K900
Copy link
Author

K900 commented Jun 12, 2023

I am not, this is for the 1.10.x-fixes branch.

@K900
Copy link
Author

K900 commented Jun 12, 2023

Also I think I figured out how to appease the changelog bot.

@K900
Copy link
Author

K900 commented Jun 12, 2023

Can you restart CI? I think github is having a moment...

@samuelcolvin
Copy link
Member

Simplest to just push an empty commit.

V2 docs code is tested with pytest-examples in the standard pytest run.

@K900
Copy link
Author

K900 commented Jun 12, 2023

It's failing with something that looks unrelated now :(

@samuelcolvin samuelcolvin merged commit 5d1ab9e into pydantic:1.10.X-fixes Jun 12, 2023
45 of 48 checks passed
@samuelcolvin
Copy link
Member

Thanks for this, force merged. Well look at fixing the fastapi issue soon.

@K900
Copy link
Author

K900 commented Jun 12, 2023

Thanks, appreciate the speedy response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants