-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
Pin the customer_tests #83964
Pin the customer_tests #83964
Conversation
bad6645
to
df3d568
Compare
This needs to be applied to the LUCI version of the shard too, but I am not quite sure how to do that. cc @godofredoc |
df3d568
to
f982ca6
Compare
f982ca6
to
25c527c
Compare
Gold has detected about 35 new digest(s) on patchset 1. |
25c527c
to
7f54dc7
Compare
Apparently you need to be allow-listed to test or propose changes to the recipes repo, so I filed a bug to do that instead. #85665 |
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
test-exempt: is a test |
- (cd dev/customer_testing; pub get) | ||
# Cirrus doesn't give us the master branch, so we have to fetch it for ourselves, | ||
# otherwise we won't be able to figure out how old or new our current branch is. | ||
- git fetch origin master:master |
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.
Can the shared logic with luci be moved into a separate script? If that's available, i'll happily send a recipes CL so we no longer need to rely on the adhoc script there.
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.
Possibly. The logic here is sort of Cirrus-specific because Cirrus does weird stuff with the checkout, no idea what the impact of running those steps on LUCI would be.
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.
That makes sense. In general, infra logic always handles the source checkout. Whereas
- (cd dev/tools; pub get)
- (cd dev/customer_testing; pub get)
- rm -rf bin/cache/pkg/tests
- git clone https://github.com/flutter/tests.git bin/cache/pkg/tests
- git -C bin/cache/pkg/tests checkout `dart --enable-asserts dev/tools/bin/find_commit.dart bin/cache/pkg/tests`
- dart --enable-asserts dev/customer_testing/run_tests.dart --skip-on-fetch-failure --skip-template bin/cache/pkg/tests/registry/*.test
look like they can be shared
4cc94a5
to
00dfc8e
Compare
# revved on flutter/tests -- if you rerun a passing customer_tests | ||
# shard, it should still pass, even if we rolled one of the tests.) | ||
rm -rf ../../bin/cache/pkg/tests | ||
git clone https://github.com/flutter/tests.git ../../bin/cache/pkg/tests |
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.
We'll also need a windows version of this file (or one Dart file)
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 have no way to test it that I can think of but I tried to create one. Let me know if it works? :-)
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.
179e05e
to
2ecc74a
Compare
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.
2ecc74a
to
ebb6724
Compare
This comment has been minimized.
This comment has been minimized.
ebb6724
to
84d5a4b
Compare
This makes sure we keep using the same customer_tests shard as existed when a branch was created, rather than always tracking HEAD. Also, this moves the logic from cirrus.yml to a ci.sh script that we can reuse from other CI configuration.
84d5a4b
to
e00b34d
Compare
This makes sure we keep using the same customer_tests shard as existed when a branch was created, rather than always tracking HEAD.