Pulled out dir contents golden tool #50703
Conversation
| await step('Check output files...', () async { | ||
| // TODO(gaaclarke): We should move this into dir_contents_diff. | ||
| _withTemporaryCwd(contentsGolden, () { | ||
| final int exitCode = dirContentsDiff(basename(contentsGolden), screenshotPath); |
There was a problem hiding this comment.
This prints out the patch relative to //testing/ which is annoying but not the end of the world. We should clean it up at some point.
There was a problem hiding this comment.
Oh, in that it uses print (or similar) on a failure?
There was a problem hiding this comment.
The problem is that it is executing git diff -p as a subprocess and printing out the output from that. When you use git diff -p the patch that is generated is relative to the CWD. We don't have the path to the repository at this point. We just have to path to the golden file. Another gotcha is that git diff will generate absolute paths if you run it from a CWD outside of a repo, which is no good. I looked for a while to find a flag to git diff that would fix it but gave up. If you are looking for some light reading, don't check out git help diff haha
matanlurey
left a comment
matanlurey
left a comment
There was a problem hiding this comment.
LGTM, could you just edit the README for scenario_app/bin and scenario_app/android to mention this new capability and the golden file/how to update it?
| await step('Check output files...', () async { | ||
| // TODO(gaaclarke): We should move this into dir_contents_diff. | ||
| _withTemporaryCwd(contentsGolden, () { | ||
| final int exitCode = dirContentsDiff(basename(contentsGolden), screenshotPath); |
There was a problem hiding this comment.
Oh, in that it uses print (or similar) on a failure?
Done. Updated |
|
re-LGTM. |
gaaclarke
added
the
autosubmit
…143615) flutter/engine@2eed3fb...c4fe6f0 2024-02-16 [email protected] Pulled out dir contents golden tool (flutter/engine#50703) 2024-02-16 [email protected] Roll Dart SDK from 21b9ee6f0a52 to fa66195a3814 (3 revisions) (flutter/engine#50732) 2024-02-16 [email protected] Roll Skia from 2919d86cad12 to 6ae5032133d0 (8 revisions) (flutter/engine#50729) 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 [email protected],[email protected],[email protected] 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
fixes flutter/flutter#143459
This also starts using it as part of scenario_app.
Testing: Is part of the testing infrastructure.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.