feedback icon indicating copy to clipboard operation
feedback copied to clipboard

Added platform-aware golden image logic

Open caseycrogers opened this issue 4 years ago • 3 comments

:scroll: Description

  • Adds Windows-specific golden images
  • Uses the appropriate platform-specific image depending on current environment
  • Skips the test if the current platform is not yet supported (namely Linux)

There's a bug in testWidgets that makes group level test skipping not work. The bug is fixed in Flutter version 2.1.0 (https://github.com/flutter/flutter/pull/76174) but 2.1.0 isn't out on stable yet. We could release this PR now and clean it up a little when 2.1.0 is out (see todo note in PR) or we could wait until the bugfix is out, clean up the PR, and then merge.

:bulb: Motivation and Context

Golden image tests would fail trivially on any platform but OSX. I'm working in Windows and wanted working golden tests for my PRs against this repo.

:green_heart: How did you test it?

Ran the tests with spoofed platforms to verify skipping and image selection works as intended.

:pencil: Checklist

  • [x] I reviewed submitted code
  • [x] I added tests to verify changes
  • [x] I updated the docs if needed
  • [x] All tests passing
  • [x] No breaking changes

:crystal_ball: Next steps

  • Merge and release!
  • Update to resolve TODO once Flutter version 2.1.0 is out on stable

caseycrogers avatar May 12 '21 21:05 caseycrogers

@ueman flagging for review! This is a super small change to just the golden image tests.

caseycrogers avatar May 12 '21 21:05 caseycrogers

This is great! Do you mind pulling in the master branch? I just noticed that the tests aren't run on PRs and fixed it there.

On CI the golden image checks would have to be enabled on all platforms.

Though this raises the question how am I supposed to generate those images if I don't have access to one of the other platforms?

I believe it should be doable with a GitHub Workflow.

ueman avatar May 13 '21 05:05 ueman

@ueman I've updated this to pull from master and to remove a TODO regarding a bug in an older version of flutter!

Let me know if you need anymore next steps from me.

caseycrogers avatar May 19 '21 23:05 caseycrogers