addressed comments

Author: eleanorjboyd

src/client/testing/common/debugLauncher.ts

@@ -41,24 +41,7 @@ export class DebugLauncher implements ITestDebugLauncher {
 
     /**
      * Launches a debug session for test execution.
-     *
-     * **Cancellation handling:**
-     * Cancellation can occur from multiple sources, all properly handled:
-     * 1. **Pre-check**: If already cancelled before starting, returns immediately
-     * 2. **Token cancellation**: If the parent CancellationToken fires during debugging,
-     *    the deferred resolves and the callback is invoked to clean up resources
-     * 3. **Session termination**: When the user stops debugging (via UI or completes),
-     *    the onDidTerminateDebugSession event fires and we resolve
-     *
-     * **Multi-session support:**
-     * When debugging tests from multiple projects simultaneously, each launchDebugger()
-     * call needs to track its own debug session independently. We use a unique marker
-     * in the launch configuration to identify which session belongs to which call,
-     * avoiding race conditions with the global `activeDebugSession` property.
-     *
-     * @param options Launch configuration including test provider, args, and optional project info
-     * @param callback Called when the debug session ends (for cleanup like closing named pipes)
-     * @param sessionOptions VS Code debug session options (e.g., testRun association)
+     * Handles cancellation, multi-session support via unique markers, and cleanup.
      */
     public async launchDebugger(
         options: LaunchOptions,
@@ -105,12 +88,7 @@ export class DebugLauncher implements ITestDebugLauncher {
         );
         const debugManager = this.serviceContainer.get<IDebugService>(IDebugService);
 
-        // Generate a unique marker for this debug session.
-        // When multiple debug sessions start in parallel (e.g., debugging tests from
-        // multiple projects), we can't rely on debugManager.activeDebugSession because
-        // it's a global that could be overwritten by another concurrent session start.
-        // Instead, we embed a unique marker in our launch configuration and match it
-        // when the session starts to identify which session is ours.
+        // Unique marker to identify this session among concurrent debug sessions
         const sessionMarker = `test-${Date.now()}-${Math.random().toString(36).substring(7)}`;
         launchArgs[TEST_SESSION_MARKER_KEY] = sessionMarker;
 
@@ -341,7 +319,8 @@ export class DebugLauncher implements ITestDebugLauncher {
         launchArgs.purpose = [];
 
         // For project-based execution, get the Python path from the project's environment.
-        // This ensures debug sessions use the correct interpreter for each project.
+        // Fallback: if env API unavailable or fails, LaunchConfigurationResolver already set
+        // launchArgs.python from the active interpreter, so debugging still works.
         if (options.project && envExtApi.useEnvExtension()) {
             try {
                 const pythonEnv = await envExtApi.getEnvironment(options.project.uri);

src/client/testing/common/types.ts

@@ -27,12 +27,7 @@ export type LaunchOptions = {
     pytestPort?: string;
     pytestUUID?: string;
     runTestIdsPort?: string;
-    /**
-     * Optional Python project for project-based execution.
-     * When provided, the debug launcher will:
-     * - Use the project's associated Python environment
-     * - Name the debug session after the project
-     */
+    /** Optional Python project for project-based execution. */
     project?: PythonProject;
 };
 

src/client/testing/testController/common/projectTestExecution.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 import { CancellationToken, FileCoverageDetail, TestItem, TestRun, TestRunProfileKind, TestRunRequest } from 'vscode';
-import { traceError, traceInfo, traceVerbose } from '../../../logging';
+import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging';
 import { sendTelemetryEvent } from '../../../telemetry';
 import { EventName } from '../../../telemetry/constants';
 import { IPythonExecutionFactory } from '../../../common/process/types';
@@ -71,7 +71,7 @@ export async function executeTestsForProjects(
         traceInfo(`[test-by-project] Executing ${items.length} test item(s) for project: ${project.projectName}`);
 
         sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, {
-            tool: 'pytest',
+            tool: project.testProvider,
             debugging: isDebugMode,
         });
 
@@ -100,10 +100,7 @@ export async function executeTestsForProjects(
 }
 
 /**
- * Lookup context for project resolution during a single test run.
- * Maps file paths to their resolved ProjectAdapter to avoid
- * repeated API calls and linear searches.
- * Created fresh per run and discarded after grouping completes.
+ * Lookup context for avoiding redundant API calls within a single test run.
  */
 interface ProjectLookupContext {
     /** Maps file URI fsPath → resolved ProjectAdapter (or undefined if no match) */
@@ -113,15 +110,8 @@ interface ProjectLookupContext {
 }
 
 /**
- * Groups test items by their owning project using the Python Environment API.
- * Each test item's URI is matched to a project via the API's getPythonProject method.
- * Falls back to path-based matching when the extension API is not available.
- *
- * Uses a per-run cache to avoid redundant API calls for test items sharing the same file.
- *
- * Time complexity: O(n + p) amortized, where n = test items, p = projects
- * - Building adapter lookup map: O(p)
- * - Each test item: O(1) amortized (cached after first lookup per unique file)
+ * Groups test items by their owning project. Uses env API when available, else falls back to path matching.
+ * Time complexity: O(n + p) amortized via per-run caching.
  */
 export async function groupTestItemsByProject(
     testItems: TestItem[],
@@ -134,7 +124,8 @@ export async function groupTestItemsByProject(
         result.set(getProjectId(project.projectUri), { project, items: [] });
     }
 
-    // Build lookup context for this run - O(p) setup, enables O(1) lookups
+    // Build lookup context for this run - O(p) one-time setup, enables O(1) lookups per item.
+    // When tests are from a single project, most lookups hit the cache after the first item.
     const lookupContext: ProjectLookupContext = {
         uriToAdapter: new Map(),
         projectPathToAdapter: new Map(projects.map((p) => [p.projectUri.fsPath, p])),
@@ -150,7 +141,7 @@ export async function groupTestItemsByProject(
             }
         } else {
             // If no project matches, log it
-            traceVerbose(`[test-by-project] Could not match test item ${item.id} to a project`);
+            traceWarn(`[test-by-project] Could not match test item ${item.id} to a project`);
         }
     }
 
@@ -165,12 +156,7 @@ export async function groupTestItemsByProject(
 }
 
 /**
- * Finds the project that owns a test item based on the test item's URI.
- * Uses the Python Environment extension API when available, falling back
- * to path-based matching (longest matching path prefix).
- *
- * Results are stored in the lookup context to avoid redundant API calls for items in the same file.
- * Time complexity: O(1) amortized with context, O(p) worst case on context miss.
+ * Finds the project that owns a test item. Uses env API when available, else path-based matching.
  */
 export async function findProjectForTestItem(
     item: TestItem,
@@ -188,7 +174,9 @@ export async function findProjectForTestItem(
 
     let result: ProjectAdapter | undefined;
 
-    // Try using the Python Environment extension API first
+    // Try using the Python Environment extension API first.
+    // Legacy path: when useEnvExtension() is false, this block is skipped and we go
+    // directly to findProjectByPath() below (path-based matching).
     if (useEnvExtension()) {
         try {
             const envExtApi = await getEnvExtApi();
@@ -206,7 +194,8 @@ export async function findProjectForTestItem(
         }
     }
 
-    // Fallback: path-based matching (most specific/longest path wins)
+    // Fallback: path-based matching when env API unavailable or didn't find a match.
+    // O(p) time complexity where p = number of projects.
     if (!result) {
         result = findProjectByPath(item, projects);
     }
@@ -220,8 +209,7 @@ export async function findProjectForTestItem(
 }
 
 /**
- * Finds the project that owns a test item using path-based matching.
- * Returns the most specific (longest path) matching project.
+ * Fallback: finds project using path-based matching. O(p) time complexity.
  */
 function findProjectByPath(item: TestItem, projects: ProjectAdapter[]): ProjectAdapter | undefined {
     if (!item.uri) return undefined;