Skip to content

Add impeller key to skia gold client, Turn on a framework test shard that will run unit tests with --enable-impeller #140985

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
Closed
jonahwilliams wants to merge 16 commits into from
Closed

Add impeller key to skia gold client, Turn on a framework test shard that will run unit tests with --enable-impeller #140985

jonahwilliams wants to merge 16 commits into from

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jan 4, 2024 •
edited
Loading

Adds a Skia Gold key that describes which Impeller renderer, if any is running, trying to leave the existing hashes unaltered.

Turns on a test shard that will run impeller on macOS, though this won't run without the PR being submitted first.

@jonahwilliams jonahwilliams marked this pull request as ready for review January 5, 2024 03:29
@jonahwilliams jonahwilliams changed the title turn on Impeller unittests. Turn on a framework test shard that will run unit tests with --enable-impeller Jan 5, 2024
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. a: desktop Running on desktop labels Jan 9, 2024
@jonahwilliams jonahwilliams changed the title Turn on a framework test shard that will run unit tests with --enable-impeller Add impeller key to skia gold client, Turn on a framework test shard that will run unit tests with --enable-impeller Jan 10, 2024
christopherfujino
christopherfujino reviewed Jan 10, 2024
View reviewed changes
.ci.yaml
timeout: 60
bringup: true
properties:
cpu: x86 # https://github.com/flutter/flutter/issues/119880
Copy link
Contributor

@christopherfujino christopherfujino Jan 10, 2024

Choose a reason for hiding this comment

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

should the name just be Mac_arm64 framework_tests_impeller instead? cc @godofredoc

christopherfujino
christopherfujino reviewed Jan 10, 2024
View reviewed changes
christopherfujino
christopherfujino reviewed Jan 10, 2024
View reviewed changes
dev/integration_tests/flutter_gallery/macos/Podfile.lock Outdated
PODFILE CHECKSUM: 2e6060c123c393d6beb3ee5b7beaf789de4d2e47

COCOAPODS: 1.12.1
COCOAPODS: 1.13.0
Copy link
Contributor

@christopherfujino christopherfujino Jan 10, 2024

Choose a reason for hiding this comment

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

I'm guessing revert this in case cocoapods on CI is older?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jan 10, 2024

Choose a reason for hiding this comment

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

Agh, yeah I didn't notice that - thank you

Jonah Williams and others added 2 commits January 10, 2024 11:49
@github-actions github-actions bot removed the a: desktop Running on desktop label Jan 10, 2024
christopherfujino
christopherfujino approved these changes Jan 10, 2024
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

Piinks
Piinks approved these changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jan 11, 2024

Ahh, @christopherfujino when I accepted your suggestion it added a co-authored commit with your gmail address, which doesn't have a google CLA 🤣

@jonahwilliams jonahwilliams mentioned this pull request Jan 11, 2024
Merged
@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jan 11, 2024

Redo here due to CLA issues: #141341

auto-submit bot pushed a commit that referenced this pull request Jan 11, 2024
Add impeller key to skia gold client, Turn on a framework test shard …
3590362 
…that will run unit tests with --enable-impeller (#141341)

Redo of #140985 due to CLA issues.
@christopherfujino
Copy link
Contributor

christopherfujino commented Jan 11, 2024

Ahh, @christopherfujino when I accepted your suggestion it added a co-authored commit with your gmail address, which doesn't have a google CLA 🤣

Yeah sorry. This is a bug in the clabot, it knows about both emails, but only when i'm the only author on the PR, apparently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Piinks Piinks Piinks approved these changes

+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 framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jonahwilliams @christopherfujino @Piinks