Fix book provider load to not throw on startup (#8835)

* Fix book provider load to not throw on startup

* Move tests to stable

* Rename method

* Fix floating promises and broken test
This commit is contained in:
Charles Gagnon
2020-01-07 17:32:51 -08:00
committed by GitHub
parent b1526603cc
commit 041f34beda
4 changed files with 153 additions and 115 deletions

View File

@@ -41,28 +41,23 @@ export class BookModel implements azdata.nb.NavigationProvider {
return this._allNotebooks;
}
public async getTableOfContentFiles(workspacePath: string): Promise<void> {
try {
let notebookConfig = vscode.workspace.getConfiguration(notebookConfigKey);
let maxDepth = notebookConfig[maxBookSearchDepth];
// Use default value if user enters an invalid value
if (isNullOrUndefined(maxDepth) || maxDepth < 0) {
maxDepth = 5;
} else if (maxDepth === 0) { // No limit of search depth if user enters 0
maxDepth = undefined;
}
public async getTableOfContentFiles(folderPath: string): Promise<void> {
let notebookConfig = vscode.workspace.getConfiguration(notebookConfigKey);
let maxDepth = notebookConfig[maxBookSearchDepth];
// Use default value if user enters an invalid value
if (isNullOrUndefined(maxDepth) || maxDepth < 0) {
maxDepth = 5;
} else if (maxDepth === 0) { // No limit of search depth if user enters 0
maxDepth = undefined;
}
let p = path.join(workspacePath, '**', '_data', 'toc.yml').replace(/\\/g, '/');
let tableOfContentPaths = await glob(p, { deep: maxDepth });
if (tableOfContentPaths.length > 0) {
this._tableOfContentPaths = this._tableOfContentPaths.concat(tableOfContentPaths);
vscode.commands.executeCommand('setContext', 'bookOpened', true);
} else {
vscode.window.showErrorMessage(loc.errBookInitialize);
}
} catch (error) {
console.log(error);
let p = path.join(folderPath, '**', '_data', 'toc.yml').replace(/\\/g, '/');
let tableOfContentPaths = await glob(p, { deep: maxDepth });
if (tableOfContentPaths.length > 0) {
this._tableOfContentPaths = this._tableOfContentPaths.concat(tableOfContentPaths);
vscode.commands.executeCommand('setContext', 'bookOpened', true);
} else {
throw new Error(loc.missingTocError);
}
}

View File

@@ -27,7 +27,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
// For testing
private _errorMessage: string;
private _onReadAllTOCFiles: vscode.EventEmitter<void> = new vscode.EventEmitter<void>();
private _openAsUntitled: boolean;
public viewId: string;
public books: BookModel[];
@@ -37,29 +36,24 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
this._openAsUntitled = openAsUntitled;
this._extensionContext = extensionContext;
this.books = [];
this.initialize(workspaceFolders.map(a => a.uri.fsPath)).catch(e => console.error(e));
this.initialize(workspaceFolders).catch(e => console.error(e));
this.viewId = view;
this.prompter = new CodeAdapter();
}
private async initialize(bookPaths: string[]): Promise<void> {
private async initialize(workspaceFolders: vscode.WorkspaceFolder[]): Promise<void> {
await vscode.commands.executeCommand('setContext', 'unsavedBooks', this._openAsUntitled);
await Promise.all(bookPaths.map(async (bookPath) => {
let book: BookModel = new BookModel(bookPath, this._openAsUntitled, this._extensionContext);
await book.initializeContents();
this.books.push(book);
if (!this.currentBook) {
this.currentBook = book;
await Promise.all(workspaceFolders.map(async (workspaceFolder) => {
try {
await this.createAndAddBookModel(workspaceFolder.uri.fsPath);
} catch {
// no-op, not all workspace folders are going to be valid books
}
}));
this._initializeDeferred.resolve();
}
public get onReadAllTOCFiles(): vscode.Event<void> {
return this._onReadAllTOCFiles.event;
}
public get initialized(): Promise<void> {
return this._initializeDeferred.promise;
}
@@ -73,7 +67,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
await this.showPreviewFile(urlToOpen);
}
else {
await this.initialize([bookPath]);
await this.createAndAddBookModel(bookPath);
let bookViewer = vscode.window.createTreeView(this.viewId, { showCollapseAll: true, treeDataProvider: this });
this.currentBook = this.books.filter(book => book.bookPath === bookPath)[0];
bookViewer.reveal(this.currentBook.bookItems[0], { expand: vscode.TreeItemCollapsibleState.Expanded, focus: true, select: true });
@@ -84,6 +78,20 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
}
/**
* Creates a model for the specified folder path and adds it to the known list of books if we
* were able to successfully parse it.
* @param bookPath The path to the book folder to create the model for
*/
private async createAndAddBookModel(bookPath: string): Promise<void> {
const book: BookModel = new BookModel(bookPath, this._openAsUntitled, this._extensionContext);
await book.initializeContents();
this.books.push(book);
if (!this.currentBook) {
this.currentBook = book;
}
}
async showPreviewFile(urlToOpen?: string): Promise<void> {
if (this.currentBook) {
const bookRoot = this.currentBook.bookItems[0];

View File

@@ -24,7 +24,7 @@ export const openNotebookCommand = localize('openNotebookCommand', "Open Noteboo
export const openMarkdownCommand = localize('openMarkdownCommand', "Open Markdown");
export const openExternalLinkCommand = localize('openExternalLinkCommand', "Open External Link");
export const errBookInitialize = localize('bookInitializeFailed', "Book initialize failed: Failed to recognize the structure.");
export const missingTocError = localize('bookInitializeFailed', "Failed to find a toc.yml.");
export function missingFileError(title: string): string { return localize('missingFileError', "Missing file : {0}", title); }
export function invalidTocFileError(error: string): string { return localize('InvalidError.tocFile', "{0}", error); }
export function invalidTocError(title: string): string { return localize('Invalid toc.yml', "Error: {0} has an incorrect toc.yml file", title); }

View File

@@ -5,7 +5,6 @@
import * as vscode from 'vscode';
import * as should from 'should';
import * as TypeMoq from 'typemoq';
import * as path from 'path';
import * as fs from 'fs-extra';
import * as rimraf from 'rimraf';
@@ -13,6 +12,7 @@ import * as os from 'os';
import * as uuid from 'uuid';
import { BookTreeViewProvider } from '../../book/bookTreeView';
import { BookTreeItem } from '../../book/bookTreeItem';
import { MockExtensionContext } from '../common/stubs';
export interface ExpectedBookItem {
title: string;
@@ -34,9 +34,13 @@ export function equalBookItems(book: BookTreeItem, expectedBook: ExpectedBookIte
}
}
describe('BookTreeViewProviderTests @UNSTABLE@', function() {
describe('BookTreeViewProviderTests', function() {
describe('BookTreeViewProvider.getChildren', function (): void {
describe('BookTreeViewProvider', () => {
let mockExtensionContext: vscode.ExtensionContext;
let nonBookFolderPath: string;
let bookFolderPath: string;
let rootFolderPath: string;
let expectedNotebook1: ExpectedBookItem;
let expectedNotebook2: ExpectedBookItem;
@@ -45,18 +49,14 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
let expectedExternalLink: ExpectedBookItem;
let expectedBook: ExpectedBookItem;
let mockExtensionContext: TypeMoq.IMock<vscode.ExtensionContext>;
let bookTreeViewProvider: BookTreeViewProvider;
let book: BookTreeItem;
let notebook1: BookTreeItem;
this.beforeAll(async () => {
console.log('Generating random rootFolderPath...');
mockExtensionContext = new MockExtensionContext();
rootFolderPath = path.join(os.tmpdir(), `BookTestData_${uuid.v4()}`);
console.log('Random rootFolderPath generated.');
let dataFolderPath = path.join(rootFolderPath, '_data');
let contentFolderPath = path.join(rootFolderPath, 'content');
let configFile = path.join(rootFolderPath, '_config.yml');
nonBookFolderPath = path.join(rootFolderPath, `NonBook`);
bookFolderPath = path.join(rootFolderPath, `Book`);
let dataFolderPath = path.join(bookFolderPath, '_data');
let contentFolderPath = path.join(bookFolderPath, 'content');
let configFile = path.join(bookFolderPath, '_config.yml');
let tableOfContentsFile = path.join(dataFolderPath, 'toc.yml');
let notebook1File = path.join(contentFolderPath, 'notebook1.ipynb');
let notebook2File = path.join(contentFolderPath, 'notebook2.ipynb');
@@ -93,8 +93,9 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
sections: [expectedNotebook1, expectedMarkdown, expectedExternalLink],
title: 'Test Book'
};
console.log('Creating temporary folders and files...');
await fs.mkdir(rootFolderPath);
await fs.mkdir(bookFolderPath);
await fs.mkdir(nonBookFolderPath);
await fs.mkdir(dataFolderPath);
await fs.mkdir(contentFolderPath);
await fs.writeFile(configFile, 'title: Test Book');
@@ -103,58 +104,94 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
await fs.writeFile(notebook2File, '');
await fs.writeFile(notebook3File, '');
await fs.writeFile(markdownFile, '');
console.log('Temporary folders and files created.');
mockExtensionContext = TypeMoq.Mock.ofType<vscode.ExtensionContext>();
});
it('should initialize correctly with empty workspace array', async () => {
const bookTreeViewProvider = new BookTreeViewProvider([], mockExtensionContext, false, 'bookTreeView');
await bookTreeViewProvider.initialized;
});
it('should initialize correctly with workspace containing non-book path', async () => {
let folder: vscode.WorkspaceFolder = {
uri: vscode.Uri.file(rootFolderPath),
uri: vscode.Uri.file(nonBookFolderPath),
name: '',
index: 0
};
console.log('Creating BookTreeViewProvider...');
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext.object, false, 'bookTreeView');
let tocRead = new Promise((resolve, reject) => bookTreeViewProvider.onReadAllTOCFiles(() => resolve()));
let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000));
await Promise.race([tocRead, errorCase.then(() => { throw new Error('Table of Contents were not ready in time'); })]);
console.log('BookTreeViewProvider successfully created.');
const bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext, false, 'bookTreeView');
await bookTreeViewProvider.initialized;
});
it('should return all book nodes when element is undefined', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren();
should(children).be.Array();
should(children.length).equal(1);
book = children[0];
should(book.title).equal(expectedBook.title);
it('should initialize correctly with workspace containing both book and non-book paths', async () => {
const book: vscode.WorkspaceFolder = {
uri: vscode.Uri.file(bookFolderPath),
name: '',
index: 0
};
const nonBook: vscode.WorkspaceFolder = {
uri: vscode.Uri.file(nonBookFolderPath),
name: '',
index: 0
};
const bookTreeViewProvider = new BookTreeViewProvider([book, nonBook], mockExtensionContext, false, 'bookTreeView');
await bookTreeViewProvider.initialized;
should(bookTreeViewProvider.books.length).equal(1, 'Expected book was not initialized');
});
it('should return all page nodes when element is a book', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren(book);
should(children).be.Array();
should(children.length).equal(3);
notebook1 = children[0];
const markdown = children[1];
const externalLink = children[2];
equalBookItems(notebook1, expectedNotebook1);
equalBookItems(markdown, expectedMarkdown);
equalBookItems(externalLink, expectedExternalLink);
describe('BookTreeViewProvider.getChildren', function (): void {
let bookTreeViewProvider: BookTreeViewProvider;
let book: BookTreeItem;
let notebook1: BookTreeItem;
this.beforeAll(async () => {
let folder: vscode.WorkspaceFolder = {
uri: vscode.Uri.file(rootFolderPath),
name: '',
index: 0
};
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext, false, 'bookTreeView');
let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000));
await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]);
});
it('should return all book nodes when element is undefined', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren();
should(children).be.Array();
should(children.length).equal(1);
book = children[0];
should(book.title).equal(expectedBook.title);
});
it('should return all page nodes when element is a book', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren(book);
should(children).be.Array();
should(children.length).equal(3);
notebook1 = children[0];
const markdown = children[1];
const externalLink = children[2];
equalBookItems(notebook1, expectedNotebook1);
equalBookItems(markdown, expectedMarkdown);
equalBookItems(externalLink, expectedExternalLink);
});
it('should return all sections when element is a notebook', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren(notebook1);
should(children).be.Array();
should(children.length).equal(2);
const notebook2 = children[0];
const notebook3 = children[1];
equalBookItems(notebook2, expectedNotebook2);
equalBookItems(notebook3, expectedNotebook3);
});
this.afterAll(async function () {
console.log('Removing temporary files...');
if (fs.existsSync(rootFolderPath)) {
rimraf.sync(rootFolderPath);
}
console.log('Successfully removed temporary files.');
});
});
it('should return all sections when element is a notebook', async function (): Promise<void> {
const children = await bookTreeViewProvider.getChildren(notebook1);
should(children).be.Array();
should(children.length).equal(2);
const notebook2 = children[0];
const notebook3 = children[1];
equalBookItems(notebook2, expectedNotebook2);
equalBookItems(notebook3, expectedNotebook3);
});
this.afterAll(async function () {
console.log('Removing temporary files...');
if (fs.existsSync(rootFolderPath)) {
rimraf.sync(rootFolderPath);
}
console.log('Successfully removed temporary files.');
});
});
describe('BookTreeViewProvider.getTableOfContentFiles', function (): void {
@@ -170,22 +207,21 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
let tableOfContentsFileIgnore = path.join(rootFolderPath, 'toc.yml');
await fs.mkdir(rootFolderPath);
await fs.mkdir(dataFolderPath);
await fs.writeFile(tableOfContentsFile, '');
await fs.writeFile(tableOfContentsFile, '- title: Notebook1\n url: /notebook1\n sections:\n - title: Notebook2\n url: /notebook2\n - title: Notebook3\n url: /notebook3\n- title: Markdown\n url: /markdown\n- title: GitHub\n url: https://github.com/\n external: true');
await fs.writeFile(tableOfContentsFileIgnore, '');
let mockExtensionContext = TypeMoq.Mock.ofType<vscode.ExtensionContext>();
const mockExtensionContext = new MockExtensionContext();
folder = {
uri: vscode.Uri.file(rootFolderPath),
name: '',
index: 0
};
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext.object, false, 'bookTreeView');
let tocRead = new Promise((resolve, reject) => bookTreeViewProvider.onReadAllTOCFiles(() => resolve()));
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext, false, 'bookTreeView');
let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000));
await Promise.race([tocRead, errorCase.then(() => { throw new Error('Table of Contents were not ready in time'); })]);
await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]);
});
it('should ignore toc.yml files not in _data folder', function(): void {
bookTreeViewProvider.currentBook.getTableOfContentFiles(folder.uri.toString());
it('should ignore toc.yml files not in _data folder', async () => {
await bookTreeViewProvider.currentBook.getTableOfContentFiles(rootFolderPath);
for (let p of bookTreeViewProvider.currentBook.tableOfContentPaths) {
should(p.toLocaleLowerCase()).equal(tableOfContentsFile.replace(/\\/g, '/').toLocaleLowerCase());
}
@@ -199,40 +235,40 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
});
describe('BookTreeViewProvider.getBooks', function (): void {
describe('BookTreeViewProvider.getBooks @UNSTABLE@', function (): void {
let rootFolderPath: string;
let configFile: string;
let folder: vscode.WorkspaceFolder;
let bookTreeViewProvider: BookTreeViewProvider;
let mockExtensionContext: TypeMoq.IMock<vscode.ExtensionContext>;
let tocFile: string;
this.beforeAll(async () => {
rootFolderPath = path.join(os.tmpdir(), `BookTestData_${uuid.v4()}`);
let dataFolderPath = path.join(rootFolderPath, '_data');
configFile = path.join(rootFolderPath, '_config.yml');
let tableOfContentsFile = path.join(dataFolderPath, 'toc.yml');
tocFile = path.join(dataFolderPath, 'toc.yml');
await fs.mkdir(rootFolderPath);
await fs.mkdir(dataFolderPath);
await fs.writeFile(tableOfContentsFile, 'title: Test');
mockExtensionContext = TypeMoq.Mock.ofType<vscode.ExtensionContext>();
await fs.writeFile(tocFile, 'title: Test');
const mockExtensionContext = new MockExtensionContext();
folder = {
uri: vscode.Uri.file(rootFolderPath),
name: '',
index: 0
};
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext.object, false, 'bookTreeView');
let tocRead = new Promise((resolve, reject) => bookTreeViewProvider.onReadAllTOCFiles(() => resolve()));
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext, false, 'bookTreeView');
let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000));
await Promise.race([tocRead, errorCase.then(() => { throw new Error('Table of Contents were not ready in time'); })]);
await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]);
});
it('should show error message if config.yml file not found', function(): void {
bookTreeViewProvider.currentBook.readBooks();
it('should show error message if config.yml file not found', async () => {
await bookTreeViewProvider.currentBook.readBooks();
should(bookTreeViewProvider.errorMessage.toLocaleLowerCase()).equal(('ENOENT: no such file or directory, open \'' + configFile + '\'').toLocaleLowerCase());
});
it('should show error if toc.yml file format is invalid', async function(): Promise<void> {
await fs.writeFile(configFile, 'title: Test Book');
bookTreeViewProvider.currentBook.readBooks();
await bookTreeViewProvider.currentBook.readBooks();
should(bookTreeViewProvider.errorMessage).equal('Error: Test Book has an incorrect toc.yml file');
});
@@ -244,7 +280,7 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
});
describe('BookTreeViewProvider.getSections', function (): void {
describe('BookTreeViewProvider.getSections @UNSTABLE@', function (): void {
let rootFolderPath: string;
let tableOfContentsFile: string;
let bookTreeViewProvider: BookTreeViewProvider;
@@ -271,16 +307,15 @@ describe('BookTreeViewProviderTests @UNSTABLE@', function() {
await fs.writeFile(tableOfContentsFile, '- title: Notebook1\n url: /notebook1\n- title: Notebook2\n url: /notebook2');
await fs.writeFile(notebook2File, '');
let mockExtensionContext = TypeMoq.Mock.ofType<vscode.ExtensionContext>();
const mockExtensionContext = new MockExtensionContext();
folder = {
uri: vscode.Uri.file(rootFolderPath),
name: '',
index: 0
};
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext.object, false, 'bookTreeView');
let tocRead = new Promise((resolve, reject) => bookTreeViewProvider.onReadAllTOCFiles(() => resolve()));
bookTreeViewProvider = new BookTreeViewProvider([folder], mockExtensionContext, false, 'bookTreeView');
let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000));
await Promise.race([tocRead, errorCase.then(() => { throw new Error('Table of Contents were not ready in time'); })]);
await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]);
});
it('should show error if notebook or markdown file is missing', async function(): Promise<void> {