This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Remove --enable-software-rendering from iOS scenario tests #45093
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zanderso
force-pushed
the
rm-ios-scenario-software-renderer
branch
8 times, most recently
from
5d0fa1e to
7aba939
Compare
chinmaygarde
approved these changes
Aug 25, 2023
zanderso
force-pushed
the
rm-ios-scenario-software-renderer
branch
from
7aba939 to
8964e98
Compare
|
@dnfield This PR turns off software rendering except for a bunch of the platform view tests, where neither Skia nor Impeller GPU backends work. It also corrects the usage of the |
zanderso
force-pushed
the
rm-ios-scenario-software-renderer
branch
from
8964e98 to
b0ac5a8
Compare
zanderso
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 25, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Aug 25, 2023
…133355) flutter/engine@1471967...1ec7b89 2023-08-25 zanderso@users.noreply.github.com Remove --enable-software-rendering from iOS scenario tests (flutter/engine#45093) 2023-08-25 skia-flutter-autoroll@skia.org Roll Skia from 56bb647a49ac to 76672468e8d7 (3 revisions) (flutter/engine#45121) 2023-08-25 yatendraroria2001@gmail.com Fix: Complete Documentation for RasterStatus::kSkipAndRetry (flutter/engine#44880) 2023-08-25 30870216+gaaclarke@users.noreply.github.com [Impeller] Updated TextureSourceVK docs and deleted unused ivars (flutter/engine#45123) 2023-08-25 zanderso@users.noreply.github.com Revert "Fix global tests doing nothing." (flutter/engine#45125) 2023-08-25 skia-flutter-autoroll@skia.org Roll Skia from ba7c5258d2b4 to 56bb647a49ac (3 revisions) (flutter/engine#45118) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jimgraham@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke
pushed a commit
to gaaclarke/engine
that referenced
this pull request
Aug 30, 2023
8 tasks
auto-submit bot
pushed a commit
that referenced
this pull request
Sep 7, 2024
Follow-up to #45093. Remove software rendering for the golden/screenshot compare iOS scenario tests. Throw an exception if this flag is passed into the test app. Delete `RenderingSelectionTest`. This did require updating some screenshots, so there are changes. Note, these golden tests are still not running on Impeller, but I'm going to follow-up in #55012. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
auto-submit bot
pushed a commit
that referenced
this pull request
Sep 9, 2024
The iOS golden scenario tests are supposed to run on Skia and Impeller, but are actually running on Skia twice. #### Issue 1: script-side Regression from #46329. Currently, the tests log "Running simulator tests with Impeller" but then you see that it's not: ``` Running simulator tests with Impeller ... 2024-09-06 13:38:07.692810-0700 Scenarios[73857:470266] [IMPORTANT:flutter/shell/common/shell.cc(456)] [Action Required] The application opted out of Impeller by either using the --no-enable-impeller flag or FLTEnableImpeller=false plist flag. This option is going to go away in an upcoming Flutter release. Remove the explicit opt-out. If you need to opt-out, report a bug describing the issue. ``` https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8737514337106072353/+/u/test:_Scenario_App_Integration_Tests/stdout `INFOPLIST_FILE="Scenarios/Info_Impeller.plist"` isn't passed into the xcodebuild any more. I confirmed #46329 caused this by inserting a comment in the middle of the `xcodebuild` command, which caused the last argument after the comment to not be passed in. #### Issue 2: test-side Regression from #45093. [The logic deciding whether to use Skia or Impeller goldens is incorrect](https://github.com/flutter/engine/blob/397987aedeb4ed41ec945fcbe9d347e579513bf4/testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m#L84-L90) since `FLTEnableImpeller` is set in the app Info.plist, not the UI test, so the bundle should be "dev.flutter.Scenarios". That means even if the script was passing in the right Info.plist, the tests would fail since they would have compared the Skia screenshots. #### This PR 1. Once #55016 merges (edit: done), these tests will no longer be running with the software renderer. Remove all the test skips so they all run on Impeller. 2. Add the missing Impeller golden screenshots. 3. Update the default Info.plist to use Impeller, so if another script mishap happens, it will default to testing Impeller twice, and not Skia. Add a Skia Info.plist variant instead. 4. Update the test logic to check the right bundle ID Info.plist to decide whether to compare against the Skia or Impeller screenshots. Prefer Impeller so if another test-side mishap happens, it will also default to testing Impeller and not Skia. It will only use the Skia goldens if the bool is set in the Info.plist, and that bool is NO (not just if it's missing). 5. All this made the now-default Impeller tests pass, but when passing in `INFOPLIST_FILE="Scenarios/Info_Skia.plist"` the app wasn't launching, with the error: > Scenarios encountered an error (Failed to install or launch the test runner. (Underlying Error: Simulator device returned an error for the requested operation. Failed to create promise. (Underlying Error: Failed to create app extension placeholder for /Users/chrome-bot/Library/Developer/Xcode/DerivedData/Scenarios-aypjgouuxctxctfazxalyegcximf/Build/Products/Debug-iphonesimulator/Scenarios.app/PlugIns/ScenariosShare.appex. Failed to create promise. (Underlying Error: Failed to set placeholder attributes dev.flutter.Scenarios.ScenariosShare. Failed to create promise. (Underlying Error: extensionDictionary must be set in placeholder attributes for an app extension placeholder. Invalid placeholder attributes.))))) This is identical to the error in #53717. I finally realized it was because the ScenariosShare app extension requires specific extension keys in its plist, which weren't present in the app Info.plist at `Scenarios/Info_Skia.plist`. So I changed the `INFOPLIST_FILE` path to `$(TARGET_NAME)/Info_Skia.plist` so it would resolve to target-specific copies like `Scenarios/ScenariosShare/Info_Skia.plist`. That meant I had to add a few more copies where they didn't exist in the target path. Dependent on #55016 landing. Fixes flutter/flutter#131888 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
jesswrd
pushed a commit
to jesswrd/engine
that referenced
this pull request
Sep 11, 2024
Follow-up to flutter#45093. Remove software rendering for the golden/screenshot compare iOS scenario tests. Throw an exception if this flag is passed into the test app. Delete `RenderingSelectionTest`. This did require updating some screenshots, so there are changes. Note, these golden tests are still not running on Impeller, but I'm going to follow-up in flutter#55012. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
jesswrd
pushed a commit
to jesswrd/engine
that referenced
this pull request
Sep 11, 2024
The iOS golden scenario tests are supposed to run on Skia and Impeller, but are actually running on Skia twice. #### Issue 1: script-side Regression from flutter#46329. Currently, the tests log "Running simulator tests with Impeller" but then you see that it's not: ``` Running simulator tests with Impeller ... 2024-09-06 13:38:07.692810-0700 Scenarios[73857:470266] [IMPORTANT:flutter/shell/common/shell.cc(456)] [Action Required] The application opted out of Impeller by either using the --no-enable-impeller flag or FLTEnableImpeller=false plist flag. This option is going to go away in an upcoming Flutter release. Remove the explicit opt-out. If you need to opt-out, report a bug describing the issue. ``` https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8737514337106072353/+/u/test:_Scenario_App_Integration_Tests/stdout `INFOPLIST_FILE="Scenarios/Info_Impeller.plist"` isn't passed into the xcodebuild any more. I confirmed flutter#46329 caused this by inserting a comment in the middle of the `xcodebuild` command, which caused the last argument after the comment to not be passed in. #### Issue 2: test-side Regression from flutter#45093. [The logic deciding whether to use Skia or Impeller goldens is incorrect](https://github.com/flutter/engine/blob/397987aedeb4ed41ec945fcbe9d347e579513bf4/testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m#L84-L90) since `FLTEnableImpeller` is set in the app Info.plist, not the UI test, so the bundle should be "dev.flutter.Scenarios". That means even if the script was passing in the right Info.plist, the tests would fail since they would have compared the Skia screenshots. #### This PR 1. Once flutter#55016 merges (edit: done), these tests will no longer be running with the software renderer. Remove all the test skips so they all run on Impeller. 2. Add the missing Impeller golden screenshots. 3. Update the default Info.plist to use Impeller, so if another script mishap happens, it will default to testing Impeller twice, and not Skia. Add a Skia Info.plist variant instead. 4. Update the test logic to check the right bundle ID Info.plist to decide whether to compare against the Skia or Impeller screenshots. Prefer Impeller so if another test-side mishap happens, it will also default to testing Impeller and not Skia. It will only use the Skia goldens if the bool is set in the Info.plist, and that bool is NO (not just if it's missing). 5. All this made the now-default Impeller tests pass, but when passing in `INFOPLIST_FILE="Scenarios/Info_Skia.plist"` the app wasn't launching, with the error: > Scenarios encountered an error (Failed to install or launch the test runner. (Underlying Error: Simulator device returned an error for the requested operation. Failed to create promise. (Underlying Error: Failed to create app extension placeholder for /Users/chrome-bot/Library/Developer/Xcode/DerivedData/Scenarios-aypjgouuxctxctfazxalyegcximf/Build/Products/Debug-iphonesimulator/Scenarios.app/PlugIns/ScenariosShare.appex. Failed to create promise. (Underlying Error: Failed to set placeholder attributes dev.flutter.Scenarios.ScenariosShare. Failed to create promise. (Underlying Error: extensionDictionary must be set in placeholder attributes for an app extension placeholder. Invalid placeholder attributes.))))) This is identical to the error in flutter#53717. I finally realized it was because the ScenariosShare app extension requires specific extension keys in its plist, which weren't present in the app Info.plist at `Scenarios/Info_Skia.plist`. So I changed the `INFOPLIST_FILE` path to `$(TARGET_NAME)/Info_Skia.plist` so it would resolve to target-specific copies like `Scenarios/ScenariosShare/Info_Skia.plist`. That meant I had to add a few more copies where they didn't exist in the target path. Dependent on flutter#55016 landing. Fixes flutter/flutter#131888 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.