From ead285b1b558b1f6845ecc4dcfbefd1eb10a8ff8 Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 11:33:20 -0800 Subject: [PATCH 1/7] Fix getIncludes call with E2E test. --- .../SingleRootProject/tests/copilotProviders.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index 626ed287c..00e14f434 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -26,11 +26,13 @@ describe('copilotProviders Tests', () => { let vscodeExtension: vscode.Extension; let telemetryStub: sinon.SinonStub; - const includedFiles = process.platform === 'win32' ? + const includedFiles: string[] = process.platform === 'win32' ? ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] : ['/system/include/vector', '/system/include/string', '/home/src/my_project/foo.h']; - const rootUri = vscode.Uri.file(process.platform === 'win32' ? 'C:\\src\\my_project' : '/home/src/my_project'); - const expectedInclude = process.platform === 'win32' ? 'file:///c%3A/src/my_project/foo.h' : 'file:///home/src/my_project/foo.h'; + const rootUri: vscode.Uri = vscode.Uri.file(process.platform === 'win32' ? 'C:\\src\\my_project' : '/home/src/my_project'); + const expectedInclude: string = process.platform === 'win32' ? 'file:///c%3A/src/my_project/foo.h' : 'file:///home/src/my_project/foo.h'; + const sourceFileUri: vscode.Uri = vscode.Uri.file(process.platform === 'win32' ? 'file:///c%3A/src/my_project/foo.cpp' : 'file:///home/src/my_project/foo.cpp'); + const maxDepth: number = 10; beforeEach(() => { proxyquire.noPreserveCache(); // Tells proxyquire to not fetch the module from cache @@ -71,7 +73,7 @@ describe('copilotProviders Tests', () => { activeClientStub = sinon.createStubInstance(DefaultClient); getActiveClientStub = sinon.stub(extension, 'getActiveClient').returns(activeClientStub); - activeClientStub.getIncludes.resolves({ includedFiles: [] }); + activeClientStub.getIncludes.resolves({ includedFiles: [] })(sourceFileUri, maxDepth); telemetryStub = sinon.stub(telemetry, 'logCopilotEvent').returns(); }); @@ -83,7 +85,7 @@ describe('copilotProviders Tests', () => { { vscodeExtension?: vscode.Extension; getIncludeFiles?: GetIncludesResult; projectContext?: ProjectContext; rootUri?: vscode.Uri; flags?: Record } = { vscodeExtension: undefined, getIncludeFiles: undefined, projectContext: undefined, rootUri: undefined, flags: {} } ) => { - activeClientStub.getIncludes.resolves(getIncludeFiles); + activeClientStub.getIncludes.resolves(getIncludeFiles)(sourceFileUri, maxDepth); sinon.stub(lmTool, 'getProjectContext').resolves(projectContext); sinon.stub(activeClientStub, 'RootUri').get(() => rootUri); mockCopilotApi.registerRelatedFilesProvider.callsFake((_providerId: { extensionId: string; languageId: string }, callback: (uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }>) => { From 774bb518de5f307ceaa5edd24ca70b07047bf241 Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 11:47:40 -0800 Subject: [PATCH 2/7] Try a fix. --- .../SingleRootProject/tests/copilotProviders.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index 00e14f434..fe5e96aa6 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -32,7 +32,6 @@ describe('copilotProviders Tests', () => { const rootUri: vscode.Uri = vscode.Uri.file(process.platform === 'win32' ? 'C:\\src\\my_project' : '/home/src/my_project'); const expectedInclude: string = process.platform === 'win32' ? 'file:///c%3A/src/my_project/foo.h' : 'file:///home/src/my_project/foo.h'; const sourceFileUri: vscode.Uri = vscode.Uri.file(process.platform === 'win32' ? 'file:///c%3A/src/my_project/foo.cpp' : 'file:///home/src/my_project/foo.cpp'); - const maxDepth: number = 10; beforeEach(() => { proxyquire.noPreserveCache(); // Tells proxyquire to not fetch the module from cache @@ -73,7 +72,7 @@ describe('copilotProviders Tests', () => { activeClientStub = sinon.createStubInstance(DefaultClient); getActiveClientStub = sinon.stub(extension, 'getActiveClient').returns(activeClientStub); - activeClientStub.getIncludes.resolves({ includedFiles: [] })(sourceFileUri, maxDepth); + activeClientStub.getIncludes.resolves({ includedFiles: [] }); telemetryStub = sinon.stub(telemetry, 'logCopilotEvent').returns(); }); @@ -85,14 +84,14 @@ describe('copilotProviders Tests', () => { { vscodeExtension?: vscode.Extension; getIncludeFiles?: GetIncludesResult; projectContext?: ProjectContext; rootUri?: vscode.Uri; flags?: Record } = { vscodeExtension: undefined, getIncludeFiles: undefined, projectContext: undefined, rootUri: undefined, flags: {} } ) => { - activeClientStub.getIncludes.resolves(getIncludeFiles)(sourceFileUri, maxDepth); + activeClientStub.getIncludes.resolves(getIncludeFiles); sinon.stub(lmTool, 'getProjectContext').resolves(projectContext); sinon.stub(activeClientStub, 'RootUri').get(() => rootUri); mockCopilotApi.registerRelatedFilesProvider.callsFake((_providerId: { extensionId: string; languageId: string }, callback: (uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }>) => { if (_providerId.languageId === 'cpp') { const tokenSource = new vscode.CancellationTokenSource(); try { - callbackPromise = callback(vscode.Uri.parse('file:///test-extension-path'), { flags: flags ?? {} }, tokenSource.token); + callbackPromise = callback(sourceFileUri, { flags: flags ?? {} }, tokenSource.token); } finally { tokenSource.dispose(); } From 578968a700aa3f6c217db534a31a08711bdee39a Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 12:22:37 -0800 Subject: [PATCH 3/7] Another fix. --- Extension/src/LanguageServer/copilotProviders.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index ce232865f..6409a7bc7 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -10,7 +10,7 @@ import * as util from '../common'; import * as logger from '../logger'; import * as telemetry from '../telemetry'; import { GetIncludesResult } from './client'; -import { getClients } from './extension'; +import { clients } from './extension'; import { getCompilerArgumentFilterMap, getProjectContext } from './lmTool'; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); @@ -158,7 +158,7 @@ export async function registerRelatedFilesProvider(): Promise { } async function getIncludes(uri: vscode.Uri, maxDepth: number): Promise { - const client = getClients().getClientFor(uri); + const client = clients.getClientFor(uri); const includes = await client.getIncludes(uri, maxDepth); const wksFolder = client.RootUri?.toString(); From dfead679c51684ba81c91cef564b80c7e8e1e8df Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 14:06:18 -0800 Subject: [PATCH 4/7] Finally fix it. Also, try enabling SingleRootProject for ci. --- .github/workflows/job-compile-and-test.yml | 4 ++++ Extension/src/LanguageServer/copilotProviders.ts | 4 ++-- .../SingleRootProject/tests/copilotProviders.test.ts | 9 ++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/job-compile-and-test.yml b/.github/workflows/job-compile-and-test.yml index 2b3998d49..60e84e5a8 100644 --- a/.github/workflows/job-compile-and-test.yml +++ b/.github/workflows/job-compile-and-test.yml @@ -42,6 +42,10 @@ jobs: run: yarn test working-directory: Extension + - name: Run SingleRootProject tests + run: yarn test --scenario=SingleRootProject + working-directory: Extension + # NOTE : We can't run the test that require the native binary files # yet -- there will be an update soon that allows the tester to # acquire them on-the-fly diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index 6409a7bc7..ce232865f 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -10,7 +10,7 @@ import * as util from '../common'; import * as logger from '../logger'; import * as telemetry from '../telemetry'; import { GetIncludesResult } from './client'; -import { clients } from './extension'; +import { getClients } from './extension'; import { getCompilerArgumentFilterMap, getProjectContext } from './lmTool'; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); @@ -158,7 +158,7 @@ export async function registerRelatedFilesProvider(): Promise { } async function getIncludes(uri: vscode.Uri, maxDepth: number): Promise { - const client = clients.getClientFor(uri); + const client = getClients().getClientFor(uri); const includes = await client.getIncludes(uri, maxDepth); const wksFolder = client.RootUri?.toString(); diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index fe5e96aa6..31b7615a4 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -10,6 +10,7 @@ import * as sinon from 'sinon'; import * as vscode from 'vscode'; import * as util from '../../../../src/common'; import { DefaultClient, GetIncludesResult } from '../../../../src/LanguageServer/client'; +import { ClientCollection } from '../../../../src/LanguageServer/clientCollection'; import { CopilotApi, CopilotTrait } from '../../../../src/LanguageServer/copilotProviders'; import * as extension from '../../../../src/LanguageServer/extension'; import * as lmTool from '../../../../src/LanguageServer/lmTool'; @@ -19,7 +20,7 @@ import * as telemetry from '../../../../src/telemetry'; describe('copilotProviders Tests', () => { let moduleUnderTest: any; let mockCopilotApi: sinon.SinonStubbedInstance; - let getActiveClientStub: sinon.SinonStub; + let getClientsStub: sinon.SinonStub; let activeClientStub: sinon.SinonStubbedInstance; let vscodeGetExtensionsStub: sinon.SinonStub; let callbackPromise: Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }> | undefined; @@ -71,7 +72,9 @@ describe('copilotProviders Tests', () => { }; activeClientStub = sinon.createStubInstance(DefaultClient); - getActiveClientStub = sinon.stub(extension, 'getActiveClient').returns(activeClientStub); + const clientsStub = sinon.createStubInstance(ClientCollection); + getClientsStub = sinon.stub(extension, 'getClients').returns(clientsStub); + clientsStub.getClientFor.returns(activeClientStub); activeClientStub.getIncludes.resolves({ includedFiles: [] }); telemetryStub = sinon.stub(telemetry, 'logCopilotEvent').returns(); }); @@ -130,7 +133,7 @@ describe('copilotProviders Tests', () => { ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once'); ok(mockCopilotApi.registerRelatedFilesProvider.calledWithMatch(sinon.match({ extensionId: 'test-extension-id', languageId: sinon.match.in(['c', 'cpp', 'cuda-cpp']) })), 'registerRelatedFilesProvider should be called with the correct providerId and languageId'); - ok(getActiveClientStub.callCount !== 0, 'getActiveClient should be called'); + ok(getClientsStub.callCount !== 0, 'getClients should be called'); ok(callbackPromise, 'callbackPromise should be defined'); ok(result, 'result should be defined'); ok(result.entries.length === 1, 'result.entries should have 1 included file'); From 22391b0c72d1bba6e1ec4daf934bcea5d05524d7 Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 14:28:12 -0800 Subject: [PATCH 5/7] Add a --skipCheckBinaries arg. --- .github/workflows/job-compile-and-test.yml | 3 ++- Extension/.scripts/common.ts | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/job-compile-and-test.yml b/.github/workflows/job-compile-and-test.yml index 60e84e5a8..703f1136d 100644 --- a/.github/workflows/job-compile-and-test.yml +++ b/.github/workflows/job-compile-and-test.yml @@ -42,8 +42,9 @@ jobs: run: yarn test working-directory: Extension + # These tests don't require the binary. - name: Run SingleRootProject tests - run: yarn test --scenario=SingleRootProject + run: yarn test --scenario=SingleRootProject --skipCheckBinaries working-directory: Extension # NOTE : We can't run the test that require the native binary files diff --git a/Extension/.scripts/common.ts b/Extension/.scripts/common.ts index 490d464c2..598ff97e8 100644 --- a/Extension/.scripts/common.ts +++ b/Extension/.scripts/common.ts @@ -334,6 +334,9 @@ export async function checkDTS() { } export async function checkBinaries() { + if ($switches.includes('--skipCheckBinaries')) { + return false; + } let failing = false; failing = !await assertAnyFile(['bin/cpptools.exe', 'bin/cpptools']) && (quiet || warn(`The native binary files are not present. You should either build or install the native binaries\n\n.`)) || failing; From 6496721d55c86b914c6385e8b67634053c5cd0f8 Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 14:41:21 -0800 Subject: [PATCH 6/7] Skip Linux. --- .github/workflows/job-compile-and-test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/job-compile-and-test.yml b/.github/workflows/job-compile-and-test.yml index 703f1136d..f5100f34e 100644 --- a/.github/workflows/job-compile-and-test.yml +++ b/.github/workflows/job-compile-and-test.yml @@ -43,7 +43,9 @@ jobs: working-directory: Extension # These tests don't require the binary. + # On Linux, it is failing with: Test run terminated with signal SIGSEGV - name: Run SingleRootProject tests + if: ${{ inputs.platform != 'linux' }} run: yarn test --scenario=SingleRootProject --skipCheckBinaries working-directory: Extension From 1df5fcce4def13c5b21a771a3be5cbf220c85060 Mon Sep 17 00:00:00 2001 From: Sean McManus Date: Fri, 27 Dec 2024 15:30:35 -0800 Subject: [PATCH 7/7] Update comment. --- .github/workflows/job-compile-and-test.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/job-compile-and-test.yml b/.github/workflows/job-compile-and-test.yml index f5100f34e..1756d2986 100644 --- a/.github/workflows/job-compile-and-test.yml +++ b/.github/workflows/job-compile-and-test.yml @@ -43,7 +43,8 @@ jobs: working-directory: Extension # These tests don't require the binary. - # On Linux, it is failing with: Test run terminated with signal SIGSEGV + # On Linux, it is failing (before the tests actually run) with: Test run terminated with signal SIGSEGV. + # But it works on Linux during the E2E test. - name: Run SingleRootProject tests if: ${{ inputs.platform != 'linux' }} run: yarn test --scenario=SingleRootProject --skipCheckBinaries