Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 7, 2021

Revert #85288 , because it changes how error codes are propagated.

Adds a test that proves that exit codes are roughly correct ( oh boy why didn't we have this one before)

Fixes #85989

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 7, 2021
@google-cla google-cla bot added the cla: yes label Jul 7, 2021
@jonahwilliams jonahwilliams requested a review from yjbanov July 7, 2021 03:27
@jonahwilliams jonahwilliams requested a review from zanderso July 7, 2021 15:38
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ questions.

Directory tempDir;

setUp(() {
tempDir = createResolvedTempDirectorySync('exit_code_test.');
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be flutter_exit_code_test. or is the flutter_ added by createResolvedTempDirectorySync?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup -

final Directory tempDirectory = fileSystem.systemTempDirectory.createTempSync('flutter_$prefix');

]);
expect(result.stdout, contains('args: ["(?!Golden).+"]'));
}, skip: !platform.isWindows);
}, skip: 'Reverted in https://github.com/flutter/flutter/pull/86000');
Copy link
Member

Choose a reason for hiding this comment

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

Huh. I didn't know this could be a string. Does this just act like true, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, but I think it will display in the test output

@skia-gold
Copy link

Gold has detected about 2 new digest(s) on patchset 5.
View them at https://flutter-gold.skia.org/cl/github/86000

@jonahwilliams
Copy link
Contributor Author

The customer tests : customer_testing-linux shard seems to be stuck in a bad git state. In theory this change shouldn't effect it so i'm going to land and worry about that later

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #86000 at sha f4d6264

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Jul 7, 2021
@jonahwilliams
Copy link
Contributor Author

Landing to prevent more breakages from creeping in the tree

@jonahwilliams jonahwilliams merged commit b1b0d20 into flutter:master Jul 7, 2021
@jonahwilliams jonahwilliams deleted the fix_batch_exit_code branch July 7, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All Windows shards are silently failing on framework LUCI

3 participants