updates based on comments feedback

eleanorjboyd · eleanorjboyd · commit 5c23abb8f8aa · 2025-01-17T11:43:21.000-08:00
diff --git a/bundled/scripts/noConfigScripts/debugpy b/bundled/scripts/noConfigScripts/debugpy
@@ -1,4 +1,3 @@
 #! /bin/bash
 # Bash script
 python $BUNDLED_DEBUGPY_PATH --listen 0 --wait-for-client $@
-echo "Executed: python $BUNDLED_DEBUGPY_PATH --listen 0 --wait-for-client $@"
diff --git a/bundled/scripts/noConfigScripts/debugpy.bat b/bundled/scripts/noConfigScripts/debugpy.bat
@@ -1,3 +1,3 @@
 @echo off
 :: Bat script
-python %BUNDLED_DEBUGPY_PATH% --listen 0 --wait-for-client %*
+python %BUNDLED_DEBUGPY_PATH% --listen 0 --wait-for-client %*
diff --git a/src/extension/extensionInit.ts b/src/extension/extensionInit.ts
@@ -248,7 +248,9 @@ export async function registerDebugger(context: IExtensionContext): Promise<IExt
         window.activeTextEditor?.document.languageId === 'python',
     );
 
-    registerNoConfigDebug(context);
+    context.subscriptions.push(
+        await registerNoConfigDebug(context.environmentVariableCollection, context.extensionPath),
+    );
 
     return buildApi();
 }
diff --git a/src/extension/noConfigDebugInit.ts b/src/extension/noConfigDebugInit.ts
@@ -3,41 +3,61 @@
 
 import * as fs from 'fs';
 import * as path from 'path';
-import { IExtensionContext } from './common/types';
-import { DebugSessionOptions, RelativePattern } from 'vscode';
+import * as crypto from 'crypto';
+import * as os from 'os';
+import { DebugSessionOptions, Disposable, GlobalEnvironmentVariableCollection, RelativePattern } from 'vscode';
 import { createFileSystemWatcher, debugStartDebugging } from './utils';
-import { traceError, traceVerbose } from './common/log/logging';
+import { traceError, traceLog, traceVerbose } from './common/log/logging';
 
 /**
  * Registers the configuration-less debugging setup for the extension.
  *
  * This function sets up environment variables and a file system watcher to
  * facilitate debugging without requiring a pre-configured launch.json file.
  *
- * @param context - The extension context which provides access to the environment variable collection and subscriptions.
+ * @param envVarCollection - The collection of environment variables to be modified.
+ * @param extPath - The path to the extension directory.
  *
  * Environment Variables:
  * - `DEBUGPY_ADAPTER_ENDPOINTS`: Path to the file containing the debugger adapter endpoint.
  * - `BUNDLED_DEBUGPY_PATH`: Path to the bundled debugpy library.
  * - `PATH`: Appends the path to the noConfigScripts directory.
  */
