From dbb37ba02f00e3d47e624428ae86edcac7b0539b Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Thu, 16 Jul 2020 14:53:20 +0200 Subject: [PATCH 1/6] Add document count to document list in tree view --- package.json | 12 + src/explorer/collectionTreeItem.ts | 6 +- src/explorer/documentListTreeItem.ts | 81 +++++-- src/language/mongoDBService.ts | 6 - src/mdbExtensionController.ts | 12 +- .../explorer/documentListTreeItem.test.ts | 227 ++++++++++-------- .../suite/explorer/schemaTreeItem.test.ts | 5 + src/test/suite/extension.test.ts | 5 +- src/test/suite/index.ts | 4 +- src/test/suite/mdbExtensionController.test.ts | 42 ++++ src/test/suite/stubs.ts | 8 +- 11 files changed, 276 insertions(+), 132 deletions(-) diff --git a/package.json b/package.json index 5cf7a9f3f..98431ba9c 100644 --- a/package.json +++ b/package.json @@ -264,6 +264,10 @@ "command": "mdb.viewCollectionDocuments", "title": "View Documents" }, + { + "command": "mdb.refreshDocumentList", + "title": "Refresh" + }, { "command": "mdb.copyCollectionName", "title": "Copy Collection Name" @@ -423,6 +427,10 @@ "command": "mdb.viewCollectionDocuments", "when": "view == mongoDB && viewItem == documentListTreeItem" }, + { + "command": "mdb.refreshDocumentList", + "when": "view == mongoDB && viewItem == documentListTreeItem" + }, { "command": "mdb.refreshSchema", "when": "view == mongoDB && viewItem == schemaTreeItem" @@ -504,6 +512,10 @@ "command": "mdb.viewCollectionDocuments", "when": "false" }, + { + "command": "mdb.refreshDocumentList", + "when": "false" + }, { "command": "mdb.copyCollectionName", "when": "false" diff --git a/src/explorer/collectionTreeItem.ts b/src/explorer/collectionTreeItem.ts index c565e9d12..ee3e5cd77 100644 --- a/src/explorer/collectionTreeItem.ts +++ b/src/explorer/collectionTreeItem.ts @@ -81,7 +81,7 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, false, // Collapsed. MAX_DOCUMENTS_VISIBLE, - false, // No more documents to show. + null, // No document count yet. false, // Cache is not up to date. [] // Empty cache. ); @@ -158,7 +158,7 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, this._documentListChild.isExpanded, this._documentListChild.getMaxDocumentsToShow(), - this._documentListChild.hasMoreDocumentsToShow, + this._documentListChild.getDocumentCount(), this._documentListChild.cacheIsUpToDate, this._documentListChild.getChildrenCache() ); @@ -226,7 +226,7 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, false, // Collapsed. MAX_DOCUMENTS_VISIBLE, - false, // No more documents to show. + null, // No document count yet. false, // Cache is not up to date. [] // Empty cache. ); diff --git a/src/explorer/documentListTreeItem.ts b/src/explorer/documentListTreeItem.ts index 020c7aaf3..fde1e3a53 100644 --- a/src/explorer/documentListTreeItem.ts +++ b/src/explorer/documentListTreeItem.ts @@ -59,9 +59,8 @@ export default class DocumentListTreeItem extends vscode.TreeItem contextValue = DOCUMENT_LIST_ITEM; - // We fetch 1 more than this in order to see if there are more to fetch. + private _documentCount: number | null; private _maxDocumentsToShow: number; - hasMoreDocumentsToShow: boolean; collectionName: string; databaseName: string; @@ -79,7 +78,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem dataService: any, isExpanded: boolean, maxDocumentsToShow: number, - hasMoreDocumentsToShow: boolean, + cachedDocumentCount: number | null, cacheIsUpToDate: boolean, existingCache: Array ) { @@ -95,10 +94,15 @@ export default class DocumentListTreeItem extends vscode.TreeItem this.isExpanded = isExpanded; this._maxDocumentsToShow = maxDocumentsToShow; - this.hasMoreDocumentsToShow = hasMoreDocumentsToShow; + this._documentCount = cachedDocumentCount; this._childrenCache = existingCache; this.cacheIsUpToDate = cacheIsUpToDate; + + if (this._documentCount !== null) { + // TODO: Prettify this count, 1k, 2k, 3m etc. + this.description = `${this._documentCount}`; + } } get tooltip(): string { @@ -122,9 +126,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem /* No filter */ }, { - // We fetch 1 more than the max documents to show to see if - // there are more documents we aren't showing. - limit: 1 + this._maxDocumentsToShow + limit: this._maxDocumentsToShow }, (err: Error, documents: []) => { if (err) { @@ -138,6 +140,37 @@ export default class DocumentListTreeItem extends vscode.TreeItem }); } + getCount(): Promise { + log.info( + `fetching document count from namespace ${this.namespace}` + ); + + return new Promise((resolve, reject) => { + this._dataService.count( + this.namespace, + {}, // No filter. + {}, // No options. + (err: Error | undefined, count: number) => { + if (err) { + return reject( + new Error(`Unable to get collection document count: ${err.message}`) + ); + } + + return resolve(count); + } + ); + }); + } + + hasMoreDocumentsToShow(): boolean { + if (this._documentCount === null) { + return false; + } + + return this._maxDocumentsToShow <= this._documentCount; + } + async getChildren(): Promise { if (!this.isExpanded || this.type === CollectionTypes.view) { return []; @@ -157,7 +190,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem ); }); - if (this.hasMoreDocumentsToShow) { + if (this.hasMoreDocumentsToShow()) { return [ ...this._childrenCache, // Add a `Show more...` item when there are more documents to show. @@ -180,18 +213,10 @@ export default class DocumentListTreeItem extends vscode.TreeItem } this.cacheIsUpToDate = true; - this.hasMoreDocumentsToShow = false; if (documents) { this._childrenCache = []; documents.forEach((document, index) => { - // We fetch 1 more than the max documents to see if - // there are more documents we aren't showing. - if (index === this._maxDocumentsToShow) { - this.hasMoreDocumentsToShow = true; - return; - } - this._childrenCache.push( new DocumentTreeItem(document, this.namespace, index) ); @@ -200,7 +225,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem this._childrenCache = []; } - if (this.hasMoreDocumentsToShow) { + if (this.hasMoreDocumentsToShow()) { return [ ...this._childrenCache, new ShowMoreDocumentsTreeItem( @@ -238,20 +263,30 @@ export default class DocumentListTreeItem extends vscode.TreeItem this.isExpanded = false; this.cacheIsUpToDate = false; this._maxDocumentsToShow = MAX_DOCUMENTS_VISIBLE; - this.hasMoreDocumentsToShow = false; } - onDidExpand(): Promise { + async onDidExpand(): Promise { + try { + // We fetch the document when we expand in order to + // show the document count in the tree item `description`. + this._documentCount = await this.getCount(); + } catch (err) { + vscode.window.showInformationMessage( + `Unable to fetch document count: ${err}` + ); + return false; + } + this.cacheIsUpToDate = false; this.isExpanded = true; - return Promise.resolve(true); + return true; } resetCache(): void { this._childrenCache = []; this.cacheIsUpToDate = false; - this.hasMoreDocumentsToShow = false; + this._documentCount = null; } getChildrenCache(): Array { @@ -261,4 +296,8 @@ export default class DocumentListTreeItem extends vscode.TreeItem getMaxDocumentsToShow(): number { return this._maxDocumentsToShow; } + + getDocumentCount(): number | null { + return this._documentCount; + } } diff --git a/src/language/mongoDBService.ts b/src/language/mongoDBService.ts index 6d27ced90..eb4ad651d 100644 --- a/src/language/mongoDBService.ts +++ b/src/language/mongoDBService.ts @@ -13,12 +13,6 @@ const fs = require('fs'); export const languageServerWorkerFileName = 'languageServerWorker.js'; -type SslFileOptions = { - sslCA?: string; - sslKey?: string; - sslCert?: string; -}; - export default class MongoDBService { _serviceProvider?: CliServiceProvider; _runtime?: ElectronRuntime; diff --git a/src/mdbExtensionController.ts b/src/mdbExtensionController.ts index 13e7d9ac4..e9fcbefb6 100644 --- a/src/mdbExtensionController.ts +++ b/src/mdbExtensionController.ts @@ -19,6 +19,7 @@ import ConnectionTreeItem from './explorer/connectionTreeItem'; import SchemaTreeItem from './explorer/schemaTreeItem'; import DocumentTreeItem from './explorer/documentTreeItem'; import WebviewController from './views/webviewController'; +import DocumentListTreeItem from './explorer/documentListTreeItem'; const log = createLogger('commands'); @@ -388,7 +389,9 @@ export default class MDBExtensionController implements vscode.Disposable { ); this.registerCommand( 'mdb.viewCollectionDocuments', - (element: CollectionTreeItem): Promise => { + ( + element: CollectionTreeItem | DocumentListTreeItem + ): Promise => { const namespace = `${element.databaseName}.${element.collectionName}`; return this._editorsController.onViewCollectionDocuments(namespace); } @@ -400,6 +403,13 @@ export default class MDBExtensionController implements vscode.Disposable { return this._explorerController.refresh(); } ); + this.registerCommand( + 'mdb.refreshDocumentList', + (documentsListTreeItem: DocumentListTreeItem): Promise => { + documentsListTreeItem.resetCache(); + return this._explorerController.refresh(); + } + ); this.registerCommand( 'mdb.refreshSchema', (schemaTreeItem: SchemaTreeItem): Promise => { diff --git a/src/test/suite/explorer/documentListTreeItem.test.ts b/src/test/suite/explorer/documentListTreeItem.test.ts index 6b2acac42..43e415cac 100644 --- a/src/test/suite/explorer/documentListTreeItem.test.ts +++ b/src/test/suite/explorer/documentListTreeItem.test.ts @@ -7,9 +7,8 @@ import DocumentListTreeItem, { CollectionTypes, MAX_DOCUMENTS_VISIBLE } from '../../../explorer/documentListTreeItem'; -import { ext } from '../../../extensionConstants'; -import { DataServiceStub, TestExtensionContext, mockDocuments } from '../stubs'; +import { DataServiceStub, mockDocuments } from '../stubs'; suite('DocumentListTreeItem Test Suite', () => { test('its context value should be in the package json', () => { @@ -21,7 +20,7 @@ suite('DocumentListTreeItem Test Suite', () => { 'not_real_dataservice', false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); @@ -46,7 +45,7 @@ suite('DocumentListTreeItem Test Suite', () => { 'not_real_dataservice', false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); @@ -66,28 +65,47 @@ suite('DocumentListTreeItem Test Suite', () => { ); }); - test('when not expanded it does not show documents', (done) => { - const testDocumentListTreeItem = new DocumentListTreeItem( - 'mock_collection_name', - 'mock_db_name', - CollectionTypes.collection, - new DataServiceStub(), - false, - MAX_DOCUMENTS_VISIBLE, - false, - false, - [] - ); + suite('when not expanded', () => { + test('it does not show documents', async () => { + const testDocumentListTreeItem = new DocumentListTreeItem( + 'mock_collection_name', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + null, + false, + [] + ); + + const collections = await testDocumentListTreeItem.getChildren(); + assert( + collections.length === 0, + `Expected no collections to be returned, found ${collections.length}` + ); + }); - testDocumentListTreeItem - .getChildren() - .then((collections) => { - assert( - collections.length === 0, - `Expected no collections to be returned, found ${collections.length}` - ); - }) - .then(done, done); + test('it does not have a document count in the description', async () => { + const testDocumentListTreeItem = new DocumentListTreeItem( + 'mock_collection_name', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + null, + false, + [] + ); + + await testDocumentListTreeItem.getChildren(); + + assert( + testDocumentListTreeItem.description === undefined, + `Expected no document count description found ${testDocumentListTreeItem.description}` + ); + }); }); test('a "view" type of document list does not show a dropdown', () => { @@ -98,18 +116,18 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); assert( testDocumentListTreeItem.collapsibleState === - vscode.TreeItemCollapsibleState.None + vscode.TreeItemCollapsibleState.None ); }); - test('when expanded shows the documents of a collection in tree', (done) => { + test('when expanded shows the documents of a collection in tree', async () => { const testDocumentListTreeItem = new DocumentListTreeItem( 'mock_collection_name_1', 'mock_db_name', @@ -117,28 +135,25 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); testDocumentListTreeItem.onDidExpand(); - testDocumentListTreeItem - .getChildren() - .then((documents) => { - assert( - documents.length === 11, - `Expected 11 documents to be returned, found ${documents.length}` - ); - assert( - documents[1].label === `"${mockDocuments[1]._id}"`, - `Expected a tree item child with the label document name ${mockDocuments[1]._id} found ${documents[1].label}` - ); - }) - .then(done, done); + const documents = await testDocumentListTreeItem.getChildren(); + + assert( + documents.length === 10, + `Expected 10 documents to be returned, found ${documents.length}` + ); + assert( + documents[1].label === `"${mockDocuments[1]._id}"`, + `Expected a tree item child with the label document name ${mockDocuments[1]._id} found ${documents[1].label}` + ); }); - test('it should show a show more item when there are more documents to show', (done) => { + test('it should show a show more item when there are more documents to show', async () => { const testDocumentListTreeItem = new DocumentListTreeItem( 'mock_collection_name_2', 'mock_db_name', @@ -147,28 +162,25 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); testDocumentListTreeItem.onDidExpand(); - testDocumentListTreeItem - .getChildren() - .then((documents) => { - assert( - documents.length === 11, - `Expected 11 documents to be returned, found ${documents.length}` - ); - assert( - documents[10].label === 'Show more...', - `Expected a tree item child with the label "show more..." found ${documents[10].label}` - ); - }) - .then(done, done); + const documents = await testDocumentListTreeItem.getChildren(); + + assert( + documents.length === 11, + `Expected 11 documents to be returned, found ${documents.length}` + ); + assert( + documents[10].label === 'Show more...', + `Expected a tree item child with the label "show more..." found ${documents[10].label}` + ); }); - test('it should show more documents after the show more click handler is called', (done) => { + test('it should show more documents after the show more click handler is called', async () => { const testDocumentListTreeItem = new DocumentListTreeItem( 'mock_collection_name_3', 'mock_db_name', @@ -176,7 +188,7 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); @@ -184,26 +196,22 @@ suite('DocumentListTreeItem Test Suite', () => { testDocumentListTreeItem.onDidExpand(); testDocumentListTreeItem.onShowMoreClicked(); - testDocumentListTreeItem - .getChildren() - .then((documents) => { - assert( - documents.length === 21, - `Expected 21 documents to be returned, found ${documents.length}` - ); - assert( - documents[19].label === `"${mockDocuments[19]._id}"`, - `Expected a document tree item with the label ${mockDocuments[19]._id}, found ${documents[19].label}` - ); - assert( - documents[20].label === 'Show more...', - `Expected a tree item child with the label "show more..." found ${documents[10].label}` - ); - }) - .then(done, done); + const documents = await testDocumentListTreeItem.getChildren(); + assert( + documents.length === 20, + `Expected 20 documents to be returned, found ${documents.length}` + ); + assert( + documents[19].label === `"${mockDocuments[19]._id}"`, + `Expected a document tree item with the label ${mockDocuments[19]._id}, found ${documents[19].label}` + ); + assert( + documents[20].label === 'Show more...', + `Expected a tree item child with the label "show more..." found ${documents[10].label}` + ); }); - test('it should not show a show more item when there not are more documents to show', (done) => { + test('it should not show a show more item when there not are more documents to show', async () => { const testDocumentListTreeItem = new DocumentListTreeItem( 'mock_collection_name_4', 'mock_db_name', @@ -211,35 +219,30 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); - testDocumentListTreeItem.onDidExpand(); + await testDocumentListTreeItem.onDidExpand(); // Increase the max to 30 ish. testDocumentListTreeItem.onShowMoreClicked(); testDocumentListTreeItem.onShowMoreClicked(); - testDocumentListTreeItem - .getChildren() - .then((documents) => { - assert( - documents.length === 25, - `Expected 25 documents to be returned, found ${documents.length}` - ); - assert( - documents[documents.length - 1].label !== 'Show more...', - 'Expected the last tree item to not have the label "show more..."' - ); - }) - .then(done, done); + const documents = await testDocumentListTreeItem.getChildren(); + + assert( + documents.length === 25, + `Expected 25 documents to be returned, found ${documents.length}` + ); + assert( + documents[documents.length - 1].label !== 'Show more...', + 'Expected the last tree item to not have the label "show more..."' + ); }); test('it shows a documents icon', () => { - ext.context = new TestExtensionContext(); - const testCollectionViewTreeItem = new DocumentListTreeItem( 'mock_collection_name_4', 'mock_db_name', @@ -247,7 +250,7 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); @@ -265,7 +268,7 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - false, + null, false, [] ); @@ -276,4 +279,36 @@ suite('DocumentListTreeItem Test Suite', () => { 'Expected icon path to point to an svg by the name "documents" with a light mode' ); }); + + test('it shows the document count after it has been expanded.', async () => { + const testDocumentListTreeItem = new DocumentListTreeItem( + 'mock_collection_name_4', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + null, + false, + [] + ); + + await testDocumentListTreeItem.onDidExpand(); + + assert(testDocumentListTreeItem.getDocumentCount() === 25); + + const testNewDocListItem = new DocumentListTreeItem( + 'mock_collection_name_4', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + testDocumentListTreeItem.getDocumentCount(), + false, + [] + ); + + assert(testNewDocListItem.description === '25'); + }); }); diff --git a/src/test/suite/explorer/schemaTreeItem.test.ts b/src/test/suite/explorer/schemaTreeItem.test.ts index 21818195b..ead1195bc 100644 --- a/src/test/suite/explorer/schemaTreeItem.test.ts +++ b/src/test/suite/explorer/schemaTreeItem.test.ts @@ -9,11 +9,14 @@ import SchemaTreeItem, { FIELDS_TO_SHOW } from '../../../explorer/schemaTreeItem'; import { fieldIsExpandable } from '../../../explorer/fieldTreeItem'; +import { ext } from '../../../extensionConstants'; + import { seedDataAndCreateDataService, cleanupTestDB, TEST_DB_NAME } from '../dbTestHelper'; +import { TestExtensionContext } from '../stubs'; suite('SchemaTreeItem Test Suite', () => { afterEach(() => { @@ -354,6 +357,8 @@ suite('SchemaTreeItem Test Suite', () => { }); test('it should have an icon with the name schema', () => { + ext.context = new TestExtensionContext(); + const testSchemaTreeItem = new SchemaTreeItem( 'favoritePiesIWantToEatRightNow', TEST_DB_NAME, diff --git a/src/test/suite/extension.test.ts b/src/test/suite/extension.test.ts index 747041d0f..a0f013abd 100644 --- a/src/test/suite/extension.test.ts +++ b/src/test/suite/extension.test.ts @@ -18,7 +18,7 @@ suite('Extension Test Suite', () => { createTerminalStub.returns({ sendText: fakeSendTerminalText, - show: () => {} + show: () => {}, }); sandbox.replace(vscode.window, 'createTerminal', createTerminalStub); }); @@ -53,12 +53,13 @@ suite('Extension Test Suite', () => { 'mdb.refreshDatabase', 'mdb.addCollection', 'mdb.viewCollectionDocuments', + 'mdb.refreshDocumentList', 'mdb.copyCollectionName', 'mdb.refreshCollection', 'mdb.refreshSchema', // Editor commands. - 'mdb.codeLens.showMoreDocumentsClicked' + 'mdb.codeLens.showMoreDocumentsClicked', ]; for (let i = 0; i < expectedCommands.length; i++) { diff --git a/src/test/suite/index.ts b/src/test/suite/index.ts index 2db916e28..28dfe5d57 100644 --- a/src/test/suite/index.ts +++ b/src/test/suite/index.ts @@ -15,6 +15,7 @@ type KeyTar = typeof keytarType; export function run(): Promise { const reporterOptions = { spec: '-', + color: true, 'mocha-junit-reporter': path.join(__dirname, './test-results.xml') }; @@ -23,8 +24,9 @@ export function run(): Promise { reporter: 'mocha-multi', reporterOptions, ui: 'tdd' + // color: true }); - mocha.useColors(true); + // mocha.color = true; const testsRoot = path.join(__dirname, '..'); diff --git a/src/test/suite/mdbExtensionController.test.ts b/src/test/suite/mdbExtensionController.test.ts index ac57cea33..4224082a6 100644 --- a/src/test/suite/mdbExtensionController.test.ts +++ b/src/test/suite/mdbExtensionController.test.ts @@ -14,6 +14,7 @@ import { } from '../../explorer'; import { mdbTestExtension } from './stubbableMdbExtension'; import { StorageScope } from '../../storage/storageController'; +import DocumentListTreeItem from '../../explorer/documentListTreeItem'; const sinon = require('sinon'); const testDatabaseURI = 'mongodb://localhost:27018'; @@ -382,6 +383,47 @@ suite('MDBExtensionController Test Suite', () => { .then(done, done); }); + test('mdb.refreshDocumentList command should update the document count and call to refresh the explorer controller', async () => { + const mockTreeItem = new DocumentListTreeItem( + 'dolphin_collection', + 'ocean_database', + CollectionTypes.collection, + {}, + false, + 5, + 5, + false, + [] + ); + + mockTreeItem.isExpanded = true; + + const mockExplorerControllerRefresh = sinon.fake.resolves(); + sinon.replace( + mdbTestExtension.testExtensionController._explorerController, + 'refresh', + mockExplorerControllerRefresh + ); + + await vscode.commands.executeCommand( + 'mdb.refreshDocumentList', + mockTreeItem + ); + + assert( + mockTreeItem.cacheIsUpToDate === false, + 'Expected document list cache to be out of date.' + ); + assert( + mockTreeItem.getDocumentCount() === null, + 'Expected document count to be null.' + ); + assert( + mockExplorerControllerRefresh.called === true, + 'Expected explorer controller refresh to be called.' + ); + }); + test('mdb.refreshSchema command should reset its cache and call to refresh the explorer controller', (done) => { const mockTreeItem = new SchemaTreeItem( 'zebraWearwolf', diff --git a/src/test/suite/stubs.ts b/src/test/suite/stubs.ts index 0b7c12245..9f7ba9af4 100644 --- a/src/test/suite/stubs.ts +++ b/src/test/suite/stubs.ts @@ -117,6 +117,10 @@ class DataServiceStub { find(namespace: string, filter: any, options: any, callback: any): void { callback(null, mockDocuments.slice(0, options.limit)); } + + count(namespace: string, filter: any, options: any, callback: any): void { + callback(null, mockDocuments.length); + } } const mockPosition = new vscode.Position(0, 0); @@ -172,9 +176,9 @@ class MockLanguageServerController { this.client = null; } - activate(): void {} + activate(): void { /* */ } - deactivate(): void {} + deactivate(): void { /* */ } executeAll(codeToEvaluate: string): Promise { return Promise.resolve('Result'); From f27dd7adb344b8dbe31a997d799b09ff382421ba Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Thu, 16 Jul 2020 15:55:26 +0200 Subject: [PATCH 2/6] Add tests for collection document count --- src/explorer/documentListTreeItem.ts | 4 + .../suite/explorer/databaseTreeItem.test.ts | 109 ++++++++---------- .../explorer/documentListTreeItem.test.ts | 14 +-- src/test/suite/index.ts | 5 +- 4 files changed, 62 insertions(+), 70 deletions(-) diff --git a/src/explorer/documentListTreeItem.ts b/src/explorer/documentListTreeItem.ts index fde1e3a53..cda81a2d8 100644 --- a/src/explorer/documentListTreeItem.ts +++ b/src/explorer/documentListTreeItem.ts @@ -208,6 +208,9 @@ export default class DocumentListTreeItem extends vscode.TreeItem let documents; try { documents = await this.getDocuments(); + if (this._documentCount === null) { + this._documentCount = await this.getCount(); + } } catch (err) { return Promise.reject(err); } @@ -284,6 +287,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem } resetCache(): void { + this.isExpanded = false; this._childrenCache = []; this.cacheIsUpToDate = false; this._documentCount = null; diff --git a/src/test/suite/explorer/databaseTreeItem.test.ts b/src/test/suite/explorer/databaseTreeItem.test.ts index 20745fba3..0ec03f3d3 100644 --- a/src/test/suite/explorer/databaseTreeItem.test.ts +++ b/src/test/suite/explorer/databaseTreeItem.test.ts @@ -87,7 +87,7 @@ suite('DatabaseTreeItem Test Suite', () => { .then(done, done); }); - test('when expanded and collapsed its collections cache their expanded documents', (done) => { + test('when expanded and collapsed its collections cache their expanded documents', async () => { const testDatabaseTreeItem = new DatabaseTreeItem( mockDatabaseNames[1], new DataServiceStub(), @@ -96,69 +96,58 @@ suite('DatabaseTreeItem Test Suite', () => { {} ); - testDatabaseTreeItem.onDidExpand(); + await testDatabaseTreeItem.onDidExpand(); - testDatabaseTreeItem - .getChildren() - .then((collectionTreeItems: CollectionTreeItem[]) => { - assert( - collectionTreeItems[1].isExpanded === false, - 'Expected collection tree item not to be expanded on default.' - ); + const collectionTreeItems = await testDatabaseTreeItem.getChildren(); - collectionTreeItems[1].onDidExpand(); - const documentListItem = collectionTreeItems[1].getDocumentListChild(); - if (!documentListItem) { - assert(false, 'No document list tree item found on collection.'); - return; - } - documentListItem.onDidExpand(); - documentListItem.onShowMoreClicked(); - - documentListItem.getChildren().then((documents: any[]) => { - const amountOfDocs = documents.length; - const expectedDocs = 21; - assert( - expectedDocs === amountOfDocs, - `Expected ${expectedDocs} documents, recieved ${amountOfDocs}` - ); - - testDatabaseTreeItem.onDidCollapse(); - testDatabaseTreeItem - .getChildren() - .then((postCollapseCollectionTreeItems) => { - assert( - postCollapseCollectionTreeItems.length === 0, - `Expected the database tree to return no children when collapsed, found ${collectionTreeItems.length}` - ); + assert( + collectionTreeItems[1].isExpanded === false, + 'Expected collection tree item not to be expanded on default.' + ); - testDatabaseTreeItem.onDidExpand(); - testDatabaseTreeItem - .getChildren() - .then((newCollectionTreeItems) => { - assert( - newCollectionTreeItems[1].isExpanded === true, - 'Expected collection tree item to be expanded from cache.' - ); + await collectionTreeItems[1].onDidExpand(); + const documentListItem = collectionTreeItems[1].getDocumentListChild(); + if (!documentListItem) { + assert(false, 'No document list tree item found on collection.'); + } + await documentListItem.onDidExpand(); + documentListItem.onShowMoreClicked(); + + const documents = await documentListItem.getChildren(); + const amountOfDocs = documents.length; + const expectedDocs = 21; + assert( + expectedDocs === amountOfDocs, + `Expected ${expectedDocs} documents, recieved ${amountOfDocs}` + ); - newCollectionTreeItems[1] - .getDocumentListChild() - .getChildren() - .then((documentsPostCollapseExpand) => { - // It should cache that we activated show more. - const amountOfCachedDocs = - documentsPostCollapseExpand.length; - const expectedCachedDocs = 21; - assert( - amountOfCachedDocs === expectedCachedDocs, - `Expected a cached ${expectedCachedDocs} documents to be returned, found ${amountOfCachedDocs}` - ); - }) - .then(done, done); - }); - }); - }); - }); + testDatabaseTreeItem.onDidCollapse(); + const postCollapseCollectionTreeItems = await testDatabaseTreeItem.getChildren(); + + assert( + postCollapseCollectionTreeItems.length === 0, + `Expected the database tree to return no children when collapsed, found ${collectionTreeItems.length}` + ); + + await testDatabaseTreeItem.onDidExpand(); + const newCollectionTreeItems = await testDatabaseTreeItem.getChildren(); + + assert( + newCollectionTreeItems[1].isExpanded === true, + 'Expected collection tree item to be expanded from cache.' + ); + + const documentsPostCollapseExpand = await newCollectionTreeItems[1] + .getDocumentListChild() + .getChildren(); + + // It should cache that we activated show more. + const amountOfCachedDocs = documentsPostCollapseExpand.length; + const expectedCachedDocs = 21; + assert( + amountOfCachedDocs === expectedCachedDocs, + `Expected a cached ${expectedCachedDocs} documents to be returned, found ${amountOfCachedDocs}` + ); }); test('collections are displayed in the alphanumerical order', (done) => { diff --git a/src/test/suite/explorer/documentListTreeItem.test.ts b/src/test/suite/explorer/documentListTreeItem.test.ts index 43e415cac..fb8ec51d5 100644 --- a/src/test/suite/explorer/documentListTreeItem.test.ts +++ b/src/test/suite/explorer/documentListTreeItem.test.ts @@ -139,13 +139,13 @@ suite('DocumentListTreeItem Test Suite', () => { false, [] ); - testDocumentListTreeItem.onDidExpand(); + await testDocumentListTreeItem.onDidExpand(); const documents = await testDocumentListTreeItem.getChildren(); assert( - documents.length === 10, - `Expected 10 documents to be returned, found ${documents.length}` + documents.length === 11, + `Expected 11 documents to be returned, found ${documents.length}` ); assert( documents[1].label === `"${mockDocuments[1]._id}"`, @@ -166,7 +166,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, [] ); - testDocumentListTreeItem.onDidExpand(); + await testDocumentListTreeItem.onDidExpand(); const documents = await testDocumentListTreeItem.getChildren(); @@ -193,13 +193,13 @@ suite('DocumentListTreeItem Test Suite', () => { [] ); - testDocumentListTreeItem.onDidExpand(); + await testDocumentListTreeItem.onDidExpand(); testDocumentListTreeItem.onShowMoreClicked(); const documents = await testDocumentListTreeItem.getChildren(); assert( - documents.length === 20, - `Expected 20 documents to be returned, found ${documents.length}` + documents.length === 21, + `Expected 21 documents to be returned, found ${documents.length}` ); assert( documents[19].label === `"${mockDocuments[19]._id}"`, diff --git a/src/test/suite/index.ts b/src/test/suite/index.ts index 28dfe5d57..4ffd10a21 100644 --- a/src/test/suite/index.ts +++ b/src/test/suite/index.ts @@ -15,7 +15,6 @@ type KeyTar = typeof keytarType; export function run(): Promise { const reporterOptions = { spec: '-', - color: true, 'mocha-junit-reporter': path.join(__dirname, './test-results.xml') }; @@ -24,9 +23,9 @@ export function run(): Promise { reporter: 'mocha-multi', reporterOptions, ui: 'tdd' - // color: true }); - // mocha.color = true; + + mocha.useColors(true); const testsRoot = path.join(__dirname, '..'); From da3f3e5b30eb3353f5afd9c528a4a4a9d45882a3 Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Fri, 17 Jul 2020 13:21:18 +0200 Subject: [PATCH 3/6] Change behavior to showing count when collection is open, make abbreviations uppercase --- package-lock.json | 13 ++- package.json | 1 + src/explorer/collectionTreeItem.ts | 82 +++++++++++---- src/explorer/databaseTreeItem.ts | 5 +- src/explorer/documentListTreeItem.ts | 77 +++++---------- src/explorer/explorerTreeController.ts | 12 +-- .../suite/explorer/collectionTreeItem.test.ts | 94 ++++++++++++------ .../suite/explorer/databaseTreeItem.test.ts | 1 + .../explorer/documentListTreeItem.test.ts | 99 ++++++++++++++++--- .../suite/explorer/documentTreeItem.test.ts | 6 +- src/test/suite/extension.test.ts | 4 +- src/test/suite/index.ts | 1 - src/test/suite/mdbExtensionController.test.ts | 67 +++++++++---- src/test/suite/stubs.ts | 2 +- 14 files changed, 311 insertions(+), 153 deletions(-) diff --git a/package-lock.json b/package-lock.json index 67886ab5c..10a2dc61c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9927,6 +9927,12 @@ "resolved": "https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz", "integrity": "sha1-W/Rejkm6QYnhfUgnid/RW9FAt7Y=" }, + "numeral": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/numeral/-/numeral-1.5.6.tgz", + "integrity": "sha1-ODHbloRRuc9q/5v5WSXx7443sz8=", + "optional": true + }, "progress": { "version": "1.1.8", "resolved": "https://registry.npmjs.org/progress/-/progress-1.1.8.tgz", @@ -10584,10 +10590,9 @@ "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=" }, "numeral": { - "version": "1.5.6", - "resolved": "https://registry.npmjs.org/numeral/-/numeral-1.5.6.tgz", - "integrity": "sha1-ODHbloRRuc9q/5v5WSXx7443sz8=", - "optional": true + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/numeral/-/numeral-2.0.6.tgz", + "integrity": "sha1-StCAk21EPCVhrtnyGX7//iX05QY=" }, "oauth-sign": { "version": "0.9.0", diff --git a/package.json b/package.json index 98431ba9c..1c88bf3ee 100644 --- a/package.json +++ b/package.json @@ -659,6 +659,7 @@ "mongodb-data-service": "^16.8.1", "mongodb-ns": "^2.2.0", "mongodb-schema": "^8.2.5", + "numeral": "^2.0.6", "react": "^16.13.1", "react-dom": "^16.13.1", "react-redux": "^7.2.0", diff --git a/src/explorer/collectionTreeItem.ts b/src/explorer/collectionTreeItem.ts index ee3e5cd77..6e7c1f4a8 100644 --- a/src/explorer/collectionTreeItem.ts +++ b/src/explorer/collectionTreeItem.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; const path = require('path'); +import { createLogger } from '../logging'; import DocumentListTreeItem, { CollectionTypes, MAX_DOCUMENTS_VISIBLE @@ -10,6 +11,8 @@ import TreeItemParent from './treeItemParentInterface'; import SchemaTreeItem from './schemaTreeItem'; import { getImagesPath } from '../extensionConstants'; +const log = createLogger('tree view collection folder'); + type CollectionModelType = { name: string; type: CollectionTypes; @@ -36,9 +39,11 @@ export default class CollectionTreeItem extends vscode.TreeItem collection: CollectionModelType; collectionName: string; databaseName: string; + namespace: string; private _dataService: any; private _type: CollectionTypes; + documentCount: number | null; isExpanded: boolean; @@ -50,6 +55,7 @@ export default class CollectionTreeItem extends vscode.TreeItem dataService: any, isExpanded: boolean, cacheIsUpToDate: boolean, + cachedDocumentCount: number | null, existingDocumentListChild?: DocumentListTreeItem, existingSchemaChild?: SchemaTreeItem, existingIndexListChild?: IndexListTreeItem @@ -64,12 +70,15 @@ export default class CollectionTreeItem extends vscode.TreeItem this.collection = collection; this.collectionName = collection.name; this.databaseName = databaseName; + this.namespace = `${this.databaseName}.${this.collectionName}`; this._type = collection.type; // Type can be `collection` or `view`. this._dataService = dataService; this.isExpanded = isExpanded; + this.documentCount = cachedDocumentCount; + this.cacheIsUpToDate = cacheIsUpToDate; this._documentListChild = existingDocumentListChild @@ -81,7 +90,8 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, false, // Collapsed. MAX_DOCUMENTS_VISIBLE, - null, // No document count yet. + this.documentCount, + this.refreshDocumentCount, false, // Cache is not up to date. [] // Empty cache. ); @@ -119,9 +129,13 @@ export default class CollectionTreeItem extends vscode.TreeItem return element; } - getChildren(): Thenable { + async getChildren(): Promise { if (!this.isExpanded) { - return Promise.resolve([]); + return []; + } + + if (this.documentCount === null) { + await this.refreshDocumentCount(); } // Update cache if one of the children has been expanded/collapsed. @@ -130,11 +144,7 @@ export default class CollectionTreeItem extends vscode.TreeItem } if (this.cacheIsUpToDate) { - return Promise.resolve([ - this._documentListChild, - this._schemaChild, - this._indexListChild - ]); + return [this._documentListChild, this._schemaChild, this._indexListChild]; } this.cacheIsUpToDate = true; @@ -143,11 +153,7 @@ export default class CollectionTreeItem extends vscode.TreeItem // is ensure to be set by vscode. this.rebuildChildrenCache(); - return Promise.resolve([ - this._documentListChild, - this._schemaChild, - this._indexListChild - ]); + return [this._documentListChild, this._schemaChild, this._indexListChild]; } rebuildDocumentListTreeItem(): void { @@ -158,7 +164,8 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, this._documentListChild.isExpanded, this._documentListChild.getMaxDocumentsToShow(), - this._documentListChild.getDocumentCount(), + this.documentCount, + this.refreshDocumentCount, this._documentListChild.cacheIsUpToDate, this._documentListChild.getChildrenCache() ); @@ -209,15 +216,18 @@ export default class CollectionTreeItem extends vscode.TreeItem this.cacheIsUpToDate = false; } - onDidExpand(): Promise { + async onDidExpand(): Promise { this.isExpanded = true; this.cacheIsUpToDate = false; - return Promise.resolve(true); + await this.refreshDocumentCount(); + + return true; } resetCache(): void { this.cacheIsUpToDate = false; + this.documentCount = null; this._documentListChild = new DocumentListTreeItem( this.collectionName, @@ -226,7 +236,8 @@ export default class CollectionTreeItem extends vscode.TreeItem this._dataService, false, // Collapsed. MAX_DOCUMENTS_VISIBLE, - null, // No document count yet. + this.documentCount, + this.refreshDocumentCount, false, // Cache is not up to date. [] // Empty cache. ); @@ -268,6 +279,43 @@ export default class CollectionTreeItem extends vscode.TreeItem return this._documentListChild.getMaxDocumentsToShow(); } + getCount(): Promise { + log.info(`fetching document count from namespace ${this.namespace}`); + + return new Promise((resolve, reject) => { + this._dataService.estimatedCount( + this.namespace, + {}, // No options. + (err: Error | undefined, count: number) => { + if (err) { + return reject( + new Error( + `Unable to get collection document count: ${err.message}` + ) + ); + } + + return resolve(count); + } + ); + }); + } + + refreshDocumentCount = async (): Promise => { + try { + // We fetch the document when we expand in order to + // show the document count in the tree item `description`. + this.documentCount = await this.getCount(); + } catch (err) { + vscode.window.showInformationMessage( + `Unable to fetch document count: ${err}` + ); + return false; + } + + return true; + }; + async onDropCollectionClicked(): Promise { const collectionName = this.collectionName; diff --git a/src/explorer/databaseTreeItem.ts b/src/explorer/databaseTreeItem.ts index 606e49afc..56024b9e5 100644 --- a/src/explorer/databaseTreeItem.ts +++ b/src/explorer/databaseTreeItem.ts @@ -85,6 +85,7 @@ export default class DatabaseTreeItem extends vscode.TreeItem this._dataService, pastChildrenCache[collectionName].isExpanded, pastChildrenCache[collectionName].cacheIsUpToDate, + pastChildrenCache[collectionName].documentCount, pastChildrenCache[collectionName].getDocumentListChild(), pastChildrenCache[collectionName].getSchemaChild(), pastChildrenCache[collectionName].getIndexListChild() @@ -122,6 +123,7 @@ export default class DatabaseTreeItem extends vscode.TreeItem this._dataService, pastChildrenCache[collection.name].isExpanded, pastChildrenCache[collection.name].cacheIsUpToDate, + pastChildrenCache[collection.name].documentCount, pastChildrenCache[collection.name].getDocumentListChild(), pastChildrenCache[collection.name].getSchemaChild(), pastChildrenCache[collection.name].getIndexListChild() @@ -132,7 +134,8 @@ export default class DatabaseTreeItem extends vscode.TreeItem this.databaseName, this._dataService, false, // Not expanded. - false // No cache. + false, // No cache. + null // No document count yet. ); } }); diff --git a/src/explorer/documentListTreeItem.ts b/src/explorer/documentListTreeItem.ts index cda81a2d8..bc891deb1 100644 --- a/src/explorer/documentListTreeItem.ts +++ b/src/explorer/documentListTreeItem.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import * as numeral from 'numeral'; const path = require('path'); import { createLogger } from '../logging'; @@ -50,6 +51,10 @@ const getCollapsableStateForDocumentList = ( : vscode.TreeItemCollapsibleState.Collapsed; }; +export const formatDocCount = (count: number): string => { + return `${numeral(count).format('0a')}`.toUpperCase(); +}; + export default class DocumentListTreeItem extends vscode.TreeItem implements TreeItemParent, vscode.TreeDataProvider { cacheIsUpToDate = false; @@ -59,7 +64,13 @@ export default class DocumentListTreeItem extends vscode.TreeItem contextValue = DOCUMENT_LIST_ITEM; - private _documentCount: number | null; + // We display the document count in the description of the + // document list tree item, even when it hasn't been expanded. + // When it is expanded, we want to ensure that number is up to date. + // This function tells the parent collection folder to refresh the count. + refreshDocumentCount: () => Promise; + + _documentCount: number | null; private _maxDocumentsToShow: number; collectionName: string; @@ -79,6 +90,7 @@ export default class DocumentListTreeItem extends vscode.TreeItem isExpanded: boolean, maxDocumentsToShow: number, cachedDocumentCount: number | null, + refreshDocumentCount: () => Promise, cacheIsUpToDate: boolean, existingCache: Array ) { @@ -96,17 +108,22 @@ export default class DocumentListTreeItem extends vscode.TreeItem this._maxDocumentsToShow = maxDocumentsToShow; this._documentCount = cachedDocumentCount; + this.refreshDocumentCount = refreshDocumentCount; + this._childrenCache = existingCache; this.cacheIsUpToDate = cacheIsUpToDate; if (this._documentCount !== null) { - // TODO: Prettify this count, 1k, 2k, 3m etc. - this.description = `${this._documentCount}`; + this.description = formatDocCount(this._documentCount); } } get tooltip(): string { - const typeString = CollectionTypes.view ? 'View' : 'Collection'; + const typeString = + this.type === CollectionTypes.view ? 'View' : 'Collection'; + if (this._documentCount !== null) { + return `${typeString} Documents - ${this._documentCount}`; + } return `${typeString} Documents`; } @@ -140,29 +157,6 @@ export default class DocumentListTreeItem extends vscode.TreeItem }); } - getCount(): Promise { - log.info( - `fetching document count from namespace ${this.namespace}` - ); - - return new Promise((resolve, reject) => { - this._dataService.count( - this.namespace, - {}, // No filter. - {}, // No options. - (err: Error | undefined, count: number) => { - if (err) { - return reject( - new Error(`Unable to get collection document count: ${err.message}`) - ); - } - - return resolve(count); - } - ); - }); - } - hasMoreDocumentsToShow(): boolean { if (this._documentCount === null) { return false; @@ -208,24 +202,19 @@ export default class DocumentListTreeItem extends vscode.TreeItem let documents; try { documents = await this.getDocuments(); - if (this._documentCount === null) { - this._documentCount = await this.getCount(); - } } catch (err) { return Promise.reject(err); } this.cacheIsUpToDate = true; + this._childrenCache = []; if (documents) { - this._childrenCache = []; documents.forEach((document, index) => { this._childrenCache.push( new DocumentTreeItem(document, this.namespace, index) ); }); - } else { - this._childrenCache = []; } if (this.hasMoreDocumentsToShow()) { @@ -269,28 +258,20 @@ export default class DocumentListTreeItem extends vscode.TreeItem } async onDidExpand(): Promise { - try { - // We fetch the document when we expand in order to - // show the document count in the tree item `description`. - this._documentCount = await this.getCount(); - } catch (err) { - vscode.window.showInformationMessage( - `Unable to fetch document count: ${err}` - ); - return false; - } - this.cacheIsUpToDate = false; this.isExpanded = true; + await this.refreshDocumentCount(); + return true; } - resetCache(): void { + async resetCache(): Promise { this.isExpanded = false; this._childrenCache = []; this.cacheIsUpToDate = false; - this._documentCount = null; + + await this.refreshDocumentCount(); } getChildrenCache(): Array { @@ -300,8 +281,4 @@ export default class DocumentListTreeItem extends vscode.TreeItem getMaxDocumentsToShow(): number { return this._maxDocumentsToShow; } - - getDocumentCount(): number | null { - return this._documentCount; - } } diff --git a/src/explorer/explorerTreeController.ts b/src/explorer/explorerTreeController.ts index 34a0a79f4..ae6583517 100644 --- a/src/explorer/explorerTreeController.ts +++ b/src/explorer/explorerTreeController.ts @@ -11,6 +11,7 @@ import MDBConnectionsTreeItem from './mdbConnectionsTreeItem'; import { createLogger } from '../logging'; import { DOCUMENT_LIST_ITEM, CollectionTypes } from './documentListTreeItem'; +import TreeItemParentInterface from './treeItemParentInterface'; const log = createLogger('explorer controller'); @@ -140,16 +141,9 @@ implements vscode.TreeDataProvider { } getChildren( - element?: - | MDBConnectionsTreeItem - | ConnectionTreeItem - | DatabaseTreeItem - | CollectionTreeItem + element?: any ): Thenable< - | MDBConnectionsTreeItem[] - | ConnectionTreeItem[] - | DatabaseTreeItem[] - | CollectionTreeItem[] + | any[] > { // When no element is present we are at the root. if (!element) { diff --git a/src/test/suite/explorer/collectionTreeItem.test.ts b/src/test/suite/explorer/collectionTreeItem.test.ts index 6707c6ffb..0fe4c276b 100644 --- a/src/test/suite/explorer/collectionTreeItem.test.ts +++ b/src/test/suite/explorer/collectionTreeItem.test.ts @@ -6,7 +6,7 @@ import CollectionTreeItem from '../../../explorer/collectionTreeItem'; import { CollectionTypes } from '../../../explorer/documentListTreeItem'; import { ext } from '../../../extensionConstants'; -import { TestExtensionContext } from '../stubs'; +import { TestExtensionContext, DataServiceStub } from '../stubs'; suite('CollectionTreeItem Test Suite', () => { test('its context value should be in the package json', () => { @@ -20,7 +20,8 @@ suite('CollectionTreeItem Test Suite', () => { 'mock_db_name', 'imaginary data service', false, - false + false, + null ); contributes.menus['view/item/context'].forEach((contextItem) => { @@ -35,41 +36,68 @@ suite('CollectionTreeItem Test Suite', () => { ); }); - test('when expanded shows a documents folder and schema folder', (done) => { + test('when expanded shows a documents folder and schema folder', async () => { const testCollectionTreeItem = new CollectionTreeItem( { name: 'mock_collection_name_1', type: CollectionTypes.collection }, 'mock_db_name', - 'imaginary data service', + new DataServiceStub(), + false, false, - false - ); - - testCollectionTreeItem.onDidExpand(); - - testCollectionTreeItem - .getChildren() - .then((children) => { - assert( - children.length === 3, - `Expected 3 children to be returned, found ${children.length}` - ); - assert( - children[0].label === 'Documents', - `Expected first child tree item to be named Documents found ${children[0].label}` - ); - assert( - children[1].label === 'Schema', - `Expected the second child tree item to be named Schema found ${children[1].label}` - ); - assert( - children[2].label === 'Indexes', - `Expected the second child tree item to be named Indexes found ${children[2].label}` - ); - }) - .then(done, done); + null + ); + + await testCollectionTreeItem.onDidExpand(); + + const collectionChildren = await testCollectionTreeItem.getChildren(); + + assert( + collectionChildren.length === 3, + `Expected 3 children to be returned, found ${collectionChildren.length}` + ); + assert( + collectionChildren[0].label === 'Documents', + `Expected first child tree item to be named Documents found ${collectionChildren[0].label}` + ); + assert( + collectionChildren[1].label === 'Schema', + `Expected the second child tree item to be named Schema found ${collectionChildren[1].label}` + ); + assert( + collectionChildren[2].label === 'Indexes', + `Expected the second child tree item to be named Indexes found ${collectionChildren[2].label}` + ); + }); + + test('when expanded it shows the document count in the description of the document list', async () => { + const testCollectionTreeItem = new CollectionTreeItem( + { + name: 'mock_collection_name_1', + type: CollectionTypes.collection + }, + 'mock_db_name', + { + estimatedCount: (ns, options, cb): void => cb(null, 5000) + }, + false, + false, + null + ); + + await testCollectionTreeItem.onDidExpand(); + + const collectionChildren = await testCollectionTreeItem.getChildren(); + + assert( + collectionChildren[0].label === 'Documents', + `Expected document list label to be 'Documents' got '${collectionChildren[0].label}'` + ); + assert( + collectionChildren[0].description === '5K', + `Expected document list description to be '5K' got '${collectionChildren[0].description}'` + ); }); test('a view should show a different icon from a collection', () => { @@ -83,7 +111,8 @@ suite('CollectionTreeItem Test Suite', () => { 'mock_db_name', 'imaginary data service', false, - false + false, + null ); const viewIconPath: any = testCollectionViewTreeItem.iconPath; @@ -104,7 +133,8 @@ suite('CollectionTreeItem Test Suite', () => { 'mock_db_name', 'imaginary data service', false, - false + false, + null ); const collectionIconPath: any = testCollectionCollectionTreeItem.iconPath; diff --git a/src/test/suite/explorer/databaseTreeItem.test.ts b/src/test/suite/explorer/databaseTreeItem.test.ts index 0ec03f3d3..524bbc22b 100644 --- a/src/test/suite/explorer/databaseTreeItem.test.ts +++ b/src/test/suite/explorer/databaseTreeItem.test.ts @@ -106,6 +106,7 @@ suite('DatabaseTreeItem Test Suite', () => { ); await collectionTreeItems[1].onDidExpand(); + await collectionTreeItems[1].getChildren(); const documentListItem = collectionTreeItems[1].getDocumentListChild(); if (!documentListItem) { assert(false, 'No document list tree item found on collection.'); diff --git a/src/test/suite/explorer/documentListTreeItem.test.ts b/src/test/suite/explorer/documentListTreeItem.test.ts index fb8ec51d5..fd00ddb3e 100644 --- a/src/test/suite/explorer/documentListTreeItem.test.ts +++ b/src/test/suite/explorer/documentListTreeItem.test.ts @@ -5,6 +5,7 @@ const { contributes } = require('../../../../package.json'); import DocumentListTreeItem, { CollectionTypes, + formatDocCount, MAX_DOCUMENTS_VISIBLE } from '../../../explorer/documentListTreeItem'; @@ -21,6 +22,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -46,6 +48,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -75,6 +78,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -95,6 +99,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -117,6 +122,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -135,7 +141,8 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - null, + 25, + (): Promise => Promise.resolve(true), false, [] ); @@ -158,11 +165,11 @@ suite('DocumentListTreeItem Test Suite', () => { 'mock_collection_name_2', 'mock_db_name', CollectionTypes.collection, - new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - null, + 25, + (): Promise => Promise.resolve(true), false, [] ); @@ -188,7 +195,8 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - null, + 25, + (): Promise => Promise.resolve(true), false, [] ); @@ -219,7 +227,8 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - null, + 25, + (): Promise => Promise.resolve(true), false, [] ); @@ -242,6 +251,49 @@ suite('DocumentListTreeItem Test Suite', () => { ); }); + test('when expanded it updates the count of documents', async () => { + let maxDocs; + const testDocumentListTreeItem = new DocumentListTreeItem( + 'mock_collection_name_1', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + maxDocs, + (): Promise => { + maxDocs = 25; + return Promise.resolve(true); + }, + false, + [] + ); + await testDocumentListTreeItem.onDidExpand(); + + const newTestDocList = new DocumentListTreeItem( + 'mock_collection_name_4', + 'mock_db_name', + CollectionTypes.collection, + new DataServiceStub(), + false, + MAX_DOCUMENTS_VISIBLE, + maxDocs, + (): Promise => Promise.resolve(true), + false, + [] + ); + + await newTestDocList.onDidExpand(); + + const documents = await newTestDocList.getChildren(); + + assert( + documents.length === 11, + `Expected 11 documents to be returned, found ${documents.length}` + ); + assert(newTestDocList.description === '25'); + }); + test('it shows a documents icon', () => { const testCollectionViewTreeItem = new DocumentListTreeItem( 'mock_collection_name_4', @@ -251,6 +303,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -269,6 +322,7 @@ suite('DocumentListTreeItem Test Suite', () => { false, MAX_DOCUMENTS_VISIBLE, null, + (): Promise => Promise.resolve(true), false, [] ); @@ -280,7 +334,7 @@ suite('DocumentListTreeItem Test Suite', () => { ); }); - test('it shows the document count after it has been expanded.', async () => { + test('it shows the document count in the description', () => { const testDocumentListTreeItem = new DocumentListTreeItem( 'mock_collection_name_4', 'mock_db_name', @@ -288,15 +342,16 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - null, + 25, + (): Promise => Promise.resolve(true), false, [] ); - await testDocumentListTreeItem.onDidExpand(); - - assert(testDocumentListTreeItem.getDocumentCount() === 25); + assert(testDocumentListTreeItem.description === '25'); + }); + test('the tooltip shows the unformated document count', () => { const testNewDocListItem = new DocumentListTreeItem( 'mock_collection_name_4', 'mock_db_name', @@ -304,11 +359,31 @@ suite('DocumentListTreeItem Test Suite', () => { new DataServiceStub(), false, MAX_DOCUMENTS_VISIBLE, - testDocumentListTreeItem.getDocumentCount(), + 2200000, + (): Promise => Promise.resolve(true), false, [] ); - assert(testNewDocListItem.description === '25'); + assert( + testNewDocListItem._documentCount === 2200000, + `Expected document count to be '2200000' found '${testNewDocListItem._documentCount}'` + ); + assert(testNewDocListItem.description === '2M'); + assert(testNewDocListItem.tooltip === 'Collection Documents - 2200000'); + }); + + suite('formatDocCount', () => { + test('It formats the document count when the count is 0', () => { + const num = 0; + const result = formatDocCount(num); + assert(result === '0'); + }); + + test('It formats the document count when the count is 10009', () => { + const num = 10009; + const result = formatDocCount(num); + assert(result === '10K'); + }); }); }); diff --git a/src/test/suite/explorer/documentTreeItem.test.ts b/src/test/suite/explorer/documentTreeItem.test.ts index 7e0ce7cf9..077adf826 100644 --- a/src/test/suite/explorer/documentTreeItem.test.ts +++ b/src/test/suite/explorer/documentTreeItem.test.ts @@ -3,7 +3,7 @@ import * as assert from 'assert'; import DocumentTreeItem from '../../../explorer/documentTreeItem'; suite('DocumentTreeItem Test Suite', () => { - test('it makes the document _id the label of the document tree item', function() { + test('it makes the document _id the label of the document tree item', function () { const mockDocument = { _id: 'mock_document_id' }; @@ -21,7 +21,7 @@ suite('DocumentTreeItem Test Suite', () => { ); }); - test('when the document has an object _id, it is stringified into the tree item label', function() { + test('when the document has an object _id, it is stringified into the tree item label', function () { const mockDocument = { _id: { someIdField: 'mock_document_id', @@ -44,7 +44,7 @@ suite('DocumentTreeItem Test Suite', () => { ); }); - test('when the document does not have an _id, its label is the supplied index', function() { + test('when the document does not have an _id, its label is the supplied index', function () { const mockDocument = { noIdField: true }; diff --git a/src/test/suite/extension.test.ts b/src/test/suite/extension.test.ts index a0f013abd..f10037ca5 100644 --- a/src/test/suite/extension.test.ts +++ b/src/test/suite/extension.test.ts @@ -18,7 +18,7 @@ suite('Extension Test Suite', () => { createTerminalStub.returns({ sendText: fakeSendTerminalText, - show: () => {}, + show: () => {} }); sandbox.replace(vscode.window, 'createTerminal', createTerminalStub); }); @@ -59,7 +59,7 @@ suite('Extension Test Suite', () => { 'mdb.refreshSchema', // Editor commands. - 'mdb.codeLens.showMoreDocumentsClicked', + 'mdb.codeLens.showMoreDocumentsClicked' ]; for (let i = 0; i < expectedCommands.length; i++) { diff --git a/src/test/suite/index.ts b/src/test/suite/index.ts index 4ffd10a21..2db916e28 100644 --- a/src/test/suite/index.ts +++ b/src/test/suite/index.ts @@ -24,7 +24,6 @@ export function run(): Promise { reporterOptions, ui: 'tdd' }); - mocha.useColors(true); const testsRoot = path.join(__dirname, '..'); diff --git a/src/test/suite/mdbExtensionController.test.ts b/src/test/suite/mdbExtensionController.test.ts index 4224082a6..2788b0083 100644 --- a/src/test/suite/mdbExtensionController.test.ts +++ b/src/test/suite/mdbExtensionController.test.ts @@ -44,7 +44,8 @@ suite('MDBExtensionController Test Suite', () => { 'testDbName', {}, false, - false + false, + null ); vscode.commands @@ -86,7 +87,8 @@ suite('MDBExtensionController Test Suite', () => { 'testDbName', {}, false, - false + false, + null ); vscode.commands @@ -289,7 +291,8 @@ suite('MDBExtensionController Test Suite', () => { 'airZebra', {}, false, - false + false, + null ); const mockCopyToClipboard = sinon.fake.resolves(); @@ -352,7 +355,8 @@ suite('MDBExtensionController Test Suite', () => { 'airZebra', {}, false, - false + false, + null ); mockTreeItem.isExpanded = true; @@ -384,19 +388,32 @@ suite('MDBExtensionController Test Suite', () => { }); test('mdb.refreshDocumentList command should update the document count and call to refresh the explorer controller', async () => { - const mockTreeItem = new DocumentListTreeItem( - 'dolphin_collection', - 'ocean_database', - CollectionTypes.collection, - {}, + let count = 9000; + const mockTreeItem = new CollectionTreeItem( + { + name: 'iSawACatThatLookedLikeALionToday', + type: CollectionTypes.collection + }, + 'airZebra', + { + estimatedCount: (ns, opts, cb): void => cb(null, count) + }, false, - 5, - 5, false, - [] + null ); - mockTreeItem.isExpanded = true; + await mockTreeItem.onDidExpand(); + + const collectionChildren = await mockTreeItem.getChildren(); + const docListTreeItem = collectionChildren[0]; + console.log('collectionChildren', collectionChildren); + + assert(docListTreeItem.description === '9K'); + + count = 10000; + + docListTreeItem.isExpanded = true; const mockExplorerControllerRefresh = sinon.fake.resolves(); sinon.replace( @@ -407,16 +424,20 @@ suite('MDBExtensionController Test Suite', () => { await vscode.commands.executeCommand( 'mdb.refreshDocumentList', - mockTreeItem + docListTreeItem ); assert( - mockTreeItem.cacheIsUpToDate === false, + docListTreeItem.cacheIsUpToDate === false, + 'Expected document list cache to be out of date.' + ); + assert( + docListTreeItem.isExpanded === false, 'Expected document list cache to be out of date.' ); assert( - mockTreeItem.getDocumentCount() === null, - 'Expected document count to be null.' + mockTreeItem.documentCount === 10000, + `Expected document count to be 10000, found ${mockTreeItem.documentCount}.` ); assert( mockExplorerControllerRefresh.called === true, @@ -828,7 +849,8 @@ suite('MDBExtensionController Test Suite', () => { } }, false, - false + false, + null ); const mockInputBoxResolves = sinon.stub(); @@ -856,7 +878,8 @@ suite('MDBExtensionController Test Suite', () => { 'doesntExistDBName', testConnectionController.getActiveDataService(), false, - false + false, + null ); const mockInputBoxResolves = sinon.stub(); @@ -896,7 +919,8 @@ suite('MDBExtensionController Test Suite', () => { 'fruitsThatAreTasty', {}, false, - false + false, + null ); const mockInputBoxResolves = sinon.stub(); @@ -920,7 +944,8 @@ suite('MDBExtensionController Test Suite', () => { 'fruitsThatAreTasty', {}, false, - false + false, + null ); const mockInputBoxResolves = sinon.stub(); diff --git a/src/test/suite/stubs.ts b/src/test/suite/stubs.ts index 9f7ba9af4..22053c0cd 100644 --- a/src/test/suite/stubs.ts +++ b/src/test/suite/stubs.ts @@ -118,7 +118,7 @@ class DataServiceStub { callback(null, mockDocuments.slice(0, options.limit)); } - count(namespace: string, filter: any, options: any, callback: any): void { + estimatedCount(namespace: string, options: any, callback: any): void { callback(null, mockDocuments.length); } } From 635098bcce16a85444de9016b1a7b63acf3ad918 Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Fri, 17 Jul 2020 14:52:39 +0200 Subject: [PATCH 4/6] Clean up tests, remove unneeded try --- src/explorer/collectionTreeItem.ts | 4 +- .../suite/explorer/explorerController.test.ts | 336 ++++++++---------- src/test/suite/mdbExtensionController.test.ts | 1 - 3 files changed, 158 insertions(+), 183 deletions(-) diff --git a/src/explorer/collectionTreeItem.ts b/src/explorer/collectionTreeItem.ts index 6e7c1f4a8..d057f2aaf 100644 --- a/src/explorer/collectionTreeItem.ts +++ b/src/explorer/collectionTreeItem.ts @@ -303,8 +303,8 @@ export default class CollectionTreeItem extends vscode.TreeItem refreshDocumentCount = async (): Promise => { try { - // We fetch the document when we expand in order to - // show the document count in the tree item `description`. + // We fetch the document when we expand in order to show + // the document count in the document list tree item `description`. this.documentCount = await this.getCount(); } catch (err) { vscode.window.showInformationMessage( diff --git a/src/test/suite/explorer/explorerController.test.ts b/src/test/suite/explorer/explorerController.test.ts index 83da0d470..9774318d3 100644 --- a/src/test/suite/explorer/explorerController.test.ts +++ b/src/test/suite/explorer/explorerController.test.ts @@ -2,7 +2,8 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; import { beforeEach, afterEach } from 'mocha'; import Connection = require('mongodb-connection-model/lib/model'); -import * as sinon from 'sinon'; +const sinon = require('sinon'); + import { DefaultSavingLocations, StorageScope @@ -24,6 +25,9 @@ suite('Explorer Controller Test Suite', function () { 'defaultConnectionSavingLocation', DefaultSavingLocations['Session Only'] ); + // Here we stub the showInformationMessage process because it is too much + // for the render process and leads to crashes while testing. + sinon.replace(vscode.window, 'showInformationMessage', sinon.stub()); }); afterEach(async () => { @@ -119,51 +123,47 @@ suite('Explorer Controller Test Suite', function () { mdbTestExtension.testExtensionController._explorerController; const treeController = testExplorerController.getTreeController(); - try { - const succesfullyConnected = await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); + const succesfullyConnected = await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); - assert( - succesfullyConnected === true, - 'Expected a successful connection response.' - ); - assert( - Object.keys(testConnectionController._connections).length === 1, - 'Expected there to be 1 connection in the connection list.' - ); + assert( + succesfullyConnected === true, + 'Expected a successful connection response.' + ); + assert( + Object.keys(testConnectionController._connections).length === 1, + 'Expected there to be 1 connection in the connection list.' + ); - const activeId = testConnectionController.getActiveConnectionId(); + const activeId = testConnectionController.getActiveConnectionId(); - assert( - activeId === Object.keys(testConnectionController._connections)[0], - `Expected active connection to be '${ - Object.keys(testConnectionController._connections)[0] - }' found ${activeId}` - ); + assert( + activeId === Object.keys(testConnectionController._connections)[0], + `Expected active connection to be '${ + Object.keys(testConnectionController._connections)[0] + }' found ${activeId}` + ); - const treeControllerChildren = await treeController.getChildren(); - const connectionsItems = await treeControllerChildren[0].getChildren(); + const treeControllerChildren = await treeController.getChildren(); + const connectionsItems = await treeControllerChildren[0].getChildren(); - assert( - connectionsItems.length === 1, - `Expected there be 1 connection tree item, found ${connectionsItems.length}` - ); - assert( - connectionsItems[0].label === 'localhost:27018', - 'There should be a connection tree item with the label "localhost:27018"' - ); - assert( - connectionsItems[0].description === 'connected', - 'There should be a connection tree item with the description "connected"' - ); - assert( - connectionsItems[0].isExpanded, - 'Expected the connection tree item to be expanded' - ); - } catch (error) { - assert(false, error); - } + assert( + connectionsItems.length === 1, + `Expected there be 1 connection tree item, found ${connectionsItems.length}` + ); + assert( + connectionsItems[0].label === 'localhost:27018', + 'There should be a connection tree item with the label "localhost:27018"' + ); + assert( + connectionsItems[0].description === 'connected', + 'There should be a connection tree item with the description "connected"' + ); + assert( + connectionsItems[0].isExpanded, + 'Expected the connection tree item to be expanded' + ); }); test('only the active connection is displayed as connected in the tree', async () => { @@ -173,32 +173,28 @@ suite('Explorer Controller Test Suite', function () { mdbTestExtension.testExtensionController._explorerController; const treeController = testExplorerController.getTreeController(); - try { - const succesfullyConnected = await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); + const succesfullyConnected = await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); - assert( - succesfullyConnected === true, - 'Expected a successful connection response.' - ); - assert( - Object.keys(testConnectionController._connections).length === 1, - 'Expected there to be 1 connection in the connection list.' - ); + assert( + succesfullyConnected === true, + 'Expected a successful connection response.' + ); + assert( + Object.keys(testConnectionController._connections).length === 1, + 'Expected there to be 1 connection in the connection list.' + ); - const connectionId = - testConnectionController.getActiveConnectionId() || ''; - const connectionName = - testConnectionController._connections[connectionId].name; + const connectionId = + testConnectionController.getActiveConnectionId() || ''; + const connectionName = + testConnectionController._connections[connectionId].name; - assert( - connectionName === 'localhost:27018', - `Expected active connection name to be 'localhost:27018' found ${connectionName}` - ); - } catch (error) { - assert(false, error); - } + assert( + connectionName === 'localhost:27018', + `Expected active connection name to be 'localhost:27018' found ${connectionName}` + ); try { await testConnectionController.addNewConnectionStringAndConnect( @@ -208,31 +204,27 @@ suite('Explorer Controller Test Suite', function () { /* Silent fail (should fail) */ } - try { - const treeControllerChildren = await treeController.getChildren(); - const connectionsItems = await treeControllerChildren[0].getChildren(); + const treeControllerChildren = await treeController.getChildren(); + const connectionsItems = await treeControllerChildren[0].getChildren(); - assert( - connectionsItems.length === 2, - `Expected there be 2 connection tree item, found ${connectionsItems.length}` - ); - assert( - connectionsItems[0].label === 'localhost:27018', - `First connection tree item should have label "localhost:27018" found ${connectionsItems[0].label}` - ); - assert( - connectionsItems[0].isExpanded === false, - 'Expected the first connection tree item to not be expanded' - ); - assert( - connectionsItems[1].label === 'shouldfail:27017', - 'Second connection tree item should have label "shouldfail:27017"' - ); + assert( + connectionsItems.length === 2, + `Expected there be 2 connection tree item, found ${connectionsItems.length}` + ); + assert( + connectionsItems[0].label === 'localhost:27018', + `First connection tree item should have label "localhost:27018" found ${connectionsItems[0].label}` + ); + assert( + connectionsItems[0].isExpanded === false, + 'Expected the first connection tree item to not be expanded' + ); + assert( + connectionsItems[1].label === 'shouldfail:27017', + 'Second connection tree item should have label "shouldfail:27017"' + ); - testExplorerController.deactivate(); - } catch (error) { - assert(false, error); - } + testExplorerController.deactivate(); }); test('shows connection names sorted alphabetically in the tree', async () => { @@ -270,35 +262,31 @@ suite('Explorer Controller Test Suite', function () { storageLocation: StorageScope.WORKSPACE }; - try { - const treeControllerChildren = await treeController.getChildren(); - const connectionsItems = await treeControllerChildren[0].getChildren(); + const treeControllerChildren = await treeController.getChildren(); + const connectionsItems = await treeControllerChildren[0].getChildren(); - assert( - connectionsItems.length === 3, - `Expected there be 3 connection tree item, found ${connectionsItems.length}` - ); - assert( - connectionsItems[0].label === 'aaa', - `First connection tree item should have label "aaa" found ${connectionsItems[0].label}` - ); - assert( - connectionsItems[2].label === 'zzz', - `First connection tree item should have label "zzz" found ${connectionsItems[0].label}` - ); + assert( + connectionsItems.length === 3, + `Expected there be 3 connection tree item, found ${connectionsItems.length}` + ); + assert( + connectionsItems[0].label === 'aaa', + `First connection tree item should have label "aaa" found ${connectionsItems[0].label}` + ); + assert( + connectionsItems[2].label === 'zzz', + `First connection tree item should have label "zzz" found ${connectionsItems[0].label}` + ); - testConnectionController._connections.zzz.name = '111'; + testConnectionController._connections.zzz.name = '111'; - const afterAdditionConnectionsItems = await treeControllerChildren[0].getChildren(); - assert( - afterAdditionConnectionsItems[0].label === '111', - `First connection tree item should have label "111" found ${afterAdditionConnectionsItems[0].label}` - ); + const afterAdditionConnectionsItems = await treeControllerChildren[0].getChildren(); + assert( + afterAdditionConnectionsItems[0].label === '111', + `First connection tree item should have label "111" found ${afterAdditionConnectionsItems[0].label}` + ); - testExplorerController.deactivate(); - } catch (error) { - assert(false, error); - } + testExplorerController.deactivate(); }); test('shows the databases of connected connection in tree', async () => { @@ -308,32 +296,28 @@ suite('Explorer Controller Test Suite', function () { mdbTestExtension.testExtensionController._explorerController; const treeController = testExplorerController.getTreeController(); - try { - await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); + await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); - const treeControllerChildren = await treeController.getChildren(); - const connectionsItems = await treeControllerChildren[0].getChildren(); + const treeControllerChildren = await treeController.getChildren(); + const connectionsItems = await treeControllerChildren[0].getChildren(); - // Expand the connection. - treeControllerChildren[0].onDidExpand(); + // Expand the connection. + treeControllerChildren[0].onDidExpand(); - const databaseItems = await connectionsItems[0].getChildren(); + const databaseItems = await connectionsItems[0].getChildren(); - assert( - databaseItems.length >= 3, - `Expected there be 3 or more database tree items, found ${databaseItems.length}` - ); - assert( - databaseItems[0].label === 'admin', - `First database tree item should have label "admin" found ${connectionsItems[0].label}.` - ); + assert( + databaseItems.length >= 3, + `Expected there be 3 or more database tree items, found ${databaseItems.length}` + ); + assert( + databaseItems[0].label === 'admin', + `First database tree item should have label "admin" found ${connectionsItems[0].label}.` + ); - testExplorerController.deactivate(); - } catch (error) { - assert(false, error); - } + testExplorerController.deactivate(); }); test('caches the expanded state of databases in the tree when a connection is expanded or collapsed', async () => { @@ -343,58 +327,54 @@ suite('Explorer Controller Test Suite', function () { mdbTestExtension.testExtensionController._explorerController; const treeController = testExplorerController.getTreeController(); - try { - await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); + await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); - const rootTreeItem = await treeController.getChildren(); - const connectionsTreeItem = rootTreeItem[0]; - const connectionsItems = await connectionsTreeItem.getChildren(); + const rootTreeItem = await treeController.getChildren(); + const connectionsTreeItem = rootTreeItem[0]; + const connectionsItems = await connectionsTreeItem.getChildren(); - // Expand the connection. - const testConnectionTreeItem = connectionsItems[0]; + // Expand the connection. + const testConnectionTreeItem = connectionsItems[0]; - await testConnectionTreeItem.onDidExpand(); + await testConnectionTreeItem.onDidExpand(); - const databaseItems = await testConnectionTreeItem.getChildren(); + const databaseItems = await testConnectionTreeItem.getChildren(); - assert( - databaseItems[1].isExpanded === false, - 'Expected database tree item not to be expanded on default.' - ); + assert( + databaseItems[1].isExpanded === false, + 'Expected database tree item not to be expanded on default.' + ); - // Expand the first database item. - await databaseItems[1].onDidExpand(); + // Expand the first database item. + await databaseItems[1].onDidExpand(); - assert( - databaseItems[1].isExpanded === true, - 'Expected database tree item be expanded.' - ); + assert( + databaseItems[1].isExpanded === true, + 'Expected database tree item be expanded.' + ); - // Collapse the connection. - testConnectionTreeItem.onDidCollapse(); + // Collapse the connection. + testConnectionTreeItem.onDidCollapse(); - const databaseTreeItems = await testConnectionTreeItem.getChildren(); + const databaseTreeItems = await testConnectionTreeItem.getChildren(); - assert( - databaseTreeItems.length === 0, - `Expected the connection tree to return no children when collapsed, found ${databaseTreeItems.length}` - ); + assert( + databaseTreeItems.length === 0, + `Expected the connection tree to return no children when collapsed, found ${databaseTreeItems.length}` + ); - testConnectionTreeItem.onDidExpand(); + testConnectionTreeItem.onDidExpand(); - const newDatabaseItems = await testConnectionTreeItem.getChildren(); + const newDatabaseItems = await testConnectionTreeItem.getChildren(); - assert( - newDatabaseItems[1].isExpanded === true, - 'Expected database tree to be expanded from cache.' - ); + assert( + newDatabaseItems[1].isExpanded === true, + 'Expected database tree to be expanded from cache.' + ); - testExplorerController.deactivate(); - } catch (error) { - assert(false, error); - } + testExplorerController.deactivate(); }); test('tree view should be not created by default (shows welcome view)', () => { @@ -424,15 +404,11 @@ suite('Explorer Controller Test Suite', function () { sinon.replace(vscode.window, 'createTreeView', vscodeCreateTreeViewStub); - try { - await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); - await testConnectionController.disconnect(); + await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); + await testConnectionController.disconnect(); - assert(vscodeCreateTreeViewStub.called); - } catch (error) { - assert(false, error); - } + assert(vscodeCreateTreeViewStub.called); }); }); diff --git a/src/test/suite/mdbExtensionController.test.ts b/src/test/suite/mdbExtensionController.test.ts index 2788b0083..c37d05c3b 100644 --- a/src/test/suite/mdbExtensionController.test.ts +++ b/src/test/suite/mdbExtensionController.test.ts @@ -14,7 +14,6 @@ import { } from '../../explorer'; import { mdbTestExtension } from './stubbableMdbExtension'; import { StorageScope } from '../../storage/storageController'; -import DocumentListTreeItem from '../../explorer/documentListTreeItem'; const sinon = require('sinon'); const testDatabaseURI = 'mongodb://localhost:27018'; From fcc0199501b2ffa46944166ab90db8ab5c963644 Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Tue, 21 Jul 2020 18:55:35 +0200 Subject: [PATCH 5/6] Add comment for formatting document count --- src/explorer/documentListTreeItem.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/explorer/documentListTreeItem.ts b/src/explorer/documentListTreeItem.ts index bc891deb1..a61ac9d6e 100644 --- a/src/explorer/documentListTreeItem.ts +++ b/src/explorer/documentListTreeItem.ts @@ -52,6 +52,7 @@ const getCollapsableStateForDocumentList = ( }; export const formatDocCount = (count: number): string => { + // We format the count (30000 -> 30k) and then display it uppercase (30K). return `${numeral(count).format('0a')}`.toUpperCase(); }; From 2d4b6e554833e91fd9d024b904c25c01974c8d06 Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Tue, 21 Jul 2020 19:33:28 +0200 Subject: [PATCH 6/6] Add updated timeout and tweak tests --- .../suite/explorer/explorerController.test.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/test/suite/explorer/explorerController.test.ts b/src/test/suite/explorer/explorerController.test.ts index 9774318d3..2589ed001 100644 --- a/src/test/suite/explorer/explorerController.test.ts +++ b/src/test/suite/explorer/explorerController.test.ts @@ -6,7 +6,7 @@ const sinon = require('sinon'); import { DefaultSavingLocations, - StorageScope + StorageScope, } from '../../../storage/storageController'; import { TEST_DATABASE_URI } from '../dbTestHelper'; import { mdbTestExtension } from '../stubbableMdbExtension'; @@ -15,7 +15,9 @@ const testDatabaseURI2WithTimeout = 'mongodb://shouldfail?connectTimeoutMS=500&serverSelectionTimeoutMS=500'; suite('Explorer Controller Test Suite', function () { - this.timeout(5000); + // Longer timeout, sometimes it takes a few seconds for vscode to + // load the extension before running tests. + this.timeout(10000); beforeEach(async () => { // Don't save connections on default. @@ -82,8 +84,8 @@ suite('Explorer Controller Test Suite', function () { connectionModel: new Connection(), name: 'testConnectionName', driverUrl: 'url', - storageLocation: StorageScope.NONE - } + storageLocation: StorageScope.NONE, + }, }; testConnectionController.setConnnectingConnectionId(mockConnectionId); testConnectionController.setConnnecting(true); @@ -186,8 +188,7 @@ suite('Explorer Controller Test Suite', function () { 'Expected there to be 1 connection in the connection list.' ); - const connectionId = - testConnectionController.getActiveConnectionId() || ''; + const connectionId = testConnectionController.getActiveConnectionId() || ''; const connectionName = testConnectionController._connections[connectionId].name; @@ -234,13 +235,9 @@ suite('Explorer Controller Test Suite', function () { mdbTestExtension.testExtensionController._explorerController; const treeController = testExplorerController.getTreeController(); - try { - await testConnectionController.addNewConnectionStringAndConnect( - TEST_DATABASE_URI - ); - } catch (error) { - assert(false, error); - } + await testConnectionController.addNewConnectionStringAndConnect( + TEST_DATABASE_URI + ); const connectionId = testConnectionController.getActiveConnectionId() || ''; @@ -250,7 +247,7 @@ suite('Explorer Controller Test Suite', function () { driverUrl: '', name: 'aaa', id: 'aaa', - storageLocation: StorageScope.WORKSPACE + storageLocation: StorageScope.WORKSPACE, }; testConnectionController._connections.zzz = { @@ -259,7 +256,7 @@ suite('Explorer Controller Test Suite', function () { driverUrl: '', name: 'zzz', id: 'zzz', - storageLocation: StorageScope.WORKSPACE + storageLocation: StorageScope.WORKSPACE, }; const treeControllerChildren = await treeController.getChildren();