Skip to content

Run test imports at top level, fix pydantic and numpy imports #2324

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

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

garrettgu10
Copy link
Contributor

Importing stuff within a test() function does not invoke the top-level entropy patches we implemented, so this change should make our testing much more complete.

Should reproduce cloudflare/python-workers-examples#9.

@garrettgu10 garrettgu10 requested review from a team as code owners June 24, 2024 20:30
@garrettgu10 garrettgu10 requested review from tewaro and jp4a50 June 24, 2024 20:30
@garrettgu10 garrettgu10 force-pushed the ggu/top-level-import-tests branch from 89d8b61 to 7ba06c2 Compare June 24, 2024 20:34
Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

At the risk of doubling our CI time... I wonder if we should test both scenarios.

@garrettgu10
Copy link
Contributor Author

@dom96 I've thought about it, but my take is that the process of importing at top level is basically a superset of importing not-at-top-level, so it's acceptable to only test top level imports.

Maybe @hoodmane has more thoughts?

@garrettgu10 garrettgu10 changed the title Run test imports at top level Run test imports at top level, fix pydantic and numpy. imports Jul 1, 2024
@garrettgu10 garrettgu10 changed the title Run test imports at top level, fix pydantic and numpy. imports Run test imports at top level, fix pydantic and numpy imports Jul 1, 2024
@garrettgu10 garrettgu10 merged commit ef9869e into main Jul 1, 2024
9 checks passed
@hoodmane hoodmane deleted the ggu/top-level-import-tests branch July 1, 2024 20:37
ns476 pushed a commit to ns476/workerd that referenced this pull request Aug 2, 2024
…lare#2324)

* Run test imports at top level

* Fix the problem

---------

Co-authored-by: Hood Chatham <[email protected]>
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.

3 participants