Merge pull request #1 from github/main

Add progress messages to LGTM download option. (github#960)
marcnjaramillo · Oct 6, 2021 · 9f0a975 · 9f0a975
2 parents 39fdd0c + 21dda65
commit 9f0a975
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 12 deletions.
diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md
@@ -2,9 +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. [#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

diff --git 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<DatabaseItem | undefined> {
+  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<string | undefined> {
+  progress({
+    message: 'Choose language',
+    step: 2,
+    maxStep: 2
+  });
   if (!projectJson?.languages?.length) {
     return;
   }

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
@@ -13,19 +13,23 @@ import {
   looksLikeLgtmUrl,
   findDirWithFile,
 } from '../../databaseFetcher';
+import { ProgressCallback } 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.
   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(() => {
@@ -35,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);
+      const dbUrl = await convertToDatabaseUrl(lgtmUrl, progressSpy);
 
       expect(dbUrl).to.equal(
         'https://lgtm.com/api/v1.0/snapshots/1506465042581/javascript'
@@ -48,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);
+      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);
+      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)).to.rejectedWith(/Invalid LGTM URL/);
+      await expect(convertToDatabaseUrl(lgtmUrl, progressSpy)).to.rejectedWith(/Invalid LGTM URL/);
+      expect(progressSpy).to.have.callCount(0);
     });
   });