From f6ecbe6ed1dfab670bbd870bec0b77f62191501a Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 1 Oct 2021 12:24:43 -0700 Subject: [PATCH 1/5] Add progress messages to LGTM download option. --- extensions/ql-vscode/CHANGELOG.md | 3 +++ extensions/ql-vscode/src/databaseFetcher.ts | 21 +++++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 61c910aa25a..20685784858 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,6 +2,9 @@ ## [UNRELEASED] +- Remove line about selecting a language from the dropdown when downloading database from LGTM. This makes the download progress visible when the popup is not expanded. [#894](https://github.com/github/vscode-codeql/issues/894) +- Add progress messages to LGTM download option. This makes the two-step process (selecting a project, then selecting a language) more clear. + ## 1.5.5 - 08 September 2021 - Fix bug where a query is sometimes run before the file is saved. [#947](https://github.com/github/vscode-codeql/pull/947) diff --git a/extensions/ql-vscode/src/databaseFetcher.ts b/extensions/ql-vscode/src/databaseFetcher.ts index 301dd7ae9c9..1f8c275982a 100644 --- a/extensions/ql-vscode/src/databaseFetcher.ts +++ b/extensions/ql-vscode/src/databaseFetcher.ts @@ -72,6 +72,11 @@ export async function promptImportLgtmDatabase( progress: ProgressCallback, token: CancellationToken ): Promise { + progress({ + message: 'Choose project', + step: 1, + maxStep: 2 + }); const lgtmUrl = await window.showInputBox({ prompt: 'Enter the project slug or URL on LGTM (e.g., g/github/codeql or https://lgtm.com/projects/g/github/codeql)', @@ -81,7 +86,7 @@ export async function promptImportLgtmDatabase( } if (looksLikeLgtmUrl(lgtmUrl)) { - const databaseUrl = await convertToDatabaseUrl(lgtmUrl); + const databaseUrl = await convertToDatabaseUrl(lgtmUrl, progress); if (databaseUrl) { const item = await databaseArchiveFetcher( databaseUrl, @@ -405,7 +410,9 @@ function convertRawLgtmSlug(maybeSlug: string): string | undefined { } // exported for testing -export async function convertToDatabaseUrl(lgtmUrl: string) { +export async function convertToDatabaseUrl( + lgtmUrl: string, + progress: ProgressCallback) { try { lgtmUrl = convertRawLgtmSlug(lgtmUrl) || lgtmUrl; @@ -421,7 +428,7 @@ export async function convertToDatabaseUrl(lgtmUrl: string) { throw new Error(); } - const language = await promptForLanguage(projectJson); + const language = await promptForLanguage(projectJson, progress); if (!language) { return; } @@ -439,8 +446,14 @@ export async function convertToDatabaseUrl(lgtmUrl: string) { } async function promptForLanguage( - projectJson: any + projectJson: any, + progress: ProgressCallback ): Promise { + progress({ + message: 'Choose language', + step: 2, + maxStep: 2 + }); if (!projectJson?.languages?.length) { return; } From 5f10e48d0e27fa97b2e8407425fc1de507e83b60 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 1 Oct 2021 13:13:30 -0700 Subject: [PATCH 2/5] Add additional argument to get test passing again. --- .../no-workspace/databaseFetcher.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts index 5b5164d2766..d1b47afbe91 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts @@ -13,11 +13,13 @@ import { looksLikeLgtmUrl, findDirWithFile, } from '../../databaseFetcher'; +import { ProgressUpdate } from '../../commandRunner'; chai.use(chaiAsPromised); const expect = chai.expect; -describe('databaseFetcher', function() { +describe('databaseFetcher', function () { // These tests make API calls and may need extra time to complete. + const fakeProgress = (_: ProgressUpdate) => { /*do nothing*/ }; this.timeout(10000); describe('convertToDatabaseUrl', () => { @@ -35,7 +37,7 @@ describe('databaseFetcher', function() { it('should convert a project url to a database url', async () => { quickPickSpy.resolves('javascript'); const lgtmUrl = 'https://lgtm.com/projects/g/github/codeql'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/javascript' @@ -48,7 +50,7 @@ describe('databaseFetcher', function() { quickPickSpy.resolves('python'); const lgtmUrl = 'https://lgtm.com/projects/g/github/codeql/subpage/subpage2?query=xxx'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/python' @@ -59,7 +61,7 @@ describe('databaseFetcher', function() { quickPickSpy.resolves('python'); const lgtmUrl = 'g/github/codeql'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/python' @@ -69,7 +71,7 @@ describe('databaseFetcher', function() { it('should fail on a nonexistent project', async () => { quickPickSpy.resolves('javascript'); const lgtmUrl = 'https://lgtm.com/projects/g/github/hucairz'; - await expect(convertToDatabaseUrl(lgtmUrl)).to.rejectedWith(/Invalid LGTM URL/); + await expect(convertToDatabaseUrl(lgtmUrl, fakeProgress)).to.rejectedWith(/Invalid LGTM URL/); }); }); From d31b0ef0783e11fb1fba519288459962fd4f5827 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 1 Oct 2021 14:08:14 -0700 Subject: [PATCH 3/5] Make edits requested by @aeisenerg --- extensions/ql-vscode/CHANGELOG.md | 3 --- .../no-workspace/databaseFetcher.test.ts | 17 +++++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 3b22107bc41..beb7e1ec831 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,11 +2,8 @@ ## [UNRELEASED] -- Remove line about selecting a language from the dropdown when downloading database from LGTM. This makes the download progress visible when the popup is not expanded. [#894](https://github.com/github/vscode-codeql/issues/894) - Add progress messages to LGTM download option. This makes the two-step process (selecting a project, then selecting a language) more clear. - - Remove line about selecting a language from the dropdown when downloading database from LGTM. This makes the download progress visible when the popup is not expanded. [#894](https://github.com/github/vscode-codeql/issues/894) - - Fixed a bug where copying the version information fails when a CodeQL CLI cannot be found. [#958](https://github.com/github/vscode-codeql/pull/958) ## 1.5.5 - 08 September 2021 diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts index d1b47afbe91..5cc65199717 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts @@ -13,21 +13,23 @@ import { looksLikeLgtmUrl, findDirWithFile, } from '../../databaseFetcher'; -import { ProgressUpdate } from '../../commandRunner'; +import { ProgressCallback } from '../../commandRunner'; chai.use(chaiAsPromised); const expect = chai.expect; describe('databaseFetcher', function () { // These tests make API calls and may need extra time to complete. - const fakeProgress = (_: ProgressUpdate) => { /*do nothing*/ }; this.timeout(10000); describe('convertToDatabaseUrl', () => { let sandbox: sinon.SinonSandbox; let quickPickSpy: sinon.SinonStub; + let progressSpy: ProgressCallback; + beforeEach(() => { sandbox = sinon.createSandbox(); quickPickSpy = sandbox.stub(window, 'showQuickPick'); + progressSpy = sandbox.spy(); }); afterEach(() => { @@ -37,7 +39,7 @@ describe('databaseFetcher', function () { it('should convert a project url to a database url', async () => { quickPickSpy.resolves('javascript'); const lgtmUrl = 'https://lgtm.com/projects/g/github/codeql'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, progressSpy); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/javascript' @@ -50,28 +52,31 @@ describe('databaseFetcher', function () { quickPickSpy.resolves('python'); const lgtmUrl = 'https://lgtm.com/projects/g/github/codeql/subpage/subpage2?query=xxx'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, progressSpy); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/python' ); + expect(progressSpy).to.have.been.calledOnce; }); it('should convert a raw slug to a database url with extra path segments', async () => { quickPickSpy.resolves('python'); const lgtmUrl = 'g/github/codeql'; - const dbUrl = await convertToDatabaseUrl(lgtmUrl, fakeProgress); + const dbUrl = await convertToDatabaseUrl(lgtmUrl, progressSpy); expect(dbUrl).to.equal( 'https://lgtm.com/api/v1.0/snapshots/1506465042581/python' ); + expect(progressSpy).to.have.been.calledOnce; }); it('should fail on a nonexistent project', async () => { quickPickSpy.resolves('javascript'); const lgtmUrl = 'https://lgtm.com/projects/g/github/hucairz'; - await expect(convertToDatabaseUrl(lgtmUrl, fakeProgress)).to.rejectedWith(/Invalid LGTM URL/); + await expect(convertToDatabaseUrl(lgtmUrl, progressSpy)).to.rejectedWith(/Invalid LGTM URL/); + expect(progressSpy).to.have.been.calledOnce; }); }); From f9e9df650a5478015bfacb0d83bf478fa12cb216 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 1 Oct 2021 15:31:36 -0700 Subject: [PATCH 4/5] Fix assertion in test case --- .../src/vscode-tests/no-workspace/databaseFetcher.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts index 5cc65199717..f546ca88f7a 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts @@ -76,7 +76,7 @@ describe('databaseFetcher', function () { quickPickSpy.resolves('javascript'); const lgtmUrl = 'https://lgtm.com/projects/g/github/hucairz'; await expect(convertToDatabaseUrl(lgtmUrl, progressSpy)).to.rejectedWith(/Invalid LGTM URL/); - expect(progressSpy).to.have.been.calledOnce; + expect(progressSpy).to.have.callCount(0); }); }); From f2c86fc8887d9791baa048921c86260e1cdffc17 Mon Sep 17 00:00:00 2001 From: Marc Jaramillo Date: Mon, 4 Oct 2021 08:59:35 -0700 Subject: [PATCH 5/5] Update extensions/ql-vscode/CHANGELOG.md Co-authored-by: Shati Patel <42641846+shati-patel@users.noreply.github.com> --- extensions/ql-vscode/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index beb7e1ec831..f211dcc08e1 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,8 +2,8 @@ ## [UNRELEASED] -- Add progress messages to LGTM download option. This makes the two-step process (selecting a project, then selecting a language) more clear. -- Remove line about selecting a language from the dropdown when downloading database from LGTM. This makes the download progress visible when the popup is not expanded. [#894](https://github.com/github/vscode-codeql/issues/894) +- Add progress messages to LGTM download option. This makes the two-step process (selecting a project, then selecting a language) more clear. [#960](https://github.com/github/vscode-codeql/pull/960) +- Remove line about selecting a language from the dropdown when downloading database from LGTM. This makes the download progress visible when the popup is not expanded. [#957](https://github.com/github/vscode-codeql/pull/957) - Fixed a bug where copying the version information fails when a CodeQL CLI cannot be found. [#958](https://github.com/github/vscode-codeql/pull/958) ## 1.5.5 - 08 September 2021