Skip Android health-checks when the "Android Developer" option is disabled in Flipper settings

Summary: Skip Android health-checks when the "Android Developer" option is disabled in Flipper settings. Also made some refactoring to use immer for healthcheck reducer. Reviewed By: mweststrate Differential Revision: D19088322 fbshipit-source-id: 801d874b6e7e5af80802b4bf4313d98f1cee13f6
2019-12-16 16:35:49 -08:00
parent be53990613
commit d32774f439
6 changed files with 256 additions and 188 deletions
--- a/src/chrome/DoctorBar.tsx
+++ b/src/chrome/DoctorBar.tsx
@@ -18,29 +18,23 @@ import {
 import {State as Store} from '../reducers/index';
 import {ButtonGroup, Button} from 'flipper';
 import {FlexColumn, FlexRow} from 'flipper';
-import runHealthchecks from '../utils/runHealthchecks';
+import runHealthchecks, {
+  HealthcheckSettings,
+  HealthcheckEventsHandler,
+} from '../utils/runHealthchecks';
 import {
  initHealthcheckReport,
-  updateHealthcheckReportItem,
+  updateHealthcheckReportItemStatus,
+  updateHealthcheckReportCategoryStatus,
  startHealthchecks,
  finishHealthchecks,
-  HealthcheckReport,
-  HealthcheckReportItem,
 } from '../reducers/healthchecks';

-type StateFromProps = {};
+type StateFromProps = HealthcheckSettings;

 type DispatchFromProps = {
  setActiveSheet: (payload: ActiveSheet) => void;
-  initHealthcheckReport: (report: HealthcheckReport) => void;
-  updateHealthcheckReportItem: (
-    categoryIdx: number,
-    itemIdx: number,
-    item: HealthcheckReportItem,
-  ) => void;
-  startHealthchecks: () => void;
-  finishHealthchecks: () => void;
-};
+} & HealthcheckEventsHandler;

 type State = {
  visible: boolean;
@@ -58,12 +52,7 @@ class DoctorBar extends Component<Props, State> {
    this.showMessageIfChecksFailed();
  }
  async showMessageIfChecksFailed() {
-    const result = await runHealthchecks({
-      initHealthcheckReport: this.props.initHealthcheckReport,
-      updateHealthcheckReportItem: this.props.updateHealthcheckReportItem,
-      startHealthchecks: this.props.startHealthchecks,
-      finishHealthchecks: this.props.finishHealthchecks,
-    });
+    const result = await runHealthchecks(this.props);
    if (!result) {
      this.setVisible(true);
    }
@@ -106,13 +95,19 @@ class DoctorBar extends Component<Props, State> {
  }
 }

-export default connect<StateFromProps, DispatchFromProps, {}, Store>(null, {
-  setActiveSheet,
-  initHealthcheckReport,
-  updateHealthcheckReportItem,
-  startHealthchecks,
-  finishHealthchecks,
-})(DoctorBar);
+export default connect<StateFromProps, DispatchFromProps, {}, Store>(
+  ({settingsState}) => ({
+    enableAndroid: settingsState.enableAndroid,
+  }),
+  {
+    setActiveSheet,
+    initHealthcheckReport,
+    updateHealthcheckReportItemStatus,
+    updateHealthcheckReportCategoryStatus,
+    startHealthchecks,
+    finishHealthchecks,
+  },
+)(DoctorBar);

 const Container = styled.div({
  boxShadow: '2px 2px 2px #ccc',
--- a/src/chrome/DoctorSheet.tsx
+++ b/src/chrome/DoctorSheet.tsx
@@ -28,27 +28,22 @@ import {
  HealthcheckReportItem,
  HealthcheckReport,
  initHealthcheckReport,
-  updateHealthcheckReportItem,
+  updateHealthcheckReportItemStatus,
+  updateHealthcheckReportCategoryStatus,
  startHealthchecks,
  finishHealthchecks,
 } from '../reducers/healthchecks';
-import runHealthchecks from '../utils/runHealthchecks';
+import runHealthchecks, {
+  HealthcheckSettings,
+  HealthcheckEventsHandler,
+} from '../utils/runHealthchecks';
 import {shell} from 'electron';

 type StateFromProps = {
  report: HealthcheckReport;
-};
+} & HealthcheckSettings;

-type DispatchFromProps = {
-  initHealthcheckReport: (report: HealthcheckReport) => void;
-  updateHealthcheckReportItem: (
-    categoryIdx: number,
-    itemIdx: number,
-    item: HealthcheckReportItem,
-  ) => void;
-  startHealthchecks: () => void;
-  finishHealthchecks: () => void;
-};
+type DispatchFromProps = HealthcheckEventsHandler;

 const Container = styled(FlexColumn)({
  padding: 20,
@@ -87,7 +82,7 @@ const SideContainer = styled(FlexBox)({

 const SideContainerText = styled(Text)({
  display: 'block',
-  'word-wrap': 'break-word',
+  wordWrap: 'break-word',
 });

 const HealthcheckLabel = styled(Text)({
@@ -102,6 +97,15 @@ function HealthcheckIcon(props: {check: HealthcheckResult}) {
  switch (props.check.status) {
    case 'IN_PROGRESS':
      return <LoadingIndicator size={16} title={props.check.message} />;
+    case 'SKIPPED':
+      return (
+        <Glyph
+          size={16}
+          name={'question'}
+          color={colors.grey}
+          title={props.check.message}
+        />
+      );
    case 'SUCCESS':
      return (
        <Glyph
@@ -196,42 +200,17 @@ class DoctorSheet extends Component<Props, State> {
        ...prevState,
      };
    });
-    await runHealthchecks({
-      initHealthcheckReport: this.props.initHealthcheckReport,
-      updateHealthcheckReportItem: this.props.updateHealthcheckReportItem,
-      startHealthchecks: this.props.startHealthchecks,
-      finishHealthchecks: this.props.finishHealthchecks,
-    });
+    await runHealthchecks(this.props);
  }

  hasProblems() {
    return this.props.report.categories.some(cat =>
-      cat.checks.some(chk => chk.status != 'SUCCESS'),
+      cat.checks.some(
+        chk => chk.status === 'FAILED' || chk.status === 'WARNING',
+      ),
    );
  }

-  getHealthcheckCategoryReportItem(
-    state: HealthcheckReportCategory,
-  ): HealthcheckReportItem {
-    return {
-      label: state.label,
-      ...(state.checks.some(c => c.status === 'IN_PROGRESS')
-        ? {status: 'IN_PROGRESS'}
-        : state.checks.every(c => c.status === 'SUCCESS')
-        ? {status: 'SUCCESS'}
-        : state.checks.some(c => c.status === 'FAILED')
-        ? {
-            status: 'FAILED',
-            message: 'Doctor discovered problems with the current installation',
-          }
-        : {
-            status: 'WARNING',
-            message:
-              'Doctor discovered non-blocking problems with the current installation',
-          }),
-    };
-  }
-
  render() {
    return (
      <Container>
@@ -242,10 +221,7 @@ class DoctorSheet extends Component<Props, State> {
              (category, categoryIdx) => {
                return (
                  <CategoryContainer key={categoryIdx}>
-                    <HealthcheckDisplay
-                      check={this.getHealthcheckCategoryReportItem(category)}
-                      category={category}
-                    />
+                    <HealthcheckDisplay check={category} category={category} />
                    <CategoryContainer>
                      {category.checks.map((check, checkIdx) => (
                        <HealthcheckDisplay
@@ -295,12 +271,14 @@ class DoctorSheet extends Component<Props, State> {
 }

 export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
-  ({healthchecks: {healthcheckReport}}) => ({
+  ({healthchecks: {healthcheckReport}, settingsState}) => ({
    report: healthcheckReport,
+    enableAndroid: settingsState.enableAndroid,
  }),
  {
    initHealthcheckReport,
-    updateHealthcheckReportItem,
+    updateHealthcheckReportItemStatus,
+    updateHealthcheckReportCategoryStatus,
    startHealthchecks,
    finishHealthchecks,
  },
--- a/src/reducers/tests/healthchecks.node.tsx
+++ b/src/reducers/tests/healthchecks.node.tsx
@@ -14,7 +14,8 @@ import {
  HealthcheckReportCategory,
  HealthcheckReportItem,
  finishHealthchecks,
-  updateHealthcheckReportItem,
+  updateHealthcheckReportItemStatus,
+  updateHealthcheckReportCategoryStatus,
 } from '../healthchecks';

 const HEALTHCHECK_ITEM: HealthcheckReportItem = {
@@ -70,14 +71,14 @@ test('updateHealthcheck', () => {
  let res = reducer(undefined, initHealthcheckReport(report));
  res = reducer(
    res,
-    updateHealthcheckReportItem(0, 0, {
-      label: 'Updated Test Item',
+    updateHealthcheckReportItemStatus(0, 0, {
+      message: 'Updated Test Message',
      status: 'SUCCESS',
    }),
  );
  expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy();
-  expect(res.healthcheckReport.categories[0].checks[0].label).toEqual(
-    'Updated Test Item',
+  expect(res.healthcheckReport.categories[0].checks[0].message).toEqual(
+    'Updated Test Message',
  );
  expect(res.healthcheckReport.categories[0].checks[0].status).toEqual(
    'SUCCESS',
@@ -89,3 +90,24 @@ test('updateHealthcheck', () => {
    'WARNING',
  );
 });
+
+test('updateHealthcheckCategoryStatus', () => {
+  const report = {
+    isHealthcheckInProgress: true,
+    categories: [HEALTHCHECK_CATEGORY, HEALTHCHECK_CATEGORY],
+  };
+  let res = reducer(undefined, initHealthcheckReport(report));
+  res = reducer(
+    res,
+    updateHealthcheckReportCategoryStatus(1, {
+      status: 'FAILED',
+      message: 'Error message',
+    }),
+  );
+  expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy();
+  expect(res.healthcheckReport.categories[0].label).toEqual('Test Category');
+  expect(res.healthcheckReport.categories[0].status).toEqual('WARNING');
+  expect(res.healthcheckReport.categories[1].label).toEqual('Test Category');
+  expect(res.healthcheckReport.categories[1].status).toEqual('FAILED');
+  expect(res.healthcheckReport.categories[1].message).toEqual('Error message');
+});
--- a/src/reducers/healthchecks.tsx
+++ b/src/reducers/healthchecks.tsx
@@ -8,6 +8,7 @@
 */

 import {Actions} from './';
+import {produce} from 'flipper';

 export type State = {
  healthcheckReport: HealthcheckReport;
@@ -19,11 +20,18 @@ export type Action =
      payload: HealthcheckReport;
    }
  | {
-      type: 'UPDATE_HEALTHCHECK_REPORT_ITEM';
+      type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS';
      payload: {
        categoryIdx: number;
        itemIdx: number;
-        item: HealthcheckReportItem;
+        status: HealthcheckResult;
+      };
+    }
+  | {
+      type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS';
+      payload: {
+        categoryIdx: number;
+        status: HealthcheckResult;
      };
    }
  | {
@@ -44,6 +52,7 @@ export type HealthcheckStatus =
  | 'IN_PROGRESS'
  | 'SUCCESS'
  | 'FAILED'
+  | 'SKIPPED'
  | 'WARNING';

 export type HealthcheckResult = {
@@ -58,73 +67,64 @@ export type HealthcheckReportItem = {

 export type HealthcheckReportCategory = {
  label: string;
-  status: HealthcheckStatus;
  checks: Array<HealthcheckReportItem>;
-};
+} & HealthcheckResult;

 export type HealthcheckReport = {
  isHealthcheckInProgress: boolean;
  categories: Array<HealthcheckReportCategory>;
 };

+const updateReportItem = produce(
+  (
+    draft: State,
+    payload: {
+      categoryIdx: number;
+      itemIdx: number;
+      status: HealthcheckResult;
+    },
+  ) => {
+    Object.assign(
+      draft.healthcheckReport.categories[payload.categoryIdx].checks[
+        payload.itemIdx
+      ],
+      payload.status,
+    );
+  },
+);
+
+const updateCategoryStatus = produce(
+  (draft: State, payload: {categoryIdx: number; status: HealthcheckResult}) => {
+    Object.assign(
+      draft.healthcheckReport.categories[payload.categoryIdx],
+      payload.status,
+    );
+  },
+);
+
+const initReport = produce((draft: State, report: HealthcheckReport) => {
+  draft.healthcheckReport = report;
+});
+
+const setIsInProgress = produce((draft: State, isInProgress: boolean) => {
+  draft.healthcheckReport.isHealthcheckInProgress = isInProgress;
+});
+
 export default function reducer(
-  state: State | undefined = INITIAL_STATE,
+  draft: State | undefined = INITIAL_STATE,
  action: Actions,
 ): State {
-  if (action.type === 'INIT_HEALTHCHECK_REPORT') {
-    return {
-      ...state,
-      healthcheckReport: action.payload,
-    };
-  } else if (action.type === 'START_HEALTHCHECKS') {
-    return {
-      ...state,
-      healthcheckReport: {
-        ...state.healthcheckReport,
-        isHealthcheckInProgress: true,
-      },
-    };
-  } else if (action.type === 'FINISH_HEALTHCHECKS') {
-    return {
-      ...state,
-      healthcheckReport: {
-        ...state.healthcheckReport,
-        isHealthcheckInProgress: false,
-      },
-    };
-  } else if (action.type === 'UPDATE_HEALTHCHECK_REPORT_ITEM') {
-    return {
-      ...state,
-      healthcheckReport: {
-        ...state.healthcheckReport,
-        categories: [
-          ...state.healthcheckReport.categories.slice(
-            0,
-            action.payload.categoryIdx,
-          ),
-          {
-            ...state.healthcheckReport.categories[action.payload.categoryIdx],
-            checks: [
-              ...state.healthcheckReport.categories[
-                action.payload.categoryIdx
-              ].checks.slice(0, action.payload.itemIdx),
-              {
-                ...action.payload.item,
-              },
-              ...state.healthcheckReport.categories[
-                action.payload.categoryIdx
-              ].checks.slice(action.payload.itemIdx + 1),
-            ],
-          },
-          ...state.healthcheckReport.categories.slice(
-            action.payload.categoryIdx + 1,
-          ),
-        ],
-      },
-    };
-  } else {
-    return state;
-  }
+  return action.type === 'INIT_HEALTHCHECK_REPORT'
+    ? initReport(draft, action.payload)
+    : action.type === 'START_HEALTHCHECKS'
+    ? setIsInProgress(draft, true)
+    : action.type === 'FINISH_HEALTHCHECKS'
+    ? setIsInProgress(draft, false)
+    : action.type === 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS'
+    ? updateReportItem(draft, action.payload)
+    : action.type === 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS'
+    ? updateCategoryStatus(draft, action.payload)
+    : draft;
 }

 export const initHealthcheckReport = (report: HealthcheckReport): Action => ({
@@ -132,16 +132,27 @@ export const initHealthcheckReport = (report: HealthcheckReport): Action => ({
  payload: report,
 });

-export const updateHealthcheckReportItem = (
+export const updateHealthcheckReportItemStatus = (
  categoryIdx: number,
  itemIdx: number,
-  item: HealthcheckReportItem,
+  status: HealthcheckResult,
 ): Action => ({
-  type: 'UPDATE_HEALTHCHECK_REPORT_ITEM',
+  type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS',
  payload: {
    categoryIdx,
    itemIdx,
-    item,
+    status,
+  },
+});
+
+export const updateHealthcheckReportCategoryStatus = (
+  categoryIdx: number,
+  status: HealthcheckResult,
+): Action => ({
+  type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS',
+  payload: {
+    categoryIdx,
+    status,
  },
 });

--- a/src/utils/icons.js
+++ b/src/utils/icons.js
@@ -86,6 +86,7 @@ const ICONS = {
  'life-event-major': [16],
  target: [12, 16],
  tools: [12, 20],
+  question: [16],
  underline: [12],
  'washing-machine': [12],
  'watch-tv': [12],
--- a/src/utils/runHealthchecks.tsx
+++ b/src/utils/runHealthchecks.tsx
@@ -10,7 +10,6 @@
 import {
  HealthcheckResult,
  HealthcheckReport,
-  HealthcheckReportItem,
  HealthcheckReportCategory,
 } from '../reducers/healthchecks';
 import {getHealthchecks, getEnvInfo} from 'flipper-doctor';
@@ -20,37 +19,69 @@ let runningHealthcheck: Promise<boolean>;

 export type HealthcheckEventsHandler = {
  initHealthcheckReport: (report: HealthcheckReport) => void;
-  updateHealthcheckReportItem: (
+  updateHealthcheckReportItemStatus: (
    categoryIdx: number,
    itemIdx: number,
-    item: HealthcheckReportItem,
+    status: HealthcheckResult,
+  ) => void;
+  updateHealthcheckReportCategoryStatus: (
+    categoryIdx: number,
+    status: HealthcheckResult,
  ) => void;
  startHealthchecks: () => void;
  finishHealthchecks: () => void;
 };

+export type HealthcheckSettings = {
+  enableAndroid: boolean;
+};
+
+export type HealthcheckOptions = HealthcheckEventsHandler & HealthcheckSettings;
+
 async function launchHealthchecks(
-  dispatch: HealthcheckEventsHandler,
+  options: HealthcheckOptions,
 ): Promise<boolean> {
-  let hasProblems: boolean = true;
-  dispatch.startHealthchecks();
+  let healthchecksResult: boolean = true;
+  options.startHealthchecks();
  try {
-    const initialState: HealthcheckResult = {
+    const inProgressResult: HealthcheckResult = {
      status: 'IN_PROGRESS',
      message: 'The healthcheck is in progress',
    };
+    const androidSkippedResult: HealthcheckResult = {
+      status: 'SKIPPED',
+      message:
+        'The healthcheck was skipped because Android development is disabled in the Flipper settings',
+    };
+    const failedResult: HealthcheckResult = {
+      status: 'FAILED',
+      message: 'The healthcheck failed',
+    };
+    const warningResult: HealthcheckResult = {
+      status: 'WARNING',
+      message: 'The optional healthcheck failed',
+    };
+    const succeededResult: HealthcheckResult = {
+      status: 'SUCCESS',
+      message: undefined,
+    };
+    const healthchecks = getHealthchecks();
    const hcState: HealthcheckReport = {
      isHealthcheckInProgress: true,
-      categories: Object.values(getHealthchecks())
-        .map(category => {
+      categories: Object.entries(healthchecks)
+        .map(([categoryKey, category]) => {
          if (!category) {
            return null;
          }
+          const state: HealthcheckResult =
+            categoryKey === 'android' && !options.enableAndroid
+              ? androidSkippedResult
+              : inProgressResult;
          return {
-            ...initialState,
+            ...state,
            label: category.label,
            checks: category.healthchecks.map(x => ({
-              ...initialState,
+              ...state,
              label: x.label,
            })),
          };
@@ -58,54 +89,84 @@ async function launchHealthchecks(
        .filter(x => !!x)
        .map(x => x as HealthcheckReportCategory),
    };
-    dispatch.initHealthcheckReport(hcState);
+    options.initHealthcheckReport(hcState);
    const environmentInfo = await getEnvInfo();
-    const categories = Object.values(getHealthchecks());
-    for (let cIdx = 0; cIdx < categories.length; cIdx++) {
-      const c = categories[cIdx];
-      if (!c) {
+    const categories = Object.entries(healthchecks);
+    for (const [categoryIdx, [categoryKey, category]] of categories.entries()) {
+      if (!category) {
        continue;
      }
-      for (let hIdx = 0; hIdx < c.healthchecks.length; hIdx++) {
-        const h = c.healthchecks[hIdx];
-        const result = await h.run(environmentInfo);
-        if (result.hasProblem) {
-          hasProblems = false;
+      const isSkippedAndroidCategory =
+        categoryKey === 'android' && !options.enableAndroid;
+      const allResults: HealthcheckResult[] = [];
+      for (
+        let healthcheckIdx = 0;
+        healthcheckIdx < category.healthchecks.length;
+        healthcheckIdx++
+      ) {
+        const h = category.healthchecks[healthcheckIdx];
+        if (isSkippedAndroidCategory) {
+          options.updateHealthcheckReportItemStatus(
+            categoryIdx,
+            healthcheckIdx,
+            androidSkippedResult,
+          );
+          allResults.push(androidSkippedResult);
+        } else {
+          const result = await h.run(environmentInfo);
+          if (result.hasProblem && h.isRequired) {
+            healthchecksResult = false;
+          }
+          const status: HealthcheckResult =
+            result.hasProblem && h.isRequired
+              ? {
+                  ...failedResult,
+                  helpUrl: result.helpUrl,
+                }
+              : result.hasProblem && !h.isRequired
+              ? {
+                  ...warningResult,
+                  helpUrl: result.helpUrl,
+                }
+              : succeededResult;
+          options.updateHealthcheckReportItemStatus(
+            categoryIdx,
+            healthcheckIdx,
+            status,
+          );
+          allResults.push(status);
        }
-        dispatch.updateHealthcheckReportItem(cIdx, hIdx, {
-          ...h,
-          ...(result.hasProblem && h.isRequired
-            ? {
-                status: 'FAILED',
-                message: 'The healthcheck failed',
-                helpUrl: result.helpUrl,
-              }
-            : result.hasProblem && !h.isRequired
-            ? {
-                status: 'WARNING',
-                message: 'Doctor discovered a problem during the healthcheck',
-                helpUrl: result.helpUrl,
-              }
-            : {
-                status: 'SUCCESS',
-                message: 'The healthcheck completed succesfully',
-              }),
-        });
      }
+      const categoryStatus = {
+        label: category.label,
+        ...(allResults.some(c => c.status === 'IN_PROGRESS')
+          ? inProgressResult
+          : allResults.every(c => c.status === 'SUCCESS')
+          ? succeededResult
+          : allResults.every(c => c.status === 'SKIPPED')
+          ? androidSkippedResult
+          : allResults.some(c => c.status === 'FAILED')
+          ? failedResult
+          : warningResult),
+      };
+      options.updateHealthcheckReportCategoryStatus(
+        categoryIdx,
+        categoryStatus,
+      );
    }
  } catch {
  } finally {
-    dispatch.finishHealthchecks();
+    options.finishHealthchecks();
  }
-  return hasProblems;
+  return healthchecksResult;
 }

 export default async function runHealthchecks(
-  dispatch: HealthcheckEventsHandler,
+  options: HealthcheckOptions,
 ): Promise<boolean> {
  if (healthcheckIsRunning) {
    return runningHealthcheck;
  }
-  runningHealthcheck = launchHealthchecks(dispatch);
+  runningHealthcheck = launchHealthchecks(options);
  return runningHealthcheck;
 }