Skip to content

Include git output into error message from channel command#28658

Merged
alexmarkov merged 2 commits intoflutter:masterfrom
alexmarkov:git-output-from-channel
Mar 1, 2019
Merged

Include git output into error message from channel command#28658
alexmarkov merged 2 commits intoflutter:masterfrom
alexmarkov:git-output-from-channel

Conversation

@alexmarkov
Copy link
Contributor

@alexmarkov alexmarkov commented Feb 28, 2019 •
edited
Loading

Description

This change adds output of underlying 'git branch -r' command into the error message of "flutter channel". This is an attempt to debug flaky failures on head-head-head (dart/engine/flutter) bot:

21:21 +668 ~4 -1: test/channel_test.dart: channel list [E]                                                                                                                                             
    Exception: List channels failed: 128
    package:flutter_tools/src/base/common.dart 24:3               throwToolExit
    package:flutter_tools/src/commands/channel.dart 78:7          ChannelCommand._listChannels
    ===== asynchronous gap ===========================
    dart:async                                                    _AsyncAwaitCompleter.completeError
    package:flutter_tools/src/commands/channel.dart               ChannelCommand._listChannels
    ===== asynchronous gap ===========================
    dart:async                                                    _asyncThenWrapperHelper
    package:flutter_tools/src/commands/channel.dart               ChannelCommand._listChannels
    package:flutter_tools/src/commands/channel.dart 38:15         ChannelCommand.runCommand
    ===== asynchronous gap ===========================
    dart:async                                                    _asyncThenWrapperHelper
    package:flutter_tools/src/runner/flutter_command.dart         FlutterCommand.verifyThenRunCommand
    package:flutter_tools/src/runner/flutter_command.dart 486:33  FlutterCommand.run.<fn>
    ===== asynchronous gap ===========================
    dart:async                                                    _asyncThenWrapperHelper
    package:flutter_tools/src/runner/flutter_command.dart         FlutterCommand.run.<fn>
    package:flutter_tools/src/base/context.dart 142:29            AppContext.run.<fn>
    ===== asynchronous gap ===========================
    dart:async                                                    _asyncThenWrapperHelper
    package:flutter_tools/src/runner/flutter_command_runner.dart  FlutterCommandRunner.runCommand.<fn>
    package:flutter_tools/src/base/context.dart 142:29            AppContext.run.<fn>

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes tests for all changed/updated/fixed behaviors (See Test Coverage).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

@alexmarkov alexmarkov marked this pull request as ready for review February 28, 2019 21:23
dnfield
dnfield reviewed Mar 1, 2019
View reviewed changes
packages/flutter_tools/lib/src/commands/channel.dart Outdated
if (result != 0)
throwToolExit('List channels failed: $result', exitCode: result);
if (result != 0) {
throwToolExit('List channels failed: $result\n${rawOutput.join('\n')}',
Copy link
Contributor

@dnfield dnfield Mar 1, 2019

Choose a reason for hiding this comment

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

Could we guard this behind a verbose flag? I suspect this could be confusing for some users, and I assume CI will be running verbosely.

Copy link
Contributor Author

@alexmarkov alexmarkov Mar 1, 2019

Choose a reason for hiding this comment

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

Fixed, could you take a look once again?

I originally did it without checking verbose flag as git output is only printed if there is an error, and I think it is more confusing for user to see "List channels failed: 128" rather than the actual error message from git.

@dnfield dnfield added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Mar 1, 2019
@dnfield
Copy link
Contributor

dnfield commented Mar 1, 2019

This LGTM - @jonahwilliams what do you think?

jonahwilliams
jonahwilliams approved these changes Mar 1, 2019
View reviewed changes
Copy link
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.

LGTM

@alexmarkov alexmarkov merged commit 604e9dc into flutter:master Mar 1, 2019
@alexmarkov alexmarkov deleted the git-output-from-channel branch March 1, 2019 18:23
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dnfield dnfield dnfield left review comments

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexmarkov @dnfield @jonahwilliams @googlebot