-
Notifications
You must be signed in to change notification settings - Fork 499
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 #4255: Double number of shards available for app tests #4313
Conversation
App module tests are still running into memory issues (specifically shards 2 & 3). This change doubles the number of shards available so that hopefully the tests are better distributes and have a smaller chance of hitting memory limitations.
@anandwana001 PTAL when you get a chance (this isn't urgent, but it'll be nice to get in since I have to occasionally restart CI workflows at the moment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks! |
Ah, I apparently didn't do this correctly since the latest CI only ran 4. I suspect that I need to change something in the workflow, too. |
Ensure new shards are run in CI.
Ah there we go. Needed to make sure all of the new shards are being run (otherwise it's more or less cutting the app module tests in half). |
Welp looks like the same problem (exit code 134) happens even with 8 shards. It seems that this happened with StateFragmentLocalTest, so I'm wondering if we'll need to separate out the two state fragment tests into their own special shard since they clearly use far too many resources to be shared with a bunch of other app module tests. That being said, I'm going to restart the workflow for this PR and try to merge it since 8 shards is still better than 4 for restarting, and hopefully lowers the chance of the Gradle tests failing. |
Explanation
Fixes #4255
App module tests are still running into memory issues (specifically shards 2 & 3). This change doubles the number of shards available so that hopefully the tests are better distributes and have a smaller chance of hitting memory limitations.
Essential Checklist
For UI-specific PRs only
N/A -- This is an infrastructure change that doesn't affect app behavior.