-
Notifications
You must be signed in to change notification settings - Fork 6k
Firebase test for Platform Views on iOS #11350
Conversation
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.
--results-dir=engine_scenario_test/$GIT_REVISION/$BUILD_ID \ | ||
|
||
|
||
popd |
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: pushd
s and popd
s don't balance. Is that intentional?
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.
No. Will fix
if (golden) { | ||
XCTAttachment* goldenAttachment = [XCTAttachment attachmentWithImage:golden]; | ||
goldenAttachment.lifetime = XCTAttachmentLifetimeKeepAlways; | ||
[self addAttachment:goldenAttachment]; | ||
} else { | ||
NSLog(@"This test will fail - no golden named %@ found. Follow the steps in the README to add " |
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.
XCTFail?
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 want it to continue so it will generate the potentially good screenshot for you
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 guess I could generate the good screen shot, attach it, and fail though - which would fail slighty faster than I currently am. WDYT?
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.
It would be better for the test failure to suggest what to do than have to dig through logs.
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
git@github.com:flutter/engine.git/compare/ab097a59faf5...1efb5b7 git log ab097a5..1efb5b7 --no-merges --oneline 2019-08-21 dnfield@google.com update sim script (flutter/engine#11355) 2019-08-21 dnfield@google.com Firebase test for Platform Views on iOS (flutter/engine#11350) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (liyuqian@google.com), and stop the roller if necessary.
Once this can land, I'll update the recipe to run this in addition to the simulator tests.
Then I think I'm good to land #11070