Fix missing dev-requirements.txt in workspace dependencies list (#910)

Copilot · eleanorjboyd · web-flow · commit 55b8efd4077e · 2025-10-08T06:31:05.000Z
Fixes #506 Added an explicit glob pattern `*requirements*.txt` to ensure requirements files at the workspace root are reliably found. The fix includes: 1. **Additional search pattern**: Added `findFiles('*requirements*.txt')` to explicitly search the workspace root 2. **Deduplication logic**: Implemented deduplication using a Map keyed by file system path to handle overlapping patterns 3. **Comprehensive tests**: Created 5 new unit tests to validate the fix Now both patterns work together: - `**/*requirements*.txt` - finds requirements files in subdirectories - `*requirements*.txt` - finds requirements files at workspace root ## Requirements Files Now Discovered This fix ensures all common requirements file naming patterns are found: - `requirements.txt` - main dependencies - `dev-requirements.txt` - development dependencies ✨ - `test-requirements.txt` - testing dependencies ✨ - `docs-requirements.txt` - documentation dependencies ✨ - `prod-requirements.txt` - production dependencies ✨ - Any other `*requirements*.txt` variant ✨  <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Missing requirements file in workspace dependencies list</issue_title> > <issue_description>I have the following file structure > > ![Image](https://github.com/user-attachments/assets/fccc626f-dedd-4c6e-8d66-93b7fc47ec6e) > > - click + on venv in environments panel > - click to create environment in root directory > - custom environment > - install workspace dependencies > - I only see `requirements.txt` as an option > > I expect I would see both dev-requirements.txt and requirements.txt because they are both present in the root folder > > > ![Image](https://github.com/user-attachments/assets/0868fe35-f0c1-4b38-bf50-e40d7835a4f2) > > </issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@eleanorjboyd</author><body> > todo: adopt dev-requirements.txt as something we search in the python environment extension when finding dependency files.</body></comment_new> > </comments> > </details>  --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md
@@ -537,17 +537,16 @@ envConfig.inspect
 -   ❌ Tests that don't clean up mocks properly
 -   ❌ Overly complex test setup that's hard to understand
 
-## 🧠 Agent Learning Patterns
-
-### Key Implementation Insights
+## 🧠 Agent Learnings
 
 -   Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility (1)
 -   Use `runTests` tool for programmatic test execution rather than terminal commands for better integration and result parsing (1)
--   Mock wrapper functions (e.g., `workspaceApis.getConfiguration()`) instead of VS Code APIs directly to avoid stubbing issues (1)
+-   Mock wrapper functions (e.g., `workspaceApis.getConfiguration()`) instead of VS Code APIs directly to avoid stubbing issues (2)
 -   Start compilation with `npm run watch-tests` before test execution to ensure TypeScript files are built (1)
--   Use `sinon.match()` patterns for resilient assertions that don't break on minor output changes (1)
+-   Use `sinon.match()` patterns for resilient assertions that don't break on minor output changes (2)
 -   Fix test issues iteratively - run tests, analyze failures, apply fixes, repeat until passing (1)
 -   When fixing mock environment creation, use `null` to truly omit properties rather than `undefined` (1)
 -   Always recompile TypeScript after making import/export changes before running tests, as stubs won't work if they're applied to old compiled JavaScript that doesn't have the updated imports (2)
 -   Create proxy abstraction functions for Node.js APIs like `cp.spawn` to enable clean testing - use function overloads to preserve Node.js's intelligent typing while making the functions mockable (1)
 -   When unit tests fail with VS Code API errors like `TypeError: X is not a constructor` or `Cannot read properties of undefined (reading 'Y')`, check if VS Code APIs are properly mocked in `/src/test/unittests.ts` - add missing Task-related APIs (`Task`, `TaskScope`, `ShellExecution`, `TaskRevealKind`, `TaskPanelKind`) and namespace mocks (`tasks`) following the existing pattern of `mockedVSCode.X = vscodeMocks.vscMockExtHostedTypes.X` (1)
+-   Create minimal mock objects with only required methods and use TypeScript type assertions (e.g., mockApi as PythonEnvironmentApi) to satisfy interface requirements instead of implementing all interface methods when only specific methods are needed for the test (1)
diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts
@@ -183,13 +183,17 @@ export async function getProjectInstallable(
             const results: Uri[] = (
                 await Promise.all([
                     findFiles('**/*requirements*.txt', exclude, undefined, token),
+                    findFiles('*requirements*.txt', exclude, undefined, token),
                     findFiles('**/requirements/*.txt', exclude, undefined, token),
                     findFiles('**/pyproject.toml', exclude, undefined, token),
                 ])
             ).flat();
 
+            // Deduplicate by fsPath
+            const uniqueResults = Array.from(new Map(results.map((uri) => [uri.fsPath, uri])).values());
+
             const fsPaths = projects.map((p) => p.uri.fsPath);
-            const filtered = results
+            const filtered = uniqueResults
                 .filter((uri) => {
                     const p = api.getPythonProject(uri)?.uri.fsPath;
                     return p && fsPaths.includes(p);
diff --git a/src/test/managers/builtin/pipUtils.unit.test.ts b/src/test/managers/builtin/pipUtils.unit.test.ts
@@ -0,0 +1,202 @@
+import assert from 'assert';
+import * as path from 'path';
+import * as sinon from 'sinon';
+import { CancellationToken, Progress, ProgressOptions, Uri } from 'vscode';
+import { PythonEnvironmentApi, PythonProject } from '../../../api';
+import * as winapi from '../../../common/window.apis';
+import * as wapi from '../../../common/workspace.apis';
+import { getProjectInstallable } from '../../../managers/builtin/pipUtils';
+
+suite('Pip Utils - getProjectInstallable', () => {
+    let findFilesStub: sinon.SinonStub;
+    let withProgressStub: sinon.SinonStub;
+    // Minimal mock that only implements the methods we need for this test
+    // Using type assertion to satisfy TypeScript since we only need getPythonProject
+    let mockApi: { getPythonProject: (uri: Uri) => PythonProject | undefined };
+
+    setup(() => {
+        findFilesStub = sinon.stub(wapi, 'findFiles');
+        // Stub withProgress to immediately execute the callback
+        withProgressStub = sinon.stub(winapi, 'withProgress');
+        withProgressStub.callsFake(
+            async (
+                _options: ProgressOptions,
+                callback: (
+                    progress: Progress<{ message?: string; increment?: number }>,
+                    token: CancellationToken,
+                ) => Thenable<unknown>,
+            ) => {
+                return await callback(
+                    {} as Progress<{ message?: string; increment?: number }>,
+                    { isCancellationRequested: false } as CancellationToken,
+                );
+            },
+        );
+
+        const workspacePath = Uri.file('/test/path/root').fsPath;
+        mockApi = {
+            getPythonProject: (uri: Uri) => {
+                // Return a project for any URI in workspace
+                if (uri.fsPath.startsWith(workspacePath)) {
+                    return { name: 'workspace', uri: Uri.file(workspacePath) };
+                }
+                return undefined;
+            },
+        };
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('should find dev-requirements.txt at workspace root', async () => {
+        // Arrange: Mock findFiles to return both requirements.txt and dev-requirements.txt
+        findFilesStub.callsFake((pattern: string) => {
+            if (pattern === '**/*requirements*.txt') {
+                // This pattern might not match root-level files in VS Code
+                return Promise.resolve([]);
+            } else if (pattern === '*requirements*.txt') {
+                // This pattern should match root-level files
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([
+                    Uri.file(path.join(workspacePath, 'requirements.txt')),
+                    Uri.file(path.join(workspacePath, 'dev-requirements.txt')),
+                    Uri.file(path.join(workspacePath, 'test-requirements.txt')),
+                ]);
+            } else if (pattern === '**/requirements/*.txt') {
+                return Promise.resolve([]);
+            } else if (pattern === '**/pyproject.toml') {
+                return Promise.resolve([]);
+            }
+            return Promise.resolve([]);
+        });
+
+        // Act: Call getProjectInstallable
+        const workspacePath = Uri.file('/test/path/root').fsPath;
+        const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
+        const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
+
+        // Assert: Should find all three requirements files
+        assert.strictEqual(result.length, 3, 'Should find three requirements files');
+
+        const names = result.map((r) => r.name).sort();
+        assert.deepStrictEqual(
+            names,
+            ['dev-requirements.txt', 'requirements.txt', 'test-requirements.txt'],
+            'Should find requirements.txt, dev-requirements.txt, and test-requirements.txt',
+        );
+
+        // Verify each file has correct properties
+        result.forEach((item) => {
+            assert.strictEqual(item.group, 'Requirements', 'Should be in Requirements group');
+            assert.ok(item.args, 'Should have args');
+            assert.strictEqual(item.args?.length, 2, 'Should have 2 args');
+            assert.strictEqual(item.args?.[0], '-r', 'First arg should be -r');
+            assert.ok(item.uri, 'Should have a URI');
+        });
+    });
+
+    test('should deduplicate files found by multiple patterns', async () => {
+        // Arrange: Mock both patterns to return the same file
+        findFilesStub.callsFake((pattern: string) => {
+            if (pattern === '**/*requirements*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([Uri.file(path.join(workspacePath, 'dev-requirements.txt'))]);
+            } else if (pattern === '*requirements*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([
+                    Uri.file(path.join(workspacePath, 'dev-requirements.txt')),
+                    Uri.file(path.join(workspacePath, 'requirements.txt')),
+                ]);
+            } else if (pattern === '**/requirements/*.txt') {
+                return Promise.resolve([]);
+            } else if (pattern === '**/pyproject.toml') {
+                return Promise.resolve([]);
+            }
+            return Promise.resolve([]);
+        });
+
+        // Act: Call getProjectInstallable
+        const workspacePath = Uri.file('/test/path/root').fsPath;
+        const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
+        const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
+
+        // Assert: Should deduplicate and only have 2 unique files
+        assert.strictEqual(result.length, 2, 'Should deduplicate and have 2 unique files');
+
+        const names = result.map((r) => r.name).sort();
+        assert.deepStrictEqual(names, ['dev-requirements.txt', 'requirements.txt'], 'Should have deduplicated results');
+    });
+
+    test('should find requirements files in subdirectories', async () => {
+        // Arrange: Mock findFiles to return files in subdirectories
+        findFilesStub.callsFake((pattern: string) => {
+            if (pattern === '**/*requirements*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([Uri.file(path.join(workspacePath, 'subdir', 'dev-requirements.txt'))]);
+            } else if (pattern === '*requirements*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([Uri.file(path.join(workspacePath, 'requirements.txt'))]);
+            } else if (pattern === '**/requirements/*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                return Promise.resolve([Uri.file(path.join(workspacePath, 'requirements', 'test.txt'))]);
+            } else if (pattern === '**/pyproject.toml') {
+                return Promise.resolve([]);
+            }
+            return Promise.resolve([]);
+        });
+
+        // Act: Call getProjectInstallable
+        const workspacePath = Uri.file('/test/path/root').fsPath;
+        const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
+        const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
+
+        // Assert: Should find all files
+        assert.strictEqual(result.length, 3, 'Should find three files');
+
+        const names = result.map((r) => r.name).sort();
+        assert.deepStrictEqual(
+            names,
+            ['dev-requirements.txt', 'requirements.txt', 'test.txt'],
+            'Should find files at different levels',
+        );
+    });
+
+    test('should return empty array when no projects provided', async () => {
+        // Act: Call with no projects
+        const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, undefined);
+
+        // Assert: Should return empty array
+        assert.strictEqual(result.length, 0, 'Should return empty array');
+        assert.ok(!findFilesStub.called, 'Should not call findFiles when no projects');
+    });
+
+    test('should filter out files not in project directories', async () => {
+        // Arrange: Mock findFiles to return files from multiple directories
+        findFilesStub.callsFake((pattern: string) => {
+            if (pattern === '*requirements*.txt') {
+                const workspacePath = Uri.file('/test/path/root').fsPath;
+                const otherPath = Uri.file('/other-dir').fsPath;
+                return Promise.resolve([
+                    Uri.file(path.join(workspacePath, 'requirements.txt')),
+                    Uri.file(path.join(otherPath, 'requirements.txt')), // Should be filtered out
+                ]);
+            } else {
+                return Promise.resolve([]);
+            }
+        });
+
+        // Act: Call with only workspace project
+        const workspacePath = Uri.file('/test/path/root').fsPath;
+        const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
+        const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
+
+        // Assert: Should only include files from workspace
+        assert.strictEqual(result.length, 1, 'Should only include files from project directory');
+        const firstResult = result[0];
+        assert.ok(firstResult, 'Should have at least one result');
+        assert.strictEqual(firstResult.name, 'requirements.txt');
+        assert.ok(firstResult.uri, 'Should have a URI');
+        assert.ok(firstResult.uri.fsPath.startsWith(workspacePath), 'Should be in workspace directory');
+    });
+});
