auto recovery with login-shell when encountered command not found iss…

…ue (jest-community#941) * added fallback to login-shell logic * update doc * adding test and correct a regex
legend1202 · Nov 11, 2022 · d2725b5 · d2725b5
1 parent c7f6610
commit d2725b5
Show file tree

Hide file tree

Showing 17 changed files with 505 additions and 170 deletions.
diff --git a/README.md b/README.md
@@ -67,6 +67,7 @@ Content
   - [Troubleshooting](#troubleshooting)
     - [Jest failed to run](#jest-failed-to-run)
     - [Performance issue?](#performance-issue)
+    - [Intermittent errors for (npm/yar/node) command not found during test run or debugging](#intermittent-errors-for-npmyarnode-command-not-found-during-test-run-or-debugging)
     - [I don't see "Jest" in the bottom status bar](#i-dont-see-jest-in-the-bottom-status-bar)
     - [What to do with "Long Running Tests Warning"](#what-to-do-with-long-running-tests-warning)
     - [The tests and status do not match or some tests showing question marks unexpectedly?](#the-tests-and-status-do-not-match-or-some-tests-showing-question-marks-unexpectedly)
@@ -169,13 +170,13 @@ You can customize coverage start up behavior, style and colors, see [customizati
 
 ### How to use the extension with monorepo projects?
 
+The easiest way to setup the monorepo projects is to use the [Setup Tool](setup-wizard.md#setup-monorepo-project) and choose **Setup monorepo project**
+
 The extension supports monorepo projects in the following configurations:
 
 1. Single-root workspace: If all tests from monorepo packages can be run from a centralized location, such as project root, then a single-root workspace with proper ["jest.jestCommandLine"](#jestcommandline) and ["jest.rootPath"](#rootpath) setting should work. 
 2. Multi-root workspace: If each monorepo package has its own local jest root and configuration, a [multi-root workspaces](https://code.visualstudio.com/docs/editor/multi-root-workspaces) is required. Users can use `"jest.disabledWorkspaceFolders"` to exclude the packages from jest run. 
 
-Users might find it easier to setup monorepo project via a [Setup Tool](setup-wizard.md).
-
 Please note, a working jest environment is a prerequisite for this extension. If you are having problem running the tests from a terminal, please follow [jest](https://jestjs.io/docs/configuration) instruction to set it up first.
 
 ### How to read the StatusBar?
@@ -428,6 +429,9 @@ interface LoginShell
 By default, jest command is executed in default shell ('cmd' for windows, '/bin/sh' for non-windows). Users can use the `"jest.shell"` setting to either pass the path of another shell (e.g. "/bin/zsh") or a LoginShell config, basically a shell path and login arguments (e.g. `{"path": "/bin/bash", "args": ["--login"]}`)
 
 Note the LoginShell is only applicable for non-windows platform and could cause a bit more overhead.
+
+<a id="auto-fallback-login-shell"></a>
+_Note_: If detected shell env issue, such as `node: command not found` or `npm: no such file or directory`, the extension will fallback to a login shell to ensure tests can run correctly. If will try to auto generate a login shell configuration based on the `jest.shell` setting, otherwise, it will use the default `bash` login-shell. Currently supported auto-fallback shells are `bash`, `zsh`, `fish`.
 ### Debug Config
 
 This extension looks for jest specific debug config (`"vscode-jest-tests"` or `"vscode-jest-tests.v2"`) in the following order:
@@ -555,20 +559,13 @@ Sorry you are having trouble with the extension. If your issue did not get resol
 
   If you can run jest manually in the terminal but the extension showed error like "xxx ended unexpectedly", following are the most common causes (see [self-diagnosis](#how-to-see-more-debug-info-self-diagnosis) if you need more debug info):
 
-  - <a id="trouble-shell-env"></a>**runtime environment issue**: such as the shell env is not fully initialized upon vscode start up. A good indicator is messages prefixed with **"env:"**, or node/yarn/npm command not found, such as `env: node: No such file or directory` 
-    - This should only happened in Linux or MacOS and is caused by vscode not able to fully initialize the shell env when it starts up (more details [here](https://code.visualstudio.com/docs/supporting/faq#_resolving-shell-environment-fails)).
-    - The extension usually spawns the jest process with a non-login/not-interactive shell, which inherits the vscode env. If vscode env is not complete, the jest process could fail for example the `PATH` env variable is not correct. (more details [here](https://github.com/jest-community/vscode-jest/issues/741#issuecomment-921222851))
-    - There are many ways to workaround such issues:
-      - simply restart vscode sometimes can fix it
-      - start vscode from a terminal 
-      - add `PATH` directly to `jest.nodeEnv` settings, if that is the only problem.
-      - force the jest command to be executed in a login shell by setting ["jest.shell"](#shell) to a LoginShell. Note this might have some slight performance overhead. 
+
   - <a id="trouble-jest-cmdline"></a>**jest command line issue**: such as you usually run `yarn test` but the extension uses the default `jest` instead.
     - Try configuring the [jest.jestCommandLine](#jestcommandline) to mimic how you run jest from the terminal, such as `yarn test` or `npm run test --`. The extension can auto-config common configurations like create react apps but not custom scripts like [CRACO](https://github.com/gsoft-inc/craco).
     - or you can use the **"Run Setup Tool"** button in the error panel to resolve the configuration issue, see [Setup Tool](setup-wizard.md).  
   - **monorepo project issue**: you have a monorepo project but might not have been set up properly. 
-    - see more in [monorepo projects](#how-to-use-the-extension-with-monorepo-projects) on how to set it up.
-
+    - short answer is try [Setup monorepo project](setup-wizard.md#setup-monorepo-project) tool. Or read more detail in [how to use the extension with monorepo projects](#how-to-use-the-extension-with-monorepo-projects).
+  
 There could be other causes, such as jest test root path is different from the project's, which can be fixed by setting [jest.rootPath](#rootPath). Feel free to check out the [customization](#customization) section to manually adjust the extension if needed.
 
 ### Performance issue? 
@@ -593,6 +590,26 @@ Every project and developer are different. Experiment and pick the autoRun setti
 
 </details>
 
+### Intermittent errors for (npm/yar/node) command not found during test run or debugging
+
+This should only happen in Linux or MacOS, and is due to vscode not able to fully initialize the shell env when it starts up (more details [here](https://code.visualstudio.com/docs/supporting/faq#_resolving-shell-environment-fails)).
+
+- for test run:
+  A solution is introduced in [v5.0.2](release-notes/release-note-v5.md#v50-pre-release-roll-up), which will [automatically fallback to a login-shell](#auto-fallback-login-shell) during such situation. Hopefully, this should not be an issue any more 🤞.
+- for test debugging:
+  - you can instruct vscode debugger to use a login shell via [task/debug profile](https://code.visualstudio.com/docs/terminal/profiles#_configuring-the-taskdebug-profile), for example, adding the following in your user's settings then restart:
+
+    ```json
+    "terminal.integrated.automationProfile.osx": {
+      "args": ["-l"],
+      "path": "/bin/bash"
+    },
+    ```
+
+Alternatively, you can try the following methods if you prefer a non-login-shell solution:
+  - simply restart vscode sometimes can fix it
+  - start vscode from a terminal: type `code` from your external terminal
+
 ### I don't see "Jest" in the bottom status bar
 This means the extension is not activated. 
 

diff --git a/package.json b/package.json
@@ -501,7 +501,7 @@
   "dependencies": {
     "istanbul-lib-coverage": "^3.0.0",
     "istanbul-lib-source-maps": "^4.0.0",
-    "jest-editor-support": "^30.2.0"
+    "jest-editor-support": "^30.2.1"
   },
   "devDependencies": {
     "@types/istanbul-lib-coverage": "^2.0.2",

diff --git a/src/JestExt/core.ts b/src/JestExt/core.ts
@@ -219,11 +219,12 @@ export class JestExt {
     return actions;
   };
   private createProcessSession(): ProcessSession {
-    return createProcessSession({
+    const sessionContext = {
       ...this.extContext,
       updateWithData: this.updateWithData.bind(this),
       onRunEvent: this.events.onRunEvent,
-    });
+    };
+    return createProcessSession(sessionContext);
   }
   private toSBStats(stats: TestStats): SBTestStats {
     return { ...stats, isDirty: this.dirtyFiles.size > 0 };
@@ -630,6 +631,17 @@ export class JestExt {
     // restart jest since coverage condition has changed
     this.triggerUpdateSettings(this.extContext.settings);
   }
+  enableLoginShell(): void {
+    if (this.extContext.settings.shell.useLoginShell) {
+      return;
+    }
+    this.extContext.settings.shell.enableLoginShell();
+    this.triggerUpdateSettings(this.extContext.settings);
+    this.extContext.output.write(
+      `possible process env issue detected, restarting with a login-shell...\r\n`,
+      'warn'
+    );
+  }
 
   private setupStatusBar(): void {
     this.updateStatusBar({ state: 'initial' });

diff --git a/src/JestExt/helper.ts b/src/JestExt/helper.ts
@@ -17,9 +17,10 @@ import { pathToJest, pathToConfig, toFilePath } from '../helpers';
 import { workspaceLogging } from '../logging';
 import { JestExtContext, RunnerWorkspaceOptions } from './types';
 import { CoverageColors } from '../Coverage';
-import { platform, userInfo } from 'os';
+import { userInfo } from 'os';
 import { JestOutputTerminal } from './output-terminal';
 import { AutoRun } from './auto-run';
+import { RunShell } from './run-shell';
 
 export const isWatchRequest = (request: JestProcessRequest): boolean =>
   request.type === 'watch-tests' || request.type === 'watch-all-tests';
@@ -74,7 +75,7 @@ export const createJestExtContext = (
       options?.collectCoverage ?? settings.showCoverageOnLoad,
       settings.debugMode,
       settings.nodeEnv,
-      settings.shell
+      settings.shell.toSetting()
     );
   };
   const output = new JestOutputTerminal(workspaceFolder.name);
@@ -87,33 +88,6 @@ export const createJestExtContext = (
   };
 };
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const isLoginShell = (arg: any): arg is LoginShell =>
-  arg && typeof arg.path === 'string' && Array.isArray(arg.args);
-
-const getShell = (config: vscode.WorkspaceConfiguration): string | LoginShell | undefined => {
-  const shell = config.get<string | LoginShell>('shell');
-
-  if (!shell || typeof shell === 'string') {
-    return shell;
-  }
-
-  if (isLoginShell(shell)) {
-    if (platform() === 'win32') {
-      console.error(`LoginShell is not supported for windows currently.`);
-      return;
-    }
-    if (shell.args.length <= 0) {
-      console.error(
-        'Invalid login-shell arguments. Expect arguments like "--login" or "-l", but got:',
-        shell.args.length
-      );
-      return;
-    }
-    return shell;
-  }
-};
-
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const isTestExplorerConfigLegacy = (arg: any): arg is TestExplorerConfigLegacy =>
   typeof arg.enabled === 'boolean';
@@ -161,7 +135,7 @@ export const getExtensionResourceSettings = (uri: vscode.Uri): PluginResourceSet
     coverageColors: config.get<CoverageColors>('coverageColors'),
     testExplorer: getTestExplorer(config),
     nodeEnv: config.get<NodeEnv | null>('nodeEnv') ?? undefined,
-    shell: getShell(config) ?? undefined,
+    shell: new RunShell(config.get<string | LoginShell>('shell')),
     monitorLongRun: config.get<MonitorLongRun>('monitorLongRun') ?? undefined,
     autoRun: new AutoRun(
       config.get<JestExtAutoRunSetting | null>('autoRun'),

diff --git a/src/JestExt/process-listeners.ts b/src/JestExt/process-listeners.ts
@@ -6,16 +6,28 @@ import { ListenerSession, ListTestFilesCallback } from './process-session';
 import { Logging } from '../logging';
 import { JestRunEvent } from './types';
 import { MonitorLongRun } from '../Settings';
+import { extensionName } from '../appGlobals';
+import { RunShell } from './run-shell';
 
+// command not found error for anything but "jest", as it most likely not be caused by env issue
+const POSSIBLE_ENV_ERROR_REGEX =
+  /^(((?!(jest|react-scripts)).)*)(command not found|no such file or directory)/im;
 export class AbstractProcessListener {
   protected session: ListenerSession;
   protected readonly logging: Logging;
   public onRunEvent: vscode.EventEmitter<JestRunEvent>;
 
+  // flag indicating command not found due to process env issue
+  protected CmdNotFoundEnv: boolean;
+  private useLoginShell: RunShell['useLoginShell'];
+
   constructor(session: ListenerSession) {
     this.session = session;
     this.logging = session.context.loggingFactory.create(this.name);
     this.onRunEvent = session.context.onRunEvent;
+
+    this.CmdNotFoundEnv = false;
+    this.useLoginShell = session.context.settings.shell.useLoginShell;
   }
   protected get name(): string {
     return 'AbstractProcessListener';
@@ -67,35 +79,47 @@ export class AbstractProcessListener {
 
   protected onProcessStarting(process: JestProcess): void {
     this.session.context.onRunEvent.fire({ type: 'process-start', process });
-    this.logging('debug', `${process.request.type} onProcessStarting`);
   }
-  protected onExecutableStdErr(process: JestProcess, data: string, _raw: string): void {
-    this.logging('debug', `${process.request.type} onExecutableStdErr:`, data);
+  protected onExecutableStdErr(_process: JestProcess, data: string, _raw: string): void {
+    if (POSSIBLE_ENV_ERROR_REGEX.test(data)) {
+      this.CmdNotFoundEnv = true;
+    }
   }
-  protected onExecutableJSON(process: JestProcess, data: JestTotalResults): void {
-    this.logging('debug', `${process.request.type} onExecutableJSON:`, data);
+  protected onExecutableJSON(_process: JestProcess, _data: JestTotalResults): void {
+    // no default behavior...
   }
-  protected onExecutableOutput(process: JestProcess, data: string, _raw: string): void {
-    this.logging('debug', `${process.request.type} onExecutableOutput:`, data);
+  protected onExecutableOutput(_process: JestProcess, _data: string, _raw: string): void {
+    // no default behavior...
   }
   protected onTerminalError(process: JestProcess, data: string, _raw: string): void {
     this.logging('error', `${process.request.type} onTerminalError:`, data);
   }
   protected onProcessClose(_process: JestProcess, _code?: number, _signal?: string): void {
     // no default behavior...
   }
-  protected onProcessExit(process: JestProcess, code?: number, signal?: string): void {
-    // default behavior: logging error
-    if (this.isProcessError(code)) {
-      const error = `${process.request.type} onProcessExit: process exit with code=${code}, signal=${signal}`;
-      this.logging('warn', `${error} :`, process.toString());
+  protected onProcessExit(_process: JestProcess, _code?: number, _signal?: string): void {
+    // no default behavior
+  }
+
+  /**
+   * retry the process with login shell if possible. return true if will retry, otherwise false.
+   * @param process
+   * @param code
+   * @param signal
+   */
+  protected retryWithLoginShell(process: JestProcess, code?: number, signal?: string): boolean {
+    const msg = `${process.id} exit with code=${code}, signal=${signal}`;
+
+    if (code && code >= 127 && this.CmdNotFoundEnv && !this.useLoginShell) {
+      // enable login-shell
+      this.logging('debug', `${msg}; will retry with login-shell`);
+      vscode.commands.executeCommand(
+        `${extensionName}.with-workspace.enable-login-shell`,
+        this.session.context.workspace
+      );
+      return true;
     }
-  }
-  protected isProcessError(code?: number): boolean {
-    // code = 1 is general error, usually mean the command emit error, which should already handled by other event processing, for example when jest has failed tests.
-    // However, error beyond 1, usually means some error outside of the command it is trying to execute, so reporting here for debugging purpose
-    // see shell error code: https://www.linuxjournal.com/article/10844
-    return code != null && code > 1;
+    return false;
   }
 }
 
@@ -106,7 +130,6 @@ export class ListTestFileListener extends AbstractProcessListener {
   }
   private buffer = '';
   private stderrOutput = '';
-  private exitCode?: number;
   private onResult: ListTestFilesCallback;
 
   constructor(session: ListenerSession, onResult: ListTestFilesCallback) {
@@ -122,16 +145,12 @@ export class ListTestFileListener extends AbstractProcessListener {
     this.stderrOutput += raw;
   }
 
-  protected onProcessExit(process: JestProcess, code?: number, signal?: string): void {
-    // Note: will not fire 'exit' event, as the output is reported via the onResult() callback
-    super.onProcessExit(process, code, signal);
-    this.exitCode = code;
-  }
-
-  protected onProcessClose(process: JestProcess): void {
-    super.onProcessClose(process);
-    if (this.exitCode !== 0) {
-      return this.onResult(undefined, this.stderrOutput, this.exitCode);
+  protected onProcessClose(process: JestProcess, code?: number, signal?: string): void {
+    if (code !== 0) {
+      if (super.retryWithLoginShell(process, code, signal)) {
+        return;
+      }
+      return this.onResult(undefined, this.stderrOutput, code);
     }
 
     try {
@@ -151,7 +170,7 @@ export class ListTestFileListener extends AbstractProcessListener {
       return this.onResult(uriFiles);
     } catch (e) {
       this.logging('warn', 'failed to parse result:', this.buffer, 'error=', e);
-      this.onResult(undefined, toErrorString(e), this.exitCode);
+      this.onResult(undefined, toErrorString(e), code);
     }
   }
 }
@@ -211,7 +230,6 @@ export class RunTestListener extends AbstractProcessListener {
   // fire long-run warning once per run
   private longRunMonitor: LongRunMonitor;
   private runInfo: RunInfo | undefined;
-  private exitCode?: number;
 
   constructor(session: ListenerSession) {
     super(session);
@@ -332,6 +350,7 @@ export class RunTestListener extends AbstractProcessListener {
     if (this.shouldIgnoreOutput(message)) {
       return;
     }
+    super.onExecutableStdErr(process, message, raw);
 
     const cleaned = this.cleanupOutput(raw);
     this.handleRunStart(process, message);
@@ -388,15 +407,16 @@ export class RunTestListener extends AbstractProcessListener {
   protected onTerminalError(process: JestProcess, data: string, raw: string): void {
     this.onRunEvent.fire({ type: 'data', process, text: data, raw, newLine: true, isError: true });
   }
-  protected onProcessExit(process: JestProcess, code?: number, signal?: string): void {
-    this.runEnded();
-    super.onProcessExit(process, code, signal);
-    this.exitCode = code;
-  }
 
-  protected onProcessClose(process: JestProcess): void {
-    super.onProcessClose(process);
+  protected onProcessClose(process: JestProcess, code?: number, signal?: string): void {
+    this.runEnded();
     const error = this.handleWatchProcessCrash(process);
-    this.onRunEvent.fire({ type: 'exit', process, error, code: this.exitCode });
+
+    if (code && code > 1) {
+      if (this.retryWithLoginShell(process, code, signal)) {
+        return;
+      }
+    }
+    this.onRunEvent.fire({ type: 'exit', process, error, code });
   }
 }