-
Notifications
You must be signed in to change notification settings - Fork 29.4k
Shard firebase_test_lab_tests #56594
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
Conversation
.cirrus.yml
Outdated
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.
please don't do it this way -- just use the same style the rest of the file does
we're trying to keep this file to as rigorously simple a style as we can
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.
Are you referring to adding a SHARD_TEMPLATE
for the environment or pushing the shard numbers down into the test?
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.
i mean using matrix
dev/bots/firebase_testlab.sh
Outdated
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.
it might be a good idea to move this to test.dart rather than having its own separate shell script.
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.
Indeed that would be better. I split the difference and dumbed down .cirrus.yml and pushed the subshard -> test logic into test.dart. firebase_testlab.sh still lives, though.
.cirrus.yml
Outdated
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.
I'd really much rather we just duplicate all the text. i think the burden on people who look at this file without knowing Yaml's more obscure features is going to be much greater than the burden on people who have to deal with duplicated text.
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.
just use the same style the rest of the file
Would you prefer #50306 be reverted? I would guess so based on #50306 (comment).
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.
Well, that does end up saving a lot of duplication, huh...
Ok, so long as we have just the one mechanism for doing this, I'm good. The syntax you use here is exactly the same as in #50306?
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.
dev/bots/test.dart
Outdated
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.
It might make sense to do the trick we use elsewhere that verifies that the right number of shards are actually given (by having the last shard be named something like 5_last
or something, I forget exactly how we do it).
Up to you.
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.
Description
Split firebase_test_lab_tests into 3 shards, one per integration test.
Related Issues
Fixes #56590
Checklist
///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change