-export async function registerNoConfigDebug(context: IExtensionContext): Promise<void> {
-    const collection = context.environmentVariableCollection;
+export async function registerNoConfigDebug(
+    envVarCollection: GlobalEnvironmentVariableCollection,
+    extPath: string,
+): Promise<Disposable> {
+    const collection = envVarCollection;
+
+    // create a temp directory for the noConfigDebugAdapterEndpoints
+    // file path format: tempDir/noConfigDebugAdapterEndpoints-<randomString>/debuggerAdapterEndpoint.txt
+    const randomSuffix = crypto.randomBytes(10).toString('hex');
+    const tempDirName = `noConfigDebugAdapterEndpoints-${randomSuffix}`;
+    let tempDirPath = path.join(os.tmpdir(), tempDirName);
+    try {
+        traceLog('Attempting to use temp directory for noConfigDebugAdapterEndpoints, dir name:', tempDirName);
+        await fs.promises.mkdir(tempDirPath, { recursive: true });
+    } catch (error) {
+        // Handle the error when accessing the temp directory
+        traceError('Error accessing temp directory:', error, ' Attempt to use extension root dir instead');
+        // Make new temp directory in extension root dird
+        tempDirPath = path.join(extPath, '.temp');
+        await fs.promises.mkdir(tempDirPath, { recursive: true });
+    }
+    const tempFilePath = path.join(tempDirPath, 'debuggerAdapterEndpoint.txt');
 
     // Add env vars for DEBUGPY_ADAPTER_ENDPOINTS, BUNDLED_DEBUGPY_PATH, and PATH
-    const debugAdapterEndpointDir = path.join(context.extensionPath, 'noConfigDebugAdapterEndpoints');
-    const debuggerAdapterEndpointPath = path.join(debugAdapterEndpointDir, 'debuggerAdapterEndpoint.txt');
-    collection.replace('DEBUGPY_ADAPTER_ENDPOINTS', debuggerAdapterEndpointPath);
+    collection.replace('DEBUGPY_ADAPTER_ENDPOINTS', tempFilePath);
 
-    const noConfigScriptsDir = path.join(context.extensionPath, 'bundled', 'scripts', 'noConfigScripts');
+    const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
     const pathSeparator = process.platform === 'win32' ? ';' : ':';
     collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`);
 
-    const bundledDebugPath = path.join(context.extensionPath, 'bundled', 'libs', 'debugpy');
+    const bundledDebugPath = path.join(extPath, 'bundled', 'libs', 'debugpy');
     collection.replace('BUNDLED_DEBUGPY_PATH', bundledDebugPath);
 
     // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written
-    const fileSystemWatcher = createFileSystemWatcher(new RelativePattern(debugAdapterEndpointDir, '**/*'));
+    const fileSystemWatcher = createFileSystemWatcher(new RelativePattern(tempDirPath, '**/*'));
     const fileCreationEvent = fileSystemWatcher.onDidCreate(async (uri) => {
         const filePath = uri.fsPath;
         fs.readFile(filePath, (err, data) => {
@@ -87,6 +107,10 @@ export async function registerNoConfigDebug(context: IExtensionContext): Promise
         });
         JSON.parse;
     });
-    context.subscriptions.push(fileCreationEvent);
-    context.subscriptions.push(fileSystemWatcher);
+    return Promise.resolve(
+        new Disposable(() => {
+            fileSystemWatcher.dispose();
+            fileCreationEvent.dispose();
+        }),
+    );
 }
diff --git a/src/test/unittest/noConfigDebugInit.unit.test.ts b/src/test/unittest/noConfigDebugInit.unit.test.ts
@@ -10,45 +10,44 @@ import { DebugConfiguration, DebugSessionOptions, RelativePattern, Uri } from 'v
 import * as utils from '../../extension/utils';
 import { assert } from 'console';
 import * as fs from 'fs';
+import * as os from 'os';
+import * as crypto from 'crypto';
 
 suite('setup for no-config debug scenario', function () {
     let envVarCollectionReplaceStub: sinon.SinonStub;
     let envVarCollectionAppendStub: sinon.SinonStub;
     let context: TypeMoq.IMock<IExtensionContext>;
-    let debugAdapterEndpointDir: string;
-    let debuggerAdapterEndpointPath: string;
     let noConfigScriptsDir: string;
     let bundledDebugPath: string;
     let DEBUGPY_ADAPTER_ENDPOINTS = 'DEBUGPY_ADAPTER_ENDPOINTS';
     let BUNDLED_DEBUGPY_PATH = 'BUNDLED_DEBUGPY_PATH';
+    let tempDirPath: string;
 
     const testDataDir = path.join(__dirname, 'testData');
     const testFilePath = path.join(testDataDir, 'debuggerAdapterEndpoint.txt');
-    suiteSetup(() => {
-        // create file called testData/debuggerAdapterEndpoint.txt
-        if (!fs.existsSync(testDataDir)) {
-            fs.mkdirSync(testDataDir);
-        }
-        fs.writeFileSync(testFilePath, JSON.stringify({ client: { port: 5678 } }));
-    });
     setup(() => {
-        context = TypeMoq.Mock.ofType<IExtensionContext>();
-
-        context.setup((c) => (c as any).extensionPath).returns(() => 'fake/extension/path');
-        context.setup((c) => c.subscriptions).returns(() => []);
-        debugAdapterEndpointDir = path.join(context.object.extensionPath, 'noConfigDebugAdapterEndpoints');
-        debuggerAdapterEndpointPath = path.join(debugAdapterEndpointDir, 'debuggerAdapterEndpoint.txt');
-        noConfigScriptsDir = path.join(context.object.extensionPath, 'bundled/scripts/noConfigScripts');
-        bundledDebugPath = path.join(context.object.extensionPath, 'bundled/libs/debugpy');
+        try {
+            context = TypeMoq.Mock.ofType<IExtensionContext>();
+
+            const randomSuffix = '1234567899';
+            const tempDirName = `noConfigDebugAdapterEndpoints-${randomSuffix}`;
+            tempDirPath = path.join(os.tmpdir(), tempDirName);
+            context.setup((c) => (c as any).extensionPath).returns(() => 'fake/extension/path');
+            context.setup((c) => c.subscriptions).returns(() => []);
+            noConfigScriptsDir = path.join(context.object.extensionPath, 'bundled/scripts/noConfigScripts');
+            bundledDebugPath = path.join(context.object.extensionPath, 'bundled/libs/debugpy');
+
+            // Stub crypto.randomBytes with proper typing
+            let randomBytesStub = sinon.stub(crypto, 'randomBytes');
+            // Provide a valid Buffer object
+            randomBytesStub.callsFake((_size: number) => Buffer.from('1234567899', 'hex'));
+        } catch (error) {
+            console.error('Error in setup:', error);
+        }
     });
     teardown(() => {
         sinon.restore();
     });
-    suiteTeardown(() => {
-        if (fs.existsSync(testDataDir)) {
-            fs.rmSync(testDataDir, { recursive: true, force: true });
-        }
-    });
 
     test('should add environment variables for DEBUGPY_ADAPTER_ENDPOINTS, BUNDLED_DEBUGPY_PATH, and PATH', async () => {
         const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();
@@ -60,7 +59,7 @@ suite('setup for no-config debug scenario', function () {
             .setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
             .callback((key, value) => {
                 if (key === DEBUGPY_ADAPTER_ENDPOINTS) {
-                    assert(value === debuggerAdapterEndpointPath);
+                    assert(value.includes('noConfigDebugAdapterEndpoints-1234567899'));
                 } else if (key === BUNDLED_DEBUGPY_PATH) {
                     assert(value === bundledDebugPath);
                 }
@@ -80,7 +79,7 @@ suite('setup for no-config debug scenario', function () {
         setupFileSystemWatchers();
 
         // run init for no config debug
-        await registerNoConfigDebug(context.object);
+        await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);
 
         // assert that functions called right number of times
         sinon.assert.calledTwice(envVarCollectionReplaceStub);
@@ -94,11 +93,11 @@ suite('setup for no-config debug scenario', function () {
         let createFileSystemWatcherFunct = setupFileSystemWatchers();
 
         // Act
-        await registerNoConfigDebug(context.object);
+        await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);
 
         // Assert
         sinon.assert.calledOnce(createFileSystemWatcherFunct);
-        const expectedPattern = new RelativePattern(debugAdapterEndpointDir, '**/*');
+        const expectedPattern = new RelativePattern(tempDirPath, '**/*');
         sinon.assert.calledWith(createFileSystemWatcherFunct, expectedPattern);
     });
 
@@ -127,7 +126,7 @@ suite('setup for no-config debug scenario', function () {
         const debugStub = sinon.stub(utils, 'debugStartDebugging').resolves(true);
 
         // Act
-        await registerNoConfigDebug(context.object);
+        await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);
 
         // Assert
         sinon.assert.calledOnce(debugStub);

Original file line number	Diff line number	Diff line change
`@@ -248,7 +248,9 @@ export async function registerDebugger(context: IExtensionContext): Promise<IExt`
`248`	`248`	`window.activeTextEditor?.document.languageId === 'python',`
`249`	`249`	`);`
`250`	`250`
`251`		`- registerNoConfigDebug(context);`
	`251`	`+ context.subscriptions.push(`
	`252`	`+ await registerNoConfigDebug(context.environmentVariableCollection, context.extensionPath),`
	`253`	`+ );`
`252`	`254`
`253`	`255`	`return buildApi();`
`254`	`256`	`}`