Fix crash on vswhere search from flutter doctor by franciscojma86 · Pull Request #40263 · flutter/flutter

franciscojma86 · 2019-09-11T17:29:35Z

Description

Fixes a crash introduced on #40011 due to an incorrect type in the vswhere search

Related Issues

Tests

Relevant tests in visual_studio_test.dart were updated

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.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
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?

Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

jonahwilliams

LGTM

stuartmorgan-g · 2019-09-11T17:40:31Z

packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart

-      'productMilestoneIsPreRelease': true,
    }
  };



Please add a variant of this (_oldResponse?) that doesn't contain the reboot/complete/launchable entries, and at least one test using them; installationHasIssues still has asserts that the keys are present; we should have a test coverage that would catch that. (And also, please fix that 😀)

stuartmorgan-g

LGTM with the change above

packages/flutter_tools/lib/src/windows/visual_studio.dart

stuartmorgan-g · 2019-09-11T18:11:10Z

packages/flutter_tools/lib/src/windows/visual_studio.dart

-    assert(installationDetails[_isLaunchableKey] != null);
+    if (installationDetails[_isCompleteKey] == null ||
+        installationDetails[_isRebootRequiredKey] == null ||
+        installationDetails[_isLaunchableKey] == null) {


I'd rather we not treat these as a monolithic block, where we don't check them at all if any one is missing. Instead this can be:

if (installationDetails[_isCompleteKey] != null && !installationDetails[_isCompleteKey]) { return true; } [similarly for the others] return false;

codecov · 2019-09-11T18:32:38Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8eee93f). Click here to learn what that means.
The diff coverage is 87.5%.

@@            Coverage Diff            @@
##             master   #40263   +/-   ##
=========================================
  Coverage          ?   58.42%           
=========================================
  Files             ?      192           
  Lines             ?    18539           
  Branches          ?        0           
=========================================
  Hits              ?    10831           
  Misses            ?     7708           
  Partials          ?        0

Flag	Coverage Δ
#flutter_tool	`58.42% <87.5%> (?)`

Impacted Files	Coverage Δ
...s/flutter_tools/lib/src/windows/visual_studio.dart	`79.41% <87.5%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8eee93f...4d47813. Read the comment docs.

stuartmorgan-g · 2019-09-11T19:06:31Z

packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart

+  };
+
+  // A version of a response that doesn't include certain installation status
+  // information that might be missing in older Windows versions.


s/Windows/Visual Studio/

Done here and in other places

stuartmorgan-g

LGTM with changes noted above, with the understanding that the potential crashes will be fixed in the other PR ASAP.

Fixes a crash introduced on flutter#40011 due to an incorrect type in the vswhere search Fixes flutter#40238

Fix crash on vswhere search from flutter doctor

61aa777

franciscojma86 requested a review from stuartmorgan-g September 11, 2019 17:29

fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 11, 2019

googlebot added the cla: yes label Sep 11, 2019

franciscojma86 added c: crash Stack traces logged to the console platform-windows Building on or for Windows specifically and removed cla: yes labels Sep 11, 2019

googlebot added the cla: yes label Sep 11, 2019

franciscojma86 added the t: flutter doctor Problem related to the "flutter doctor" tool label Sep 11, 2019

franciscojma86 requested a review from jonahwilliams September 11, 2019 17:35

jonahwilliams approved these changes Sep 11, 2019

View reviewed changes

stuartmorgan-g reviewed Sep 11, 2019

View reviewed changes

stuartmorgan-g approved these changes Sep 11, 2019

View reviewed changes

stuartmorgan-g requested changes Sep 11, 2019

View reviewed changes

packages/flutter_tools/lib/src/windows/visual_studio.dart Outdated Show resolved Hide resolved

Support old version

d434b1a

stuartmorgan-g reviewed Sep 11, 2019

View reviewed changes

Address comments

dfc1d83

stuartmorgan-g reviewed Sep 11, 2019

View reviewed changes

stuartmorgan-g approved these changes Sep 11, 2019

View reviewed changes

franciscojma86 added 3 commits September 11, 2019 12:07

Windows -> VS

ec1fbb5

Make complete and lauchablee false if null

b32cd7d

Whitespace

4d47813

stuartmorgan-g merged commit f5733f7 into flutter:master Sep 12, 2019

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

Fix crash on vswhere search from flutter doctor (flutter#40263)

830cb95

Fixes a crash introduced on flutter#40011 due to an incorrect type in the vswhere search Fixes flutter#40238

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix crash on vswhere search from flutter doctor#40263

Fix crash on vswhere search from flutter doctor#40263
stuartmorgan-g merged 6 commits intoflutter:masterfrom
franciscojma86:fix

franciscojma86 commented Sep 11, 2019

Uh oh!

jonahwilliams left a comment

Uh oh!

stuartmorgan-g Sep 11, 2019

Uh oh!

franciscojma86 Sep 11, 2019

Uh oh!

stuartmorgan-g left a comment

Uh oh!

Uh oh!

stuartmorgan-g Sep 11, 2019 •

edited

Loading

Uh oh!

franciscojma86 Sep 11, 2019

Uh oh!

codecov bot commented Sep 11, 2019 •

edited

Loading

Uh oh!

stuartmorgan-g Sep 11, 2019

Uh oh!

franciscojma86 Sep 11, 2019

Uh oh!

stuartmorgan-g left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

franciscojma86 commented Sep 11, 2019

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

franciscojma86 Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stuartmorgan-g Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franciscojma86 Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

stuartmorgan-g Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

franciscojma86 Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

stuartmorgan-g Sep 11, 2019 •

edited

Loading

codecov bot commented Sep 11, 2019 •

edited

Loading