surface subchecks for failed xcode-select command

Summary: The subchecks are displayed for failed command only. Rationale: it is tricky to help people when the command fails. Surfacing subchecks should make it easier for users as well as flipper oncall to see why a check failed and what subchecks have passed Reviewed By: LukeDefeo Differential Revision: D59222954 fbshipit-source-id: dfb77b4b2d0b6facca609d6e4e9bbe59d67d9a77
facebook · Jul 1, 2024 · 8c1cbe1 · 8c1cbe1
1 parent a17d3d6
commit 8c1cbe1
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 7 deletions.
diff --git a/desktop/flipper-common/src/doctor.tsx b/desktop/flipper-common/src/doctor.tsx
@@ -69,8 +69,15 @@ export namespace FlipperDoctor {
     command: string;
   };
 
+  export type HealthcheckRunSubcheck = {
+    status: 'ok' | 'fail';
+    title: string;
+  };
+
   export type HealthcheckRunResult = {
     hasProblem: boolean;
+    /** Indicates what sub checks were passed to better communicate the problem */
+    subchecks?: HealthcheckRunSubcheck[];
     message: MessageIdWithParams;
   };
 
@@ -109,6 +116,7 @@ export namespace FlipperDoctor {
     status: HealthcheckStatus;
     isAcknowledged?: boolean;
     message?: MessageIdWithParams;
+    subchecks?: HealthcheckRunSubcheck[];
   };
 
   export type HealthcheckReportItem = {

diff --git a/desktop/flipper-server/src/doctor/fb-stubs/validateSelectedXcodeVersion.tsx b/desktop/flipper-server/src/doctor/fb-stubs/validateSelectedXcodeVersion.tsx
@@ -12,6 +12,7 @@ import {FlipperDoctor} from 'flipper-common';
 export async function validateSelectedXcodeVersion(
   _selectedPath: string,
   _availableXcode: string | null,
+  _subchecks: FlipperDoctor.HealthcheckRunSubcheck[],
 ): Promise<FlipperDoctor.HealthcheckRunResult> {
   return {
     hasProblem: false,

diff --git a/desktop/flipper-server/src/doctor/index.tsx b/desktop/flipper-server/src/doctor/index.tsx
@@ -261,6 +261,7 @@ export function getHealthchecks(
                 run: async (
                   _: FlipperDoctor.EnvironmentInfo,
                 ): Promise<FlipperDoctor.HealthcheckRunResult> => {
+                  const subchecks: FlipperDoctor.HealthcheckRunSubcheck[] = [];
                   const allApps =
                     await fs_extra.promises.readdir('/Applications');
                   // Xcode_14.2.0_xxxxxxx.app
@@ -274,11 +275,20 @@ export function getHealthchecks(
                   const availableXcode = latestXCode
                     ? path.join('/Applications', latestXCode)
                     : null;
+                  subchecks.push({
+                    status: availableXcode ? 'ok' : 'fail',
+                    title: 'Xcode in /Applications',
+                  });
 
                   const result = await tryExecuteCommand('xcode-select -p');
+                  subchecks.push({
+                    status: result.fail ? 'fail' : 'ok',
+                    title: 'xcode-select runs successfully',
+                  });
                   if (result.fail) {
                     return {
                       hasProblem: true,
+                      subchecks,
                       message: [
                         'ios.xcode-select--not_set',
                         {message: result.message, availableXcode},
@@ -287,18 +297,34 @@ export function getHealthchecks(
                   }
 
                   const selectedXcode = result.stdout.toString().trim();
-                  if (selectedXcode == '/Library/Developer/CommandLineTools') {
+                  const isSelectedXcodeCommandLineTools =
+                    selectedXcode == '/Library/Developer/CommandLineTools';
+                  subchecks.push({
+                    status: isSelectedXcodeCommandLineTools ? 'fail' : 'ok',
+                    title:
+                      'xcode-select does NOT point to "/Library/Developer/CommandLineTools"',
+                  });
+                  if (isSelectedXcodeCommandLineTools) {
                     return {
                       hasProblem: true,
+                      subchecks,
                       message: [
                         'ios.xcode-select--no_xcode_selected',
                         {availableXcode},
                       ],
                     };
                   }
-                  if ((await fs_extra.pathExists(selectedXcode)) == false) {
+
+                  const selectedXcodeExists =
+                    await fs_extra.pathExists(selectedXcode);
+                  subchecks.push({
+                    status: selectedXcodeExists ? 'ok' : 'fail',
+                    title: 'Selected Xcode exists',
+                  });
+                  if (!selectedXcodeExists) {
                     return {
                       hasProblem: true,
+                      subchecks,
                       message: [
                         'ios.xcode-select--nonexisting_selected',
                         {selected: selectedXcode, availableXcode},
@@ -309,9 +335,13 @@ export function getHealthchecks(
                     await validateSelectedXcodeVersion(
                       selectedXcode,
                       availableXcode,
+                      subchecks,
                     );
                   if (validatedXcodeVersion.hasProblem) {
-                    return validatedXcodeVersion;
+                    return {
+                      ...validatedXcodeVersion,
+                      subchecks,
+                    };
                   }
                   return {
                     hasProblem: false,

diff --git a/desktop/flipper-server/src/utils/runHealthchecks.tsx b/desktop/flipper-server/src/utils/runHealthchecks.tsx
@@ -68,6 +68,7 @@ export async function runHealthcheck(
   return checkResult.hasProblem && check.isRequired
     ? {
         status: 'FAILED',
+        subchecks: checkResult.subchecks,
         message: checkResult.message,
       }
     : checkResult.hasProblem && !check.isRequired

diff --git a/desktop/flipper-ui/src/sandy-chrome/SetupDoctorScreen.tsx b/desktop/flipper-ui/src/sandy-chrome/SetupDoctorScreen.tsx
@@ -154,10 +154,28 @@ function CollapsableCategory(props: {
           header={check.label}
           extra={<CheckIcon status={check.result.status} />}>
           {check.result.message != null ? (
-            <DoctorMessage
-              id={check.result.message[0]}
-              props={check.result.message[1]}
-            />
+            <>
+              {check.result.subchecks ? (
+                <ul>
+                  {check.result.subchecks.map((subcheck, i) => (
+                    <li key={i}>
+                      {subcheck.status === 'ok' ? (
+                        <CheckCircleFilled
+                          style={{color: theme.successColor}}
+                        />
+                      ) : (
+                        <CloseCircleFilled style={{color: theme.errorColor}} />
+                      )}{' '}
+                      {subcheck.title}
+                    </li>
+                  ))}
+                </ul>
+              ) : null}
+              <DoctorMessage
+                id={check.result.message[0]}
+                props={check.result.message[1]}
+              />
+            </>
           ) : null}
         </Collapse.Panel>
       ))}