Doctor complains Android SDK is not installed

Summary:
There are complaints about Android SDK being reported as "not installed" when it is actually installed. To address them, I changed the way how we detect SDK and also added some minimal actionable feedback for each check.

The problem with the previous implementation of Android SDK check via "envinfo" is that the library uses "sdkmanager" tool under the hood, and this tool doesn't work on Java 9+. To fix this I'm changing the way how we assume SDK is installed to simple check for "adb" tool existence.

Actionable feedback is shown on Doctor report when you click to an item.

Reviewed By: jknoxville

Differential Revision: D19517769

fbshipit-source-id: 1c21f1bdcd05c7c0ae3f97b9c3454efa2c861d26

src/chrome/DoctorSheet.tsx

@@ -88,6 +88,7 @@ const SideContainer = styled(FlexBox)({
 const SideContainerText = styled(Text)({
   display: 'block',
   wordWrap: 'break-word',
+  overflow: 'auto',
 });
 
 const HealthcheckLabel = styled(Text)({
@@ -170,6 +171,7 @@ function HealthcheckIcon(props: {checkResult: HealthcheckResult}) {
 function HealthcheckDisplay(props: {
   label: string;
   result: HealthcheckResult;
+  selected?: boolean;
   onClick?: () => void;
 }) {
   return (
@@ -177,6 +179,7 @@ function HealthcheckDisplay(props: {
       <HealthcheckDisplayContainer shrink title={props.result.message}>
         <HealthcheckIcon checkResult={props.result} />
         <HealthcheckLabel
+          bold={props.selected}
           underline={!!props.onClick}
           cursor={props.onClick && 'pointer'}
           onClick={props.onClick}>
@@ -187,27 +190,25 @@ function HealthcheckDisplay(props: {
   );
 }
 
-function SideMessageDisplay(props: {
-  isHealthcheckInProgress: boolean;
-  hasProblems: boolean;
-}) {
-  if (props.isHealthcheckInProgress) {
+function SideMessageDisplay(props: {children: React.ReactNode}) {
+  return <SideContainerText selectable>{props.children}</SideContainerText>;
+}
+
+function ResultMessage(props: {result: HealthcheckResult}) {
+  if (status === 'IN_PROGRESS') {
+    return <p>Doctor is running healthchecks...</p>;
+  } else if (hasProblems(props.result)) {
     return (
-      <SideContainerText selectable>
-        Doctor is running healthchecks...
-      </SideContainerText>
-    );
-  } else if (props.hasProblems) {
-    return (
-      <SideContainerText selectable>
-        Doctor has discovered problems with your installation.
-      </SideContainerText>
+      <p>
+        Doctor has discovered problems with your installation. Please click to
+        each item to get details.
+      </p>
     );
   } else {
     return (
-      <SideContainerText selectable>
+      <p>
         All good! Doctor has not discovered any issues with your installation.
-      </SideContainerText>
+      </p>
     );
   }
 }
@@ -224,6 +225,7 @@ function hasNewProblems(result: HealthcheckResult) {
 export type State = {
   acknowledgeCheckboxVisible: boolean;
   acknowledgeOnClose?: boolean;
+  selectedCheckKey?: string;
 };
 
 type Props = OwnProps & StateFromProps & DispatchFromProps;
@@ -296,10 +298,20 @@ class DoctorSheet extends Component<Props, State> {
     helpUrl && shell.openExternal(helpUrl);
   }
 
-  async runHealthchecks() {
+  async runHealthchecks(): Promise<void> {
     await runHealthchecks(this.props);
   }
 
+  getCheckMessage(checkKey: string): string {
+    for (const cat of Object.values(this.props.healthcheckReport.categories)) {
+      const check = Object.values(cat.checks).find(chk => chk.key === checkKey);
+      if (check) {
+        return check.result.message || '';
+      }
+    }
+    return '';
+  }
+
   render() {
     return (
       <Container>
@@ -319,12 +331,17 @@ class DoctorSheet extends Component<Props, State> {
                         {Object.values(category.checks).map(check => (
                           <HealthcheckDisplay
                             key={check.key}
+                            selected={check.key === this.state.selectedCheckKey}
                             label={check.label}
                             result={check.result}
-                            onClick={
-                              check.result.helpUrl
-                                ? () => this.openHelpUrl(check.result.helpUrl)
-                                : undefined
+                            onClick={() =>
+                              this.setState({
+                                ...this.state,
+                                selectedCheckKey:
+                                  this.state.selectedCheckKey === check.key
+                                    ? undefined
+                                    : check.key,
+                              })
                             }
                           />
                         ))}
@@ -344,12 +361,16 @@ class DoctorSheet extends Component<Props, State> {
           </HealthcheckListContainer>
           <Spacer />
           <SideContainer shrink>
-            <SideMessageDisplay
-              isHealthcheckInProgress={
-                this.props.healthcheckReport.result.status === 'IN_PROGRESS'
-              }
-              hasProblems={hasProblems(this.props.healthcheckReport.result)}
-            />
+            <SideMessageDisplay>
+              <SideContainerText selectable>
+                {this.state.selectedCheckKey && (
+                  <p>{this.getCheckMessage(this.state.selectedCheckKey)}</p>
+                )}
+                {!this.state.selectedCheckKey && (
+                  <ResultMessage result={this.props.healthcheckReport.result} />
+                )}
+              </SideContainerText>
+            </SideMessageDisplay>
           </SideContainer>
         </FlexRow>
         <FlexRow>

src/reducers/__tests__/healthchecks.node.tsx

@@ -14,8 +14,7 @@ import {
   updateHealthcheckResult,
   acknowledgeProblems,
 } from '../healthchecks';
-import {Healthchecks} from 'flipper-doctor';
-import {EnvironmentInfo} from 'flipper-doctor/lib/environmentInfo';
+import {Healthchecks, EnvironmentInfo} from 'flipper-doctor';
 
 const HEALTHCHECKS: Healthchecks = {
   ios: {
@@ -27,7 +26,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'ios.sdk',
         label: 'SDK Installed',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: false};
+          return {hasProblem: false, message: ''};
         },
       },
     ],
@@ -41,7 +40,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'android.sdk',
         label: 'SDK Installed',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: true};
+          return {hasProblem: true, message: 'Error'};
         },
       },
     ],
@@ -55,7 +54,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'common.openssl',
         label: 'OpenSSL Istalled',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: false};
+          return {hasProblem: false, message: ''};
         },
       },
     ],

src/reducers/healthchecks.tsx

@@ -60,7 +60,6 @@ export type HealthcheckResult = {
   status: HealthcheckStatus;
   isAcknowledged?: boolean;
   message?: string;
-  helpUrl?: string;
 };
 
 export type HealthcheckReportItem = {

src/utils/runHealthchecks.tsx

@@ -66,14 +66,14 @@ async function launchHealthchecks(options: HealthcheckOptions): Promise<void> {
         checkResult.hasProblem && h.isRequired
           ? {
               status: 'FAILED',
-              helpUrl: checkResult.helpUrl,
+              message: checkResult.message,
             }
           : checkResult.hasProblem && !h.isRequired
           ? {
               status: 'WARNING',
-              helpUrl: checkResult.helpUrl,
+              message: checkResult.message,
             }
-          : {status: 'SUCCESS'};
+          : {status: 'SUCCESS', message: checkResult.message};
       options.updateHealthcheckResult(categoryKey, h.key, result);
     }
   }