Merge branch 'remove-typescript-integration-tests' into 'main'

Convert typescript integration tests to unit tests

See merge request gitlab-org/gitlab-vscode-extension!192

docs/writing-tests.md

@@ -4,7 +4,10 @@ This document provides technical details about our automated tests. Please see [
 
 ## Technology choice
 
-We are using [Jest](https://jestjs.io/) for our unit tests[^1]. For integration tests, we use [`mocha`](https://mochajs.org/) as a test runner, [`assert`](https://nodejs.org/docs/latest-v12.x/api/assert.html) for assertions, and [`vscode-test`](https://code.visualstudio.com/api/working-with-extensions/testing-extension#the-test-script) to run integration tests in VS Code instance.
+- **Unit Tests**: TypeScript and [Jest](https://jestjs.io/)[^1]
+- **Integration tests**: JavaScript and [`mocha`](https://mochajs.org/) as a test runner, [`assert`](https://nodejs.org/docs/latest-v12.x/api/assert.html) for assertions, and [`vscode-test`](https://code.visualstudio.com/api/working-with-extensions/testing-extension#the-test-script) to run integration tests in VS Code instance
+
+*We choose JavaScript for integration tests because `@types/jest` and `@types/mocha` are not compatible and often cause conflicts. The integration tests are written against much more stable VS Code Extension API and so some of the TS benefits are not as pronounced.*
 
 ## Unit tests `npm run test-unit`
 

src/__mocks__/vscode.js

+const { Uri } = require('../test_utils/uri');
+
 module.exports = {
   TreeItem: function TreeItem(label, collapsibleState) {
     return { label, collapsibleState };
@@ -9,16 +11,7 @@ module.exports = {
   TreeItemCollapsibleState: {
     Collapsed: 'collapsed',
   },
-  Uri: {
-    file: path => ({
-      path,
-      with: args => ({
-        path,
-        ...args,
-      }),
-    }),
-    parse: str => str,
-  },
+  Uri,
   comments: {
     createCommentController: jest.fn(),
   },
@@ -34,6 +27,9 @@ module.exports = {
   workspace: {
     getConfiguration: jest.fn().mockReturnValue({}),
   },
+  extensions: {
+    getExtension: jest.fn(),
+  },
   CommentMode: { Preview: 1 },
   StatusBarAlignment: { Left: 0 },
   CommentThreadCollapsibleState: { Collapsed: 0, Expanded: 1 },

src/git/git_extension_wrapper.test.ts

+import { GitExtension } from '../api/git';
+import { GitExtensionWrapper } from './git_extension_wrapper';
+import { GitLabRemoteSourceProviderRepository } from '../gitlab/clone/gitlab_remote_source_provider_repository';
+import { gitlabCredentialsProvider } from '../gitlab/clone/gitlab_credentials_provider';
+import { FakeGitExtension } from '../test_utils/fake_git_extension';
+
+jest.mock('../gitlab/clone/gitlab_credentials_provider');
+jest.mock('../gitlab/clone/gitlab_remote_source_provider_repository');
+
+describe('GitExtensionWrapper', () => {
+  let fakeExtension: FakeGitExtension;
+
+  beforeEach(async () => {
+    fakeExtension = new FakeGitExtension();
+  });
+
+  it('creates a new GitLabRemoteSourceProviderRepository', async () => {
+    // TODO: maybe introduce something like an initialize method instead of doing the work in constructor
+    // eslint-disable-next-line no-new
+    new GitExtensionWrapper((fakeExtension as unknown) as GitExtension);
+
+    expect(GitLabRemoteSourceProviderRepository).toHaveBeenCalledWith(fakeExtension.gitApi);
+  });
+
+  it('adds credentials provider to the Git Extension', async () => {
+    // TODO: maybe introduce something like an initialize method instead of doing the work in constructor
+    // eslint-disable-next-line no-new
+    new GitExtensionWrapper((fakeExtension as unknown) as GitExtension);
+
+    expect(fakeExtension.gitApi.credentialsProviders).toEqual([gitlabCredentialsProvider]);
+  });
+});

src/gitlab/clone/gitlab_credentials_provider.test.ts

+import * as vscode from 'vscode';
+import { tokenService } from '../../services/token_service';
+import { GITLAB_URL } from '../../../test/integration/test_infrastructure/constants';
+import { gitlabCredentialsProvider } from './gitlab_credentials_provider';
+
+jest.mock('../../services/token_service');
+
+describe('GitLab Credentials Provider', () => {
+  beforeEach(() => {
+    tokenService.getInstanceUrls = () => [GITLAB_URL];
+    tokenService.getToken = (url: string) => (url === GITLAB_URL ? 'password' : undefined);
+  });
+
+  it('getting credentials works', async () => {
+    expect(
+      (await gitlabCredentialsProvider.getCredentials(vscode.Uri.parse(GITLAB_URL)))?.password,
+    ).toBe('password');
+  });
+
+  it('returns undefined for url without token', async () => {
+    expect(
+      await gitlabCredentialsProvider.getCredentials(vscode.Uri.parse('https://invalid.com')),
+    ).toBe(undefined);
+  });
+});

src/gitlab/clone/gitlab_remote_source_provider_repository.test.ts

+import { tokenService } from '../../services/token_service';
+import { API } from '../../api/git';
+import { GitLabRemoteSourceProviderRepository } from './gitlab_remote_source_provider_repository';
+import { FakeGitExtension } from '../../test_utils/fake_git_extension';
+
+jest.mock('../../services/token_service');
+
+describe('GitLabRemoteSourceProviderRepository', () => {
+  let fakeExtension: FakeGitExtension;
+  let tokenChangeListener: () => unknown;
+
+  beforeEach(async () => {
+    fakeExtension = new FakeGitExtension();
+    (tokenService as any).onDidChange = (listener: () => unknown, bindThis: unknown) => {
+      tokenChangeListener = () => listener.call(bindThis);
+    };
+  });
+
+  it('remote source provider created for new token', async () => {
+    tokenService.getInstanceUrls = () => ['https://test2.gitlab.com'];
+    // TODO: maybe introduce something like an initialize method instead of doing the work in constructor
+    // eslint-disable-next-line no-new
+    new GitLabRemoteSourceProviderRepository((fakeExtension.gitApi as unknown) as API);
+
+    expect(fakeExtension.gitApi.remoteSourceProviders.length).toBe(1);
+
+    tokenService.getInstanceUrls = () => ['https://test2.gitlab.com', 'https://test3.gitlab.com'];
+
+    tokenChangeListener();
+
+    expect(fakeExtension.gitApi.remoteSourceProviders.length).toBe(2);
+  });
+
+  it('remote source providers disposed after token removal', async () => {
+    tokenService.getInstanceUrls = () => ['https://test2.gitlab.com', 'https://test3.gitlab.com'];
+    // TODO: maybe introduce something like an initialize method instead of doing the work in constructor
+    // eslint-disable-next-line no-new
+    new GitLabRemoteSourceProviderRepository((fakeExtension.gitApi as unknown) as API);
+
+    expect(fakeExtension.gitApi.remoteSourceProviders.length).toBe(2);
+
+    tokenService.getInstanceUrls = () => ['https://test2.gitlab.com'];
+
+    tokenChangeListener();
+
+    expect(fakeExtension.gitApi.remoteSourceProviders.length).toBe(1);
+  });
+});

src/review/git_content_provider.test.ts

@@ -2,7 +2,7 @@ import { mocked } from 'ts-jest/utils';
 import { GitContentProvider } from './git_content_provider';
 import { GitService } from '../git_service';
 import { ApiContentProvider } from './api_content_provider';
-import { createReviewUri } from '../test_utils/entities';
+import { toReviewUri } from './review_uri';
 
 jest.mock('../git_service');
 jest.mock('./api_content_provider');
@@ -10,6 +10,13 @@ jest.mock('./api_content_provider');
 describe('GitContentProvider', () => {
   const gitContentProvider = new GitContentProvider();
 
+  const reviewUriParams = {
+    commit: 'abcdef',
+    path: '/review',
+    projectId: 1234,
+    workspacePath: 'path/to/workspace',
+  };
+
   let getFileContent: jest.Mock;
 
   beforeEach(() => {
@@ -23,7 +30,7 @@ describe('GitContentProvider', () => {
     getFileContent.mockReturnValue('Test text');
 
     const result = await gitContentProvider.provideTextDocumentContent(
-      createReviewUri(),
+      toReviewUri(reviewUriParams),
       null as any,
     );
     expect(result).toBe('Test text');
@@ -37,7 +44,7 @@ describe('GitContentProvider', () => {
     mocked(ApiContentProvider).mockReturnValue(apiContentProvider);
 
     const result = await gitContentProvider.provideTextDocumentContent(
-      createReviewUri(),
+      toReviewUri(reviewUriParams),
       null as any,
     );
     expect(result).toBe('Api content');

src/review/review_uri.test.ts

-import * as vscode from 'vscode';
-import { createReviewUri } from '../test_utils/entities';
 import { fromReviewUri, toReviewUri } from './review_uri';
 
 describe('review_uri.ts', () => {
-  const params = {
+  const reviewUriParams = {
     commit: 'abcdef',
     path: '/review',
     projectId: 1234,
     workspacePath: 'path/to/workspace',
   };
 
-  const uri = createReviewUri(params);
-
   describe('toReviewUri', () => {
     it('returns the correct Uri', () => {
-      const result = toReviewUri(params);
+      const result = toReviewUri(reviewUriParams);
 
-      expect(result).toEqual(uri);
+      expect(result.toString()).toEqual(
+        'gl-review:///review{"commit":"abcdef","workspacePath":"path/to/workspace","projectId":1234}#',
+      );
     });
   });
 
   describe('fromReviewUri', () => {
     it('returns the correct string', () => {
-      const result = fromReviewUri(uri as vscode.Uri);
+      const result = fromReviewUri(toReviewUri(reviewUriParams));
 
-      expect(result).toEqual(params);
+      expect(result).toEqual(reviewUriParams);
     });
   });
 });

src/test_utils/entities.ts

-import * as vscode from 'vscode';
 import { CustomQueryType } from '../gitlab/custom_query_type';
 
 export const issue: RestIssuable = {
@@ -61,19 +60,6 @@ export const customQuery = {
   noItemText: 'No item',
 };
 
-export const createReviewUri = ({
-  path = 'testFile.txt',
-  commit = '12345abcde',
-  workspacePath = `/path/to/workspace`,
-  projectId = 123456,
-} = {}): vscode.Uri => {
-  return {
-    path,
-    query: JSON.stringify({ commit, workspacePath, projectId }),
-    scheme: 'gl-review',
-  } as vscode.Uri;
-};
-
 export const pipeline: RestPipeline = {
   status: 'success',
   updated_at: '2021-02-12T12:06:17Z',

src/test_utils/fake_git_extension.ts

+/* eslint-disable max-classes-per-file, @typescript-eslint/no-explicit-any */
+
+const removeFromArray = (array: any[], element: any): any[] => {
+  return array.filter(el => el !== element);
+};
+
+/**
+ * This is a simple test double for the native Git extension API
+ *
+ * It allows us to test our cloning feature without mocking every response
+ * and validating arguments of function calls.
+ */
+class FakeGitApi {
+  public credentialsProviders: any[] = [];
+
+  public remoteSourceProviders: any[] = [];
+
+  registerCredentialsProvider(provider: any) {
+    this.credentialsProviders.push(provider);
+    return {
+      dispose: () => {
+        this.credentialsProviders = removeFromArray(this.credentialsProviders, provider);
+      },
+    };
+  }
+
+  registerRemoteSourceProvider(provider: any) {
+    this.remoteSourceProviders.push(provider);
+    return {
+      dispose: () => {
+        this.remoteSourceProviders = removeFromArray(this.remoteSourceProviders, provider);
+      },
+    };
+  }
+}
+
+/**
+ * This is a simple test double for the native Git extension
+ *
+ * We use it to test enabling and disabling the extension.
+ */
+export class FakeGitExtension {
+  public enabled = true;
+
+  public enablementListeners: (() => any)[] = [];
+
+  public gitApi = new FakeGitApi();
+
+  onDidChangeEnablement(listener: () => any) {
+    this.enablementListeners.push(listener);
+    return {
+      dispose: () => {
+        /* */
+      },
+    };
+  }
+
+  getAPI() {
+    return this.gitApi;
+  }
+}

src/test_utils/uri.ts

+import * as vscode from 'vscode';
+
+interface UriOptions {
+  scheme: string;
+  authority: string;
+  path: string;
+  query: string;
+  fragment: string;
+}
+
+/**
+ * This is a test double for unit-testing vscode.Uri related logic.
+ * `vscode` module gets injected into the runtime only in integration tests so
+ * Jest tests don't have access to the real implementation.
+ *
+ * This double approximates the vscode.Uri behavior closely enough, that
+ * we can use it in tests. But the logic is not identical.
+ */
+export class Uri implements vscode.Uri {
+  scheme: string;
+
+  authority: string;
+
+  path: string;
+
+  query: string;
+
+  fragment: string;
+
+  get fsPath(): string {
+    return this.path;
+  }
+
+  constructor(options: UriOptions) {
+    this.scheme = options.scheme;
+    this.authority = options.authority;
+    this.path = options.path;
+    this.query = options.query;
+    this.fragment = options.fragment;
+  }
+
+  with(change: Partial<UriOptions>): vscode.Uri {
+    return new Uri({
+      scheme: change.scheme ?? this.scheme,
+      authority: change.authority ?? this.authority,
+      path: change.path ?? this.path,
+      query: change.query ?? this.query,
+      fragment: change.fragment ?? this.fragment,
+    });
+  }
+
+  toString(skipEncoding?: boolean): string {
+    return `${this.scheme}://${this.authority}${this.path}${this.query}#${this.fragment}`;
+  }
+
+  toJSON() {
+    return JSON.stringify(this);
+  }
+
+  static parse(stringUri: string) {
+    const url = new URL(stringUri);
+    const [query, fragment] = url.search.split('#');
+    return new Uri({
+      scheme: url.protocol,
+      authority: url.hostname,
+      path: url.pathname,
+      query,
+      fragment,
+    });
+  }
+
+  static file(filePath: string) {
+    return new Uri({
+      scheme: 'file://',
+      authority: '',
+      path: filePath,
+      query: '',
+      fragment: '',
+    });
+  }
+}

test/integration/git_extension_wrapper.test.ts

-import * as assert from 'assert';
-import * as sinon from 'sinon';
-import * as vscode from 'vscode';
-import { tokenService } from '../../src/services/token_service';
-import { GITLAB_URL } from './test_infrastructure/constants';
-import { GitExtension } from '../../src/api/git';
-import { GitExtensionWrapper } from '../../src/git/git_extension_wrapper';
-
-const token = 'abcd-secret';
-
-describe('GitExtensionWrapper', () => {
-  const sandbox = sinon.createSandbox();
-
-  before(async () => {
-    await tokenService.setToken(GITLAB_URL, token);
-  });
-
-  afterEach(async () => {
-    sandbox.restore();
-  });
-
-  after(async () => {
-    await tokenService.setToken(GITLAB_URL, undefined);
-  });
-
-  it('remote source provider created', async () => {
-    const gitExtension = vscode.extensions.getExtension<GitExtension>('vscode.git')?.exports;
-    if (gitExtension) {
-      const gitAPI = gitExtension.getAPI(1);
-
-      const fakeDispose = sinon.fake();
-      const fakeRegisterRemoteSourceProvider = sinon.fake(() => ({ dispose: fakeDispose }));
-
-      sandbox.replace(gitAPI, 'registerRemoteSourceProvider', fakeRegisterRemoteSourceProvider);
-      const getApi = () => gitAPI;
-      sandbox.replace(gitExtension, 'getAPI', getApi);
-
-      const gitExtensionWrapper = new GitExtensionWrapper(gitExtension);
-
-      assert.strictEqual(
-        fakeRegisterRemoteSourceProvider.callCount,
-        1,
-        'One remote source provider should have been created',
-      );
-
-      gitExtensionWrapper.dispose();
-    } else {
-      fail('Git API not available');
-    }
-  });
-
-  it('remote source provider created for new token', async () => {
-    const gitExtension = vscode.extensions.getExtension<GitExtension>('vscode.git')?.exports;
-    if (gitExtension) {
-      const gitAPI = gitExtension.getAPI(1);
-
-      const fakeDispose = sinon.fake();
-      const fakeRegisterRemoteSourceProvider = sinon.fake(() => ({ dispose: fakeDispose }));
-
-      sandbox.replace(gitAPI, 'registerRemoteSourceProvider', fakeRegisterRemoteSourceProvider);
-      const getApi = () => gitAPI;
-      sandbox.replace(gitExtension, 'getAPI', getApi);
-
-      const gitExtensionWrapper = new GitExtensionWrapper(gitExtension);
-
-      await tokenService.setToken('https://test2.gitlab.com', 'abcde');
-
-      assert.strictEqual(
-        fakeRegisterRemoteSourceProvider.callCount,
-        2,
-        'After a newly added token, two remote source providers should have been created',
-      );
-
-      await tokenService.setToken('https://test2.gitlab.com', undefined);
-      gitExtensionWrapper.dispose();
-    } else {
-      fail('Git API not available');
-    }
-  });
-
-  it('remote source providers disposed after token removal', async () => {
-    const gitExtension = vscode.extensions.getExtension<GitExtension>('vscode.git')?.exports;
-    if (gitExtension) {
-      const gitAPI = gitExtension.getAPI(1);
-
-      const fakeDispose = sinon.fake();
-      const fakeRegisterRemoteSourceProvider = sinon.fake(() => ({ dispose: fakeDispose }));
-
-      sandbox.replace(gitAPI, 'registerRemoteSourceProvider', fakeRegisterRemoteSourceProvider);
-      const getApi = () => gitAPI;
-      sandbox.replace(gitExtension, 'getAPI', getApi);
-
-      const gitExtensionWrapper = new GitExtensionWrapper(gitExtension);
-
-      await tokenService.setToken('https://test2.gitlab.com', 'abcde');
-      await tokenService.setToken('https://test2.gitlab.com', undefined);
-
-      assert.strictEqual(
-        fakeDispose.callCount,
-        1,
-        'After removing a token, the remote source provider should be disposed',
-      );
-
-      gitExtensionWrapper.dispose();
-    } else {
-      fail('Git API not available');
-    }
-  });
-
-  it('credentials provider is created', async () => {
-    const gitExtension = vscode.extensions.getExtension<GitExtension>('vscode.git')?.exports;
-    if (gitExtension) {
-      const gitAPI = gitExtension.getAPI(1);
-
-      const apiMock = sinon.mock(gitAPI);
-      apiMock.expects('registerCredentialsProvider').once();
-      const getApi = () => gitAPI;
-
-      sandbox.replace(gitExtension, 'getAPI', getApi);
-
-      const gitlabProviderManager = new GitExtensionWrapper(gitExtension);
-
-      apiMock.verify();
-      apiMock.restore();
-
-      gitlabProviderManager.dispose();
-    } else {
-      fail('Git API not available');
-    }
-  });
-});

test/integration/gitlab_credentials_provider.test.ts

-import * as assert from 'assert';
-import * as vscode from 'vscode';
-import { tokenService } from '../../src/services/token_service';
-import { GITLAB_URL } from './test_infrastructure/constants';
-import { gitlabCredentialsProvider } from '../../src/gitlab/clone/gitlab_credentials_provider';
-
-const token = 'abcd-secret';
-
-describe('GitLab Credentials Provider', () => {
-  before(async () => {
-    await tokenService.setToken(GITLAB_URL, token);
-  });
-
-  after(async () => {
-    await tokenService.setToken(GITLAB_URL, undefined);
-  });
-
-  it('getting credentials works', async () => {
-    assert.deepStrictEqual(
-      (await gitlabCredentialsProvider.getCredentials(vscode.Uri.parse(GITLAB_URL)))?.password,
-      token,
-      'Username and token should be equal',
-    );
-  });
-
-  it('returns undefined for url without token', async () => {
-    assert.deepStrictEqual(
-      await gitlabCredentialsProvider.getCredentials(vscode.Uri.parse('https://invalid.com')),
-      undefined,
-      'there should be no user at invalid url',
-    );
-  });
-
-  it('newly created token is used', async () => {
-    const temporaryToken = 'token';
-    await tokenService.setToken('https://test2.gitlab.com', temporaryToken);
-
-    assert.deepStrictEqual(
-      (await gitlabCredentialsProvider.getCredentials(vscode.Uri.parse('https://test2.gitlab.com')))
-        ?.password,
-      temporaryToken,
-      'Username and token should be equal',
-    );
-
-    await tokenService.setToken('https://test2.gitlab.com', undefined);
-  });
-});

test/integration/gitlab_remote_source_provider.test.ts → test/integration/gitlab_remote_source_provider.test.js

-import * as assert from 'assert';
-import { graphql } from 'msw';
-import { tokenService } from '../../src/services/token_service';
-import * as projectsResponse from './fixtures/graphql/projects.json';
-import * as remoteSourceResult from './fixtures/git_api/remote_sources.json';
-import { getServer } from './test_infrastructure/mock_server.js';
-import { GITLAB_URL } from './test_infrastructure/constants';
-import { GitLabRemoteSourceProvider } from '../../src/gitlab/clone/gitlab_remote_source_provider';
+const assert = require('assert');
+const { graphql } = require('msw');
+const { tokenService } = require('../../src/services/token_service');
+const projectsResponse = require('./fixtures/graphql/projects.json');
+const remoteSourceResult = require('./fixtures/git_api/remote_sources.json');
+const { getServer } = require('./test_infrastructure/mock_server.js');
+const { GITLAB_URL } = require('./test_infrastructure/constants');
+const {
+  GitLabRemoteSourceProvider,
+} = require('../../src/gitlab/clone/gitlab_remote_source_provider');
 
 const token = 'abcd-secret';
 
 describe('GitLab Remote Source provider', () => {
-  let server: {
-    close(): void;
-  };
+  let server;
 
   before(async () => {
     server = getServer([