-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Updating cirrus fingerprint script to include goldens version #36690
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
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception. Reviewers: Read the Tree Hygine page and make sure this patch meets those guidelines before LGTMing. /cc @dnfield |
| } | ||
|
|
||
| Future<void> _checkCanSync() async { | ||
| final bool isCirrus = (platform.environment['CIRRUS_CI'] ?? '').isNotEmpty; |
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.
nit: I think we check this elsewhwer as just platform.environment.containsKey('CIRRUS_CI')
|
cc/ @LongCatIsLooong |
dnfield
left a 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.
LGTM
| flutter_pkg_cache: | ||
| folder: bin/cache/pkg | ||
| fingerprint_script: echo $OS; cat bin/internal/engine.version | ||
| fingerprint_script: echo $OS; cat bin/internal/*.version |
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 think you missed it for other platforms like Windows and macOS
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.
Ah good point. Goldens are currently only tested on Linux, but we should keep it consistent regardless. Thanks!
Description
Local golden file changes should not exist on Cirrus, but were introduced in the reverted PR #33688. This removes those local changes that are creating a sync error for the goldens repository on cirrus, and maintains a safe guard against further errant local changes, thereby preserving the testing environment.
The fingerprint script for cirrus has been updated to incorporate the goldens version when deciding to invalidate the cache or not.
Related Issues
This blocks #36087 and any further updates to the goldens repository.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?