Skip to content

Remove Cirrus support for Gold #67468

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
Merged
Piinks merged 2 commits into from
Oct 8, 2020
Merged

Remove Cirrus support for Gold #67468

Piinks merged 2 commits into from
Oct 8, 2020

Conversation

@Piinks
Copy link
Contributor

@Piinks Piinks commented Oct 6, 2020 •
edited
Loading

Description

Golden file testing is running across all platforms on LUCI now, making the Cirrus implementation no longer necessary. A few framework builds were added back to Cirrus to serve as smoke tests, but don't need to be running duplicate golden file tests. These builds were added back without the right Gold config, so PRs making a golden file change right now are blocked.

Since we are using LUCI, with this change we will no longer need to handle the special case of first time contributors, so this removes all of the ignore logic from the Gold implementation as well. :)

TODO:

  • Update the wiki to remove first time contributor guidance since the issue will be resolved

Related Issues

Fixes #67463

Tests

Updated for a Cirrus-free lifestyle.

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.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.

@google-cla google-cla bot added the cla: yes label Oct 6, 2020
@Piinks Piinks added framework flutter/packages/flutter repository. See also f: labels. and removed cla: yes labels Oct 6, 2020
@google-cla google-cla bot added the cla: yes label Oct 6, 2020
@Piinks Piinks added a: tests "flutter test", flutter_test, or one of our tests and removed cla: yes labels Oct 6, 2020
@google-cla google-cla bot added the cla: yes label Oct 6, 2020
@Piinks Piinks added c: contributor-productivity Team-specific productivity, code health, technical debt. team-infra Owned by Infrastructure team and removed cla: yes labels Oct 6, 2020
@google-cla google-cla bot added the cla: yes label Oct 6, 2020
@Piinks
Copy link
Contributor Author

Piinks commented Oct 6, 2020

FYI @kjlubick We can can cross off another manual request point. :)

Piinks
Piinks commented Oct 6, 2020
View reviewed changes
Piinks
Piinks commented Oct 6, 2020
View reviewed changes
@christopherfujino
Copy link
Contributor

christopherfujino commented Oct 7, 2020

does LUCI get goldctl via cipd?

@Piinks
Copy link
Contributor Author

Piinks commented Oct 7, 2020

does LUCI get goldctl via cipd?

It does, but that is in the luci recipe itself, I think only cirrus builds get goldctl using the scripts I'm deleting. Here's the luci recipe that ensures goldctl: https://flutter.googlesource.com/recipes/+/refs/heads/master/recipes/flutter.py#71

This was referenced Oct 7, 2020
Merged
Closed
@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 6.
View them at https://flutter-gold.skia.org/cl/github/67468

christopherfujino
christopherfujino approved these changes Oct 7, 2020
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xster
Copy link
Member

xster commented Oct 7, 2020

Thanks for digging through this @Piinks

goderbauer
goderbauer approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flutter-dashboard
Copy link

flutter-dashboard bot commented Oct 7, 2020

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #67468 at sha 38d8a24e8265da174af30ceb3dcef4ef3874a36c

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Oct 7, 2020
@Piinks Piinks mentioned this pull request Oct 7, 2020
Closed
@Piinks
Copy link
Contributor Author

Piinks commented Oct 7, 2020

While validating this on the gold dashboard, I noticed there wasn't a windows image. The Windows build here passed, but the logs don't show any tests being run, instead: ERROR: The process "flutter_tester.exe" not found.
Filed #67540

@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 7.
View them at https://flutter-gold.skia.org/cl/github/67468

@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 8.
View them at https://flutter-gold.skia.org/cl/github/67468

@flutter-dashboard
Copy link

flutter-dashboard bot commented Oct 7, 2020

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #67468 at sha cd09e13f8bfc61c9dfa20c58e82fb3485ae4841c

@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 9.
View them at https://flutter-gold.skia.org/cl/github/67468

@flutter-dashboard
Copy link

flutter-dashboard bot commented Oct 7, 2020

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #67468 at sha d5496f96deb9acc20a5a7799473f69197d1bd229

@Piinks Piinks mentioned this pull request Oct 7, 2020
Closed
@Piinks
Copy link
Contributor Author

Piinks commented Oct 7, 2020

This is blocked on #67560

@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 10.
View them at https://flutter-gold.skia.org/cl/github/67468

@skia-gold
Copy link

skia-gold commented Oct 7, 2020

Gold has detected about 1 untriaged digest(s) on patchset 11.
View them at https://flutter-gold.skia.org/cl/github/67468

@Piinks
Copy link
Contributor Author

Piinks commented Oct 7, 2020

This appears to be blocked by a different issue now. The Mac build tests are having version solving issues. I reset my git history with master to see if that would resolve it, but it has not, investigating.

@Piinks Piinks merged commit 72696f7 into flutter:master Oct 8, 2020
@Piinks Piinks mentioned this pull request Oct 12, 2020
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

@godofredoc godofredoc Awaiting requested review from godofredoc

+1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. team-infra Owned by Infrastructure team will affect goldens Changes to golden files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cirrus implementation of Gold needs to be removed

5 participants

@Piinks @christopherfujino @skia-gold @xster @goderbauer