Remove verbose or debug environment variables from Ruby env (#1889)

* Use stubs to setup Ruby and Debugger test

* Remove verbose or debug environment variables

Author: vinistock

vscode/src/ruby.ts

@@ -150,7 +150,6 @@ export class Ruby implements RubyInterface {
       }
 
       this.fetchRubyVersionInfo();
-      this.deleteGcEnvironmentVariables();
       await this.setupBundlePath();
       this._error = false;
     } catch (error: any) {
@@ -172,6 +171,8 @@ export class Ruby implements RubyInterface {
     const { env, version, yjit } = await manager.activate();
     const [major, minor, _patch] = version.split(".").map(Number);
 
+    this.sanitizeEnvironment(env);
+
     // We need to set the process environment too to make other extensions such as Sorbet find the right Ruby paths
     process.env = env;
     this._env = env;
@@ -240,12 +241,20 @@ export class Ruby implements RubyInterface {
     }
   }
 
-  private deleteGcEnvironmentVariables() {
-    Object.keys(this._env).forEach((key) => {
+  // Deletes environment variables that are known to cause issues for launching the Ruby LSP. For example, GC tuning
+  // variables or verbose settings
+  private sanitizeEnvironment(env: NodeJS.ProcessEnv) {
+    // Delete all GC tuning variables
+    Object.keys(env).forEach((key) => {
       if (key.startsWith("RUBY_GC")) {
-        delete this._env[key];
+        delete env[key];
       }
     });
+
+    // Delete verbose or debug related settings. These often make Bundler or other dependencies print things to STDOUT,
+    // which breaks the client/server communication
+    delete env.VERBOSE;
+    delete env.DEBUG;
   }
 
   private async setupBundlePath() {

vscode/src/test/suite/debugger.test.ts

@@ -4,6 +4,7 @@ import * as path from "path";
 import * as os from "os";
 
 import * as vscode from "vscode";
+import sinon from "sinon";
 
 import { Debugger } from "../../debugger";
 import { Ruby, ManagerIdentifier } from "../../ruby";
@@ -158,21 +159,25 @@ suite("Debugger", () => {
 
   test("Launching the debugger", async () => {
     // eslint-disable-next-line no-process-env
-    if (process.env.CI) {
-      await vscode.workspace
-        .getConfiguration("rubyLsp")
-        .update("rubyVersionManager", ManagerIdentifier.None, true, true);
-    }
-
-    // By default, VS Code always saves all open files when launching a debugging session. This is a problem for tests
-    // because it attempts to save an untitled test file and then we get stuck in the save file dialog with no way of
-    // closing it. We have to disable that before running this test
-    const currentSaveBeforeStart = await vscode.workspace
-      .getConfiguration("debug")
-      .get("saveBeforeStart");
-    await vscode.workspace
-      .getConfiguration("debug")
-      .update("saveBeforeStart", "none", true, true);
+    const manager = process.env.CI
+      ? ManagerIdentifier.None
+      : ManagerIdentifier.Chruby;
+
+    const configStub = sinon
+      .stub(vscode.workspace, "getConfiguration")
+      .returns({
+        get: (name: string) => {
+          if (name === "rubyVersionManager") {
+            return manager;
+          } else if (name === "bundleGemfile") {
+            return "";
+          } else if (name === "saveBeforeStart") {
+            return "none";
+          }
+
+          return undefined;
+        },
+      } as unknown as vscode.WorkspaceConfiguration);
 
     const tmpPath = fs.mkdtempSync(
       path.join(os.tmpdir(), "ruby-lsp-test-debugger"),
@@ -225,14 +230,16 @@ suite("Debugger", () => {
     }
 
     // The debugger might take a bit of time to disconnect from the editor. We need to perform cleanup when we receive
-    // the termination callback or else we try to dispose of the debugger client too early
-    vscode.debug.onDidTerminateDebugSession(async (_session) => {
-      debug.dispose();
-      context.subscriptions.forEach((subscription) => subscription.dispose());
-      fs.rmSync(tmpPath, { recursive: true, force: true });
-      await vscode.workspace
-        .getConfiguration("debug")
-        .update("saveBeforeStart", currentSaveBeforeStart, true, true);
+    // the termination callback or else we try to dispose of the debugger client too early, but we need to wait for that
+    // so that we can clean up stubs otherwise they leak into other tests.
+    await new Promise<void>((resolve) => {
+      vscode.debug.onDidTerminateDebugSession((_session) => {
+        configStub.restore();
+        debug.dispose();
+        context.subscriptions.forEach((subscription) => subscription.dispose());
+        fs.rmSync(tmpPath, { recursive: true, force: true });
+        resolve();
+      });
     });
   }).timeout(45000);
 });

vscode/src/test/suite/ruby.test.ts

@@ -1,42 +1,50 @@
+/* eslint-disable no-process-env */
 import * as assert from "assert";
-import * as fs from "fs";
 import * as path from "path";
-import * as os from "os";
 
 import * as vscode from "vscode";
+import sinon from "sinon";
 
 import { Ruby, ManagerIdentifier } from "../../ruby";
 import { WorkspaceChannel } from "../../workspaceChannel";
 import { LOG_CHANNEL } from "../../common";
 
 suite("Ruby environment activation", () => {
-  let ruby: Ruby;
+  const workspacePath = path.dirname(
+    path.dirname(path.dirname(path.dirname(__dirname))),
+  );
+  const workspaceFolder: vscode.WorkspaceFolder = {
+    uri: vscode.Uri.file(workspacePath),
+    name: path.basename(workspacePath),
+    index: 0,
+  };
 
   test("Activate fetches Ruby information when outside of Ruby LSP", async () => {
-    if (os.platform() !== "win32") {
-      // eslint-disable-next-line no-process-env
-      process.env.SHELL = "/bin/bash";
-    }
+    const manager = process.env.CI
+      ? ManagerIdentifier.None
+      : ManagerIdentifier.Chruby;
 
-    const tmpPath = fs.mkdtempSync(path.join(os.tmpdir(), "ruby-lsp-test-"));
-    fs.writeFileSync(path.join(tmpPath, ".ruby-version"), "3.3.0");
+    const configStub = sinon
+      .stub(vscode.workspace, "getConfiguration")
+      .returns({
+        get: (name: string) => {
+          if (name === "rubyVersionManager") {
+            return manager;
+          } else if (name === "bundleGemfile") {
+            return "";
+          }
+
+          return undefined;
+        },
+      } as unknown as vscode.WorkspaceConfiguration);
 
     const context = {
       extensionMode: vscode.ExtensionMode.Test,
     } as vscode.ExtensionContext;
     const outputChannel = new WorkspaceChannel("fake", LOG_CHANNEL);
 
-    ruby = new Ruby(
-      context,
-      {
-        uri: vscode.Uri.file(tmpPath),
-      } as vscode.WorkspaceFolder,
-      outputChannel,
-    );
-    await ruby.activateRuby(
-      // eslint-disable-next-line no-process-env
-      process.env.CI ? ManagerIdentifier.None : ManagerIdentifier.Chruby,
-    );
+    const ruby = new Ruby(context, workspaceFolder, outputChannel);
+    await ruby.activateRuby();
 
     assert.ok(ruby.rubyVersion, "Expected Ruby version to be set");
     assert.notStrictEqual(
@@ -45,6 +53,46 @@ suite("Ruby environment activation", () => {
       "Expected YJIT support to be set to true or false",
     );
 
-    fs.rmSync(tmpPath, { recursive: true, force: true });
+    configStub.restore();
+  }).timeout(10000);
+
+  test("Deletes verbose and GC settings from activated environment", async () => {
+    const manager = process.env.CI
+      ? ManagerIdentifier.None
+      : ManagerIdentifier.Chruby;
+
+    const configStub = sinon
+      .stub(vscode.workspace, "getConfiguration")
+      .returns({
+        get: (name: string) => {
+          if (name === "rubyVersionManager") {
+            return manager;
+          } else if (name === "bundleGemfile") {
+            return "";
+          }
+
+          return undefined;
+        },
+      } as unknown as vscode.WorkspaceConfiguration);
+
+    const context = {
+      extensionMode: vscode.ExtensionMode.Test,
+    } as vscode.ExtensionContext;
+    const outputChannel = new WorkspaceChannel("fake", LOG_CHANNEL);
+
+    const ruby = new Ruby(context, workspaceFolder, outputChannel);
+
+    process.env.VERBOSE = "1";
+    process.env.DEBUG = "WARN";
+    process.env.RUBY_GC_HEAP_GROWTH_FACTOR = "1.7";
+    await ruby.activateRuby();
+
+    assert.strictEqual(ruby.env.VERBOSE, undefined);
+    assert.strictEqual(ruby.env.DEBUG, undefined);
+    assert.strictEqual(ruby.env.RUBY_GC_HEAP_GROWTH_FACTOR, undefined);
+    delete process.env.VERBOSE;
+    delete process.env.DEBUG;
+    delete process.env.RUBY_GC_HEAP_GROWTH_FACTOR;
+    configStub.restore();
   });
 });