Skip to content

Implement takeScreenshot and add driver test for Fuchsia#48611

Merged
blasten merged 2 commits intoflutter:masterfrom
blasten:fuchsia_screenshot_driver
Jan 14, 2020
Merged

Implement takeScreenshot and add driver test for Fuchsia#48611
blasten merged 2 commits intoflutter:masterfrom
blasten:fuchsia_screenshot_driver

Conversation

@blasten
Copy link
Copy Markdown

@blasten blasten commented Jan 11, 2020

  • Implements flutter screenshot targeting a Fuchsia device.
  • Driver test that takes a screenshot and diffs it against the golden one. (The caveat is that the test needs to run on a NUC so the screen size matches. Later, we can expand that support.)

cc @iskakaushik

@fluttergithubbot fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 11, 2020
@blasten blasten force-pushed the fuchsia_screenshot_driver branch from a18149f to aadb050 Compare January 11, 2020 00:46
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not super familiar with this flow, but it looks like you can set the file output name in the flutter screenshot command, but ResidentRunner.screenshot() hardcodes 'flutter.png'.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If it sounds reasonable, I can follow up with adding a defaultScreenshotName to Device and then let ResidentRunner.screenshot() read the value.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sg. it looks like the resident runner screenshot call is already using getUniqueFile, so Device might need both a defaultScreenshotName and a defaultScreenshotExt.

Comment thread packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart Outdated
Comment thread packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart Outdated
Comment thread packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart Outdated
Copy link
Copy Markdown
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

We shouldn't be committing any new binary files into the git repo. @Piinks might know if there is a way we could use skia gold here, or else it might require a secondary repo somewhere

@Piinks
Copy link
Copy Markdown
Contributor

Piinks commented Jan 13, 2020

@blasten and I talked offline. I believe there should be away to just plug in to the Gold dashboard. We might be able to use the flutter_goldens_client package directly, or go through the flutter_goldens package.. this hasn't been done before, and I would love to see if we can continue to extend the Gold platform.

@blasten blasten force-pushed the fuchsia_screenshot_driver branch from aadb050 to 0a2f7d0 Compare January 13, 2020 23:07
@blasten blasten force-pushed the fuchsia_screenshot_driver branch from 0a2f7d0 to 3adc791 Compare January 13, 2020 23:08
@blasten
Copy link
Copy Markdown
Author

blasten commented Jan 13, 2020

PTAL

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sg. it looks like the resident runner screenshot call is already using getUniqueFile, so Device might need both a defaultScreenshotName and a defaultScreenshotExt.

@blasten blasten merged commit b973352 into flutter:master Jan 14, 2020
@blasten blasten deleted the fuchsia_screenshot_driver branch January 14, 2020 20:29
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. customer: fuchsia tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants