Skip to content

Make GatedReader a test-only symbol, and allow all bevy_asset tests to all run single threaded. #18473

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
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Make GatedReader a test-only symbol, and allow all bevy_asset tests to all run single threaded. #18473

wants to merge 3 commits into from

Conversation

andriyDev
Copy link
Contributor

Objective

  • Some tests have been disabled in single-threaded mode. Now they aren't!

Solution

  • The solution to allowing all tests to run single threaded is to make GatedReader await for gates to open rather than blocking on it.
  • This however requires adding async-channel (already in tree). To avoid this additional dependency in most cases, I made this a dev-dependency.
  • However, this now means that GatedReader can only be used in tests (in bevy_asset). We could make this a feature flag, but it seems highly unlikely that real users need a GatedReader - and if they need it for their test, they can just fork the GatedReader.

Testing

  • The bevy_asset tests pass without setting the --features multi_threaded flag!

Migration Guide

bevy_asset::io::gated::GatedReader and bevy_asset::io::gated::GatedOpener are no longer accessible to users.

@andriyDev andriyDev marked this pull request as ready for review March 22, 2025 01:54
@andriyDev andriyDev force-pushed the no-multithreaded-dep branch from c382019 to 154d50a Compare March 22, 2025 01:55
@andriyDev andriyDev changed the title Make GatedReader a test-only symbol, and allow all tests to run single threaded. Make GatedReader a test-only symbol, and allow all bevy_asset tests to all run single threaded. Mar 22, 2025
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Testing A change that impacts how we test Bevy or how users test their apps D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Async Deals with asynchronous abstractions labels Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Testing A change that impacts how we test Bevy or how users test their apps D-Async Deals with asynchronous abstractions D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andriyDev @alice-i-cecile