Skip to content

Commit 0c2b35f

Browse files
committed
Add unit tests for add/remove database
In order to do this, needed to move `databases.test.ts` to the `minimal-workspace` test folder because these tests require that there be some kind of workspace open in order to check on workspace folders. Unfortunately, during tests vscode does not allow you to convert from a single root workspace to multi-root and so several of the workspace functions needed to be stubbed out.
1 parent 9fca57b commit 0c2b35f

File tree

5 files changed

+341
-205
lines changed

5 files changed

+341
-205
lines changed

extensions/ql-vscode/src/databases.ts

+16-7
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export interface DatabaseOptions {
3838
dateAdded?: number | undefined;
3939
}
4040

41-
interface FullDatabaseOptions extends DatabaseOptions {
41+
export interface FullDatabaseOptions extends DatabaseOptions {
4242
ignoreSourceArchive: boolean;
4343
dateAdded: number | undefined;
4444
}
@@ -674,16 +674,19 @@ export class DatabaseManager extends DisposableObject {
674674
}
675675

676676
public removeDatabaseItem(item: DatabaseItem) {
677-
if (this._currentDatabaseItem == item)
677+
if (this._currentDatabaseItem == item) {
678678
this._currentDatabaseItem = undefined;
679+
}
679680
const index = this.databaseItems.findIndex(searchItem => searchItem === item);
680681
if (index >= 0) {
681682
this._databaseItems.splice(index, 1);
682683
}
683684
this.updatePersistedDatabaseList();
684685

685686
// Delete folder from workspace, if it is still there
686-
const folderIndex = (vscode.workspace.workspaceFolders || []).findIndex(folder => item.belongsToSourceArchiveExplorerUri(folder.uri));
687+
const folderIndex = (vscode.workspace.workspaceFolders || []).findIndex(
688+
folder => item.belongsToSourceArchiveExplorerUri(folder.uri)
689+
);
687690
if (folderIndex >= 0) {
688691
logger.log(`Removing workspace folder at index ${folderIndex}`);
689692
vscode.workspace.updateWorkspaceFolders(folderIndex, 1);
@@ -692,9 +695,9 @@ export class DatabaseManager extends DisposableObject {
692695
// Delete folder from file system only if it is controlled by the extension
693696
if (this.isExtensionControlledLocation(item.databaseUri)) {
694697
logger.log('Deleting database from filesystem.');
695-
fs.remove(item.databaseUri.path).then(
696-
() => logger.log(`Deleted '${item.databaseUri.path}'`),
697-
e => logger.log(`Failed to delete '${item.databaseUri.path}'. Reason: ${e.message}`));
698+
fs.remove(item.databaseUri.fsPath).then(
699+
() => logger.log(`Deleted '${item.databaseUri.fsPath}'`),
700+
e => logger.log(`Failed to delete '${item.databaseUri.fsPath}'. Reason: ${e.message}`));
698701
}
699702

700703
// note that we use undefined as the item in order to reset the entire tree
@@ -715,7 +718,13 @@ export class DatabaseManager extends DisposableObject {
715718

716719
private isExtensionControlledLocation(uri: vscode.Uri) {
717720
const storagePath = this.ctx.storagePath || this.ctx.globalStoragePath;
718-
return uri.path.startsWith(storagePath);
721+
// the uri.fsPath function on windows returns a lowercase drive letter,
722+
// but storagePath will have an uppercase drive letter. Be sure to compare
723+
// URIs to URIs only
724+
if (storagePath) {
725+
return uri.fsPath.startsWith(vscode.Uri.file(storagePath).fsPath);
726+
}
727+
return false;
719728
}
720729
}
721730

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
import 'vscode-test';
2+
import 'mocha';
3+
import * as sinon from 'sinon';
4+
import * as tmp from 'tmp';
5+
import * as fs from 'fs-extra';
6+
import * as path from 'path';
7+
import { expect } from 'chai';
8+
import { ExtensionContext, Uri, workspace } from 'vscode';
9+
10+
import {
11+
DatabaseEventKind,
12+
DatabaseManager,
13+
DatabaseItemImpl,
14+
DatabaseContents,
15+
isLikelyDbLanguageFolder,
16+
FullDatabaseOptions
17+
} from '../../databases';
18+
import { QueryServerConfig } from '../../config';
19+
import { Logger } from '../../logging';
20+
import { encodeArchiveBasePath, encodeSourceArchiveUri } from '../../archive-filesystem-provider';
21+
22+
describe('databases', () => {
23+
24+
const MOCK_DB_OPTIONS: FullDatabaseOptions = {
25+
dateAdded: 123,
26+
ignoreSourceArchive: false
27+
};
28+
29+
let databaseManager: DatabaseManager;
30+
let updateSpy: sinon.SinonSpy;
31+
let getSpy: sinon.SinonStub;
32+
let dbChangedHandler: sinon.SinonSpy;
33+
34+
let sandbox: sinon.SinonSandbox;
35+
let dir: tmp.DirResult;
36+
37+
38+
39+
beforeEach(() => {
40+
dir = tmp.dirSync();
41+
42+
sandbox = sinon.createSandbox();
43+
updateSpy = sandbox.spy();
44+
getSpy = sandbox.stub();
45+
dbChangedHandler = sandbox.spy();
46+
databaseManager = new DatabaseManager(
47+
{
48+
workspaceState: {
49+
update: updateSpy,
50+
get: getSpy
51+
},
52+
// pretend like databases added in the temp dir are controlled by the extension
53+
// so that they are deleted upon removal
54+
storagePath: dir.name
55+
} as unknown as ExtensionContext,
56+
{} as QueryServerConfig,
57+
{} as Logger,
58+
);
59+
60+
// Unfortunately, during a test it is not possible to convert from
61+
// a single root workspace to multi-root, so must stub out relevant
62+
// functions
63+
sandbox.stub(workspace, 'updateWorkspaceFolders');
64+
sandbox.spy(workspace, 'onDidChangeWorkspaceFolders');
65+
});
66+
67+
afterEach(async () => {
68+
dir.removeCallback();
69+
sandbox.restore();
70+
});
71+
72+
it('should fire events when adding and removing a db item', () => {
73+
const mockDbItem = createMockDB();
74+
const spy = sinon.spy();
75+
databaseManager.onDidChangeDatabaseItem(spy);
76+
(databaseManager as any).addDatabaseItem(mockDbItem);
77+
78+
expect((databaseManager as any)._databaseItems).to.deep.eq([mockDbItem]);
79+
expect(updateSpy).to.have.been.calledWith('databaseList', [{
80+
options: MOCK_DB_OPTIONS,
81+
uri: dbLocationUri().toString(true)
82+
}]);
83+
expect(spy).to.have.been.calledWith({
84+
item: undefined,
85+
kind: DatabaseEventKind.Add
86+
});
87+
88+
sinon.reset();
89+
90+
// now remove the item
91+
databaseManager.removeDatabaseItem(mockDbItem);
92+
expect((databaseManager as any)._databaseItems).to.deep.eq([]);
93+
expect(updateSpy).to.have.been.calledWith('databaseList', []);
94+
expect(spy).to.have.been.calledWith({
95+
item: undefined,
96+
kind: DatabaseEventKind.Remove
97+
});
98+
});
99+
100+
it('should rename a db item and emit an event', () => {
101+
const mockDbItem = createMockDB();
102+
const spy = sinon.spy();
103+
databaseManager.onDidChangeDatabaseItem(spy);
104+
(databaseManager as any).addDatabaseItem(mockDbItem);
105+
sinon.restore();
106+
107+
databaseManager.renameDatabaseItem(mockDbItem, 'new name');
108+
109+
expect(mockDbItem.name).to.eq('new name');
110+
expect(updateSpy).to.have.been.calledWith('databaseList', [{
111+
options: { ...MOCK_DB_OPTIONS, displayName: 'new name' },
112+
uri: dbLocationUri().toString(true)
113+
}]);
114+
115+
expect(spy).to.have.been.calledWith({
116+
item: undefined,
117+
kind: DatabaseEventKind.Rename
118+
});
119+
});
120+
121+
describe('add / remove database items', () => {
122+
it('should add a database item', async () => {
123+
const spy = sandbox.spy();
124+
databaseManager.onDidChangeDatabaseItem(spy);
125+
const mockDbItem = createMockDB();
126+
127+
await (databaseManager as any).addDatabaseItem(mockDbItem);
128+
129+
expect(databaseManager.databaseItems).to.deep.eq([mockDbItem]);
130+
expect(updateSpy).to.have.been.calledWith('databaseList', [{
131+
uri: dbLocationUri().toString(true),
132+
options: MOCK_DB_OPTIONS
133+
}]);
134+
135+
const mockEvent = {
136+
item: undefined,
137+
kind: DatabaseEventKind.Add
138+
};
139+
expect(spy).to.have.been.calledWith(mockEvent);
140+
});
141+
142+
it('should add a database item source archive', async function() {
143+
const mockDbItem = createMockDB();
144+
mockDbItem.name = 'xxx';
145+
await (databaseManager as any).addDatabaseSourceArchiveFolder(mockDbItem);
146+
147+
// workspace folders should be updated. We can only check the mocks since
148+
// when running as a test, we are not allowed to update the workspace folders
149+
expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(1, 0, {
150+
name: '[xxx source archive]',
151+
// must use a matcher here since vscode URIs with the same path
152+
// are not always equal due to internal state.
153+
uri: sinon.match.has('fsPath', encodeArchiveBasePath(sourceLocationUri().fsPath).fsPath)
154+
});
155+
});
156+
157+
it('should remove a database item', async () => {
158+
const mockDbItem = createMockDB();
159+
sandbox.stub(fs, 'remove').resolves();
160+
161+
// pretend that this item is the first workspace folder in the list
162+
sandbox.stub(mockDbItem, 'belongsToSourceArchiveExplorerUri').returns(true);
163+
164+
await (databaseManager as any).addDatabaseItem(mockDbItem);
165+
updateSpy.resetHistory();
166+
167+
await databaseManager.removeDatabaseItem(mockDbItem);
168+
169+
expect(databaseManager.databaseItems).to.deep.eq([]);
170+
expect(updateSpy).to.have.been.calledWith('databaseList', []);
171+
// should remove the folder
172+
expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(0, 1);
173+
174+
// should also delete the db contents
175+
expect(fs.remove).to.have.been.calledWith(mockDbItem.databaseUri.fsPath);
176+
});
177+
178+
it('should remove a database item outside of the extension controlled area', async () => {
179+
const mockDbItem = createMockDB();
180+
sandbox.stub(fs, 'remove').resolves();
181+
182+
// pretend that this item is the first workspace folder in the list
183+
sandbox.stub(mockDbItem, 'belongsToSourceArchiveExplorerUri').returns(true);
184+
185+
await (databaseManager as any).addDatabaseItem(mockDbItem);
186+
updateSpy.resetHistory();
187+
188+
// pretend that the database location is not controlled by the extension
189+
(databaseManager as any).ctx.storagePath = 'hucairz';
190+
191+
await databaseManager.removeDatabaseItem(mockDbItem);
192+
193+
expect(databaseManager.databaseItems).to.deep.eq([]);
194+
expect(updateSpy).to.have.been.calledWith('databaseList', []);
195+
// should remove the folder
196+
expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(0, 1);
197+
198+
// should NOT delete the db contents
199+
expect(fs.remove).not.to.have.been.called;
200+
});
201+
});
202+
203+
describe('resolveSourceFile', () => {
204+
it('should fail to resolve when not a uri', () => {
205+
const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
206+
(db as any)._contents.sourceArchiveUri = undefined;
207+
expect(() => db.resolveSourceFile('abc')).to.throw('Scheme is missing');
208+
});
209+
210+
it('should fail to resolve when not a file uri', () => {
211+
const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
212+
(db as any)._contents.sourceArchiveUri = undefined;
213+
expect(() => db.resolveSourceFile('http://abc')).to.throw('Invalid uri scheme');
214+
});
215+
216+
describe('no source archive', () => {
217+
it('should resolve undefined', () => {
218+
const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
219+
(db as any)._contents.sourceArchiveUri = undefined;
220+
const resolved = db.resolveSourceFile(undefined);
221+
expect(resolved.toString(true)).to.eq(dbLocationUri().toString(true));
222+
});
223+
224+
it('should resolve an empty file', () => {
225+
const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
226+
(db as any)._contents.sourceArchiveUri = undefined;
227+
const resolved = db.resolveSourceFile('file:');
228+
expect(resolved.toString()).to.eq('file:///');
229+
});
230+
});
231+
232+
describe('zipped source archive', () => {
233+
it('should encode a source archive url', () => {
234+
const db = createMockDB(encodeSourceArchiveUri({
235+
sourceArchiveZipPath: 'sourceArchive-uri',
236+
pathWithinSourceArchive: 'def'
237+
}));
238+
const resolved = db.resolveSourceFile(Uri.file('abc').toString());
239+
240+
// must recreate an encoded archive uri instead of typing out the
241+
// text since the uris will be different on windows and ubuntu.
242+
expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
243+
sourceArchiveZipPath: 'sourceArchive-uri',
244+
pathWithinSourceArchive: 'def/abc'
245+
}).toString());
246+
});
247+
248+
it('should encode a source archive url with trailing slash', () => {
249+
const db = createMockDB(encodeSourceArchiveUri({
250+
sourceArchiveZipPath: 'sourceArchive-uri',
251+
pathWithinSourceArchive: 'def/'
252+
}));
253+
const resolved = db.resolveSourceFile(Uri.file('abc').toString());
254+
255+
// must recreate an encoded archive uri instead of typing out the
256+
// text since the uris will be different on windows and ubuntu.
257+
expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
258+
sourceArchiveZipPath: 'sourceArchive-uri',
259+
pathWithinSourceArchive: 'def/abc'
260+
}).toString());
261+
});
262+
263+
it('should encode an empty source archive url', () => {
264+
const db = createMockDB(encodeSourceArchiveUri({
265+
sourceArchiveZipPath: 'sourceArchive-uri',
266+
pathWithinSourceArchive: 'def'
267+
}));
268+
const resolved = db.resolveSourceFile('file:');
269+
expect(resolved.toString()).to.eq('codeql-zip-archive://1-18/sourceArchive-uri/def/');
270+
});
271+
});
272+
273+
it('should handle an empty file', () => {
274+
const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
275+
const resolved = db.resolveSourceFile('');
276+
expect(resolved.toString()).to.eq('file:///sourceArchive-uri/');
277+
});
278+
});
279+
280+
it('should find likely db language folders', () => {
281+
expect(isLikelyDbLanguageFolder('db-javascript')).to.be.true;
282+
expect(isLikelyDbLanguageFolder('dbnot-a-db')).to.be.false;
283+
});
284+
285+
function createMockDB(
286+
// source archive location must be a real(-ish) location since
287+
// tests will add this to the workspace location
288+
sourceArchiveUri = sourceLocationUri(),
289+
databaseUri = dbLocationUri()
290+
): DatabaseItemImpl {
291+
292+
return new DatabaseItemImpl(
293+
databaseUri,
294+
{
295+
sourceArchiveUri
296+
} as DatabaseContents,
297+
MOCK_DB_OPTIONS,
298+
dbChangedHandler
299+
);
300+
}
301+
302+
function sourceLocationUri() {
303+
return Uri.file(path.join(dir.name, 'src.zip'));
304+
}
305+
306+
function dbLocationUri() {
307+
return Uri.file(path.join(dir.name, 'db'));
308+
}
309+
});
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
import { runTestsInDirectory } from '../index-template';
2+
3+
import * as sinonChai from 'sinon-chai';
4+
import * as chai from 'chai';
5+
import * as chaiAsPromised from 'chai-as-promised';
6+
chai.use(chaiAsPromised);
7+
chai.use(sinonChai);
8+
9+
210
export function run(): Promise<void> {
311
return runTestsInDirectory(__dirname);
412
}

0 commit comments

Comments
 (0)