Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Add a shard for Impeller Vulkan testing #36965

Merged
merged 6 commits into from
Oct 25, 2022

Conversation

iskakaushik
Copy link
Contributor

@iskakaushik iskakaushik commented Oct 24, 2022

There are several passing and failing tests. To prevent regressions while the work on Vulkan is in-progress, I've setup a filter for the passing tests. Eventually this will be replaced to run all the tests.

Addresses: flutter/flutter#112423

@chinmaygarde chinmaygarde changed the title Add a shard for Impeller Vulkan testing [Impeller] Add a shard for Impeller Vulkan testing Oct 24, 2022
@chinmaygarde
Copy link
Member

cc @zanderso

Copy link
Contributor

@godofredoc godofredoc left a comment

Choose a reason for hiding this comment

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

LGTM for the json builder file.

@@ -1054,6 +1070,19 @@ def main():
build_dir, engine_filter, args.coverage, args.engine_capture_core_dump
)

# Use this type to exclusively run impeller vulkan tests.
Copy link
Member

Choose a reason for hiding this comment

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

Since this looks like it will be temporary, is there a GitHub issue that can be commented here to track when we might be able to clean it up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done: filed flutter/flutter#113961 and added a ref in the comment

@iskakaushik iskakaushik force-pushed the impeller-vulkan-tests branch from 892c70e to 3244a32 Compare October 24, 2022 19:09
@flutter-dashboard
Copy link

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 (don't just cc him here, he won't see it! He's on Discord!).

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.

@iskakaushik iskakaushik force-pushed the impeller-vulkan-tests branch from bbec201 to 71fcb5b Compare October 25, 2022 15:11
@iskakaushik iskakaushik force-pushed the impeller-vulkan-tests branch from cc78a3c to 829d734 Compare October 25, 2022 17:20
# impeller vulkan tests are stable.
if 'impeller-vulkan' in types:
build_name = args.variant
xvfb.StartVirtualX(build_name, build_dir)
Copy link
Member

Choose a reason for hiding this comment

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

Consider making a contextmanager for this. Or at least put the Stop call in a finally.

@iskakaushik iskakaushik added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 25, 2022
@auto-submit auto-submit bot merged commit 40019b7 into flutter:main Oct 25, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller needs tests
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants