feat(appium): Set a proper exit code if any of required doctor checks…

… fails (#19617)

packages/appium/lib/cli/driver-command.js

@@ -67,10 +67,11 @@ export default class DriverCliCommand extends ExtensionCliCommand {
   }
 
   /**
-   * Runs doctor checks for the given driver
+   * Runs doctor checks for the given driver.
    *
    * @param {DriverDoctorOptions} opts
-   * @returns {Promise<import('@appium/types').IDoctorCheck[]>}
+   * @returns {Promise<number>} The amount of executed doctor checks.
+   * @throws {Error} If any of the mandatory Doctor checks fails.
    */
   async doctor({driver}) {
     return await super._doctor({

packages/appium/lib/cli/extension-command.js

@@ -16,7 +16,7 @@ import {packageDidChange} from '../extension/package-changed';
 import {spawn} from 'child_process';
 import {inspect} from 'node:util';
 import {pathToFileURL} from 'url';
-import {Doctor} from '../doctor/doctor';
+import {Doctor, EXIT_CODE as DOCTOR_EXIT_CODE} from '../doctor/doctor';
 import {npmPackage} from '../utils';
 import semver from 'semver';
 
@@ -742,10 +742,12 @@ class ExtensionCliCommand {
   }
 
   /**
-   * Runs doctor checks for the given extension
+   * Runs doctor checks for the given extension.
    *
    * @param {DoctorOptions} opts
-   * @returns {Promise<import('@appium/types').IDoctorCheck[]>}
+   * @returns {Promise<number>} The amount of Doctor checks that were
+   * successfully loaded and executed for the given extension
+   * @throws {Error} If any of the mandatory Doctor checks fails.
    */
   async _doctor({installSpec}) {
     if (!this.config.isInstalled(installSpec)) {
@@ -769,7 +771,7 @@ class ExtensionCliCommand {
     }
     if (!doctorSpec) {
       this.log.info(`The ${this.type} "${installSpec}" does not export any doctor checks`);
-      return [];
+      return 0;
     }
     if (!_.isPlainObject(doctorSpec) || !_.isArray(doctorSpec.checks)) {
       throw this._createFatalError(
@@ -810,14 +812,17 @@ class ExtensionCliCommand {
       .filter(isDoctorCheck);
     if (_.isEmpty(checks)) {
       this.log.info(`The ${this.type} "${installSpec}" exports no valid doctor checks`);
-      return [];
+      return 0;
     }
     this.log.debug(
       `Running ${util.pluralize('doctor check', checks.length, true)} ` +
       `for the "${installSpec}" ${this.type}`
     );
-    await new Doctor(checks).run();
-    return checks;
+    const exitCode = await new Doctor(checks).run();
+    if (exitCode !== DOCTOR_EXIT_CODE.SUCCESS) {
+      throw this._createFatalError('Treatment required');
+    }
+    return checks.length;
   }
 
   /**

packages/appium/lib/cli/plugin-command.js

@@ -69,7 +69,8 @@ export default class PluginCliCommand extends ExtensionCliCommand {
    * Runs doctor checks for the given plugin
    *
    * @param {PluginDoctorOptions} opts
-   * @returns {Promise<import('@appium/types').IDoctorCheck[]>}
+   * @returns {Promise<number>} The amount of executed doctor checks.
+   * @throws {Error} If any of the mandatory Doctor checks fails.
    */
   async doctor({plugin}) {
     return await super._doctor({

packages/appium/lib/doctor/doctor.js

@@ -2,6 +2,11 @@ import '@colors/colors';
 import _ from 'lodash';
 import { util, doctor, logger } from '@appium/support';
 
+export const EXIT_CODE = /** @type {const} */ Object.freeze({
+  SUCCESS: 0,
+  HAS_MAJOR_ISSUES: 127,
+});
+
 export class Doctor {
   /**
    * @param {DoctorCheck[]} [checks=[]]
@@ -68,7 +73,12 @@ export class Doctor {
       /** @type {string[]} */
       const fixMessages = [];
       for (const issue of issues) {
-        const message = await issue.check.fix();
+        let message;
+        try {
+          message = await issue.check.fix();
+        } catch (e) {
+          message = e.message;
+        }
         if (message) {
           fixMessages.push(message);
         }
@@ -84,7 +94,7 @@ export class Doctor {
       'The configuration cannot be automatically fixed, please do the following first:',
     ], manualIssues);
     await handleIssues([
-      '### Optional Manual Fixes  ###',
+      '### Optional Manual Fixes ###',
       'To fix these optional issues, please do the following manually:',
     ], manualIssuesOptional);
 
@@ -127,6 +137,9 @@ export class Doctor {
     }
   }
 
+  /**
+   * @returns {Promise<boolean>}
+   */
   async runAutoFixes() {
     const autoFixes = _.filter(this.foundIssues, (f) => f.check.hasAutofix());
     for (const f of autoFixes) {
@@ -136,22 +149,30 @@ export class Doctor {
     if (_.find(autoFixes, (f) => !f.fixed)) {
       // a few issues remain.
       this.log.info('Bye! A few issues remain, fix manually and/or rerun doctor!');
-    } else {
-      // nothing left to fix.
-      this.log.info('Bye! All issues have been fixed!');
+      this.log.info('');
+      return false;
     }
+    // nothing left to fix.
+    this.log.info('Bye! All issues have been fixed!');
     this.log.info('');
+    return true;
   }
 
+  /**
+   * @returns {Promise<EXIT_CODE[keyof EXIT_CODE]>}
+   */
   async run() {
     await this.diagnose();
     if (this.reportSuccess()) {
-      return;
+      return EXIT_CODE.SUCCESS;
     }
     if (await this.reportManualIssues()) {
-      return;
+      return EXIT_CODE.HAS_MAJOR_ISSUES;
+    }
+    if (!await this.runAutoFixes()) {
+      return EXIT_CODE.HAS_MAJOR_ISSUES;
     }
-    await this.runAutoFixes();
+    return EXIT_CODE.SUCCESS;
   }
 
   /**

packages/appium/test/e2e/cli-driver.e2e.spec.js

@@ -423,8 +423,8 @@ describe('Driver CLI', function () {
 
     describe('when the driver defines doctor checks', function () {
       it('should load and run them', async function () {
-        const checks = await runDoctor([driverName]);
-        checks.length.should.eql(2);
+        const checksLen = await runDoctor([driverName]);
+        checksLen.should.eql(2);
       });
     });
   });

packages/fake-driver/doctor/common.js

@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/no-var-requires */
-const {fs, doctor} = require('@appium/support');
+const {doctor} = require('@appium/support');
 
 /** @satisfies {import('@appium/types').IDoctorCheck} */
 class EnvVarAndPathCheck {
@@ -11,16 +11,7 @@ class EnvVarAndPathCheck {
   }
 
   async diagnose() {
-    const varValue = process.env[this.varName];
-    if (typeof varValue === 'undefined') {
-      return doctor.nok(`${this.varName} environment variable is NOT set!`);
-    }
-
-    if (await fs.exists(varValue)) {
-      return doctor.ok(`${this.varName} is set to: ${varValue}`);
-    }
-
-    return doctor.nok(`${this.varName} is set to '${varValue}' but this is NOT a valid path!`);
+    return doctor.ok(`${this.varName} environment variable is always set because it's fake`);
   }
 
   async fix() {
@@ -39,7 +30,3 @@ class EnvVarAndPathCheck {
 }
 
 module.exports = {EnvVarAndPathCheck};
-
-/**
- * @typedef {import('@appium/types').DoctorCheckResult} CheckResult
- */