From 22efb4f1ec615ca94e7551537c8b51492fe83160 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Sun, 31 May 2020 16:53:37 +0200 Subject: [PATCH 1/9] fix: fix multiline input --- packages/cli-repl/src/cli-repl.ts | 19 +++ packages/cli-repl/src/line-by-line-input.ts | 149 ++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 packages/cli-repl/src/line-by-line-input.ts diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index c7d37a4e9b..cee1d20184 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -24,6 +24,7 @@ import read from 'read'; import os from 'os'; import fs from 'fs'; import { redactPwd } from '.'; +import { LineByLineInput } from './line-by-line-input'; /** * Connecting text key. @@ -44,6 +45,7 @@ class CliRepl { private userId: ObjectId; private options: CliOptions; private mongoshDir: string; + private lineByLineInput: LineByLineInput; /** * Instantiate the new CLI Repl. @@ -51,6 +53,7 @@ class CliRepl { constructor(driverUri: string, driverOptions: NodeOptions, options: CliOptions) { this.options = options; this.mongoshDir = path.join(os.homedir(), '.mongodb/mongosh/'); + this.lineByLineInput = new LineByLineInput(process.stdin); this.createMongoshDir(); @@ -105,11 +108,27 @@ class CliRepl { const version = this.buildInfo.version; this.repl = repl.start({ + input: this.lineByLineInput, + output: process.stdout, prompt: '> ', writer: this.writer, completer: completer.bind(null, version), }); + const originalDisplayPrompt = this.repl.displayPrompt.bind(this.repl); + + this.repl.displayPrompt = (...args: any[]): any => { + this.lineByLineInput.nextLine(); + return originalDisplayPrompt(...args); + }; + + const originalEditorAction = this.repl.commands.editor.action.bind(this.repl); + + this.repl.commands.editor.action = (): any => { + this.lineByLineInput.disable(); + return originalEditorAction(); + }; + this.repl.defineCommand('clear', { help: '', action: () => { diff --git a/packages/cli-repl/src/line-by-line-input.ts b/packages/cli-repl/src/line-by-line-input.ts new file mode 100644 index 0000000000..95b674edac --- /dev/null +++ b/packages/cli-repl/src/line-by-line-input.ts @@ -0,0 +1,149 @@ +import { EventEmitter } from 'events'; +import { StringDecoder } from 'string_decoder'; + +const LINE_ENDING_RE = /\r?\n|\r(?!\n)/; +const CTRL_C = '\u0003'; +const CTRL_D = '\u0004'; + +/** + * A proxy for `tty.ReadStream` that allows to read + * the stream line by line. + * + * Each time a newline is encountered the stream wont emit further data + * untill `.nextLine()` is called. + * + * NOTE: the control sequences Ctrl+C and Ctrl+D are not buffered and instead + * are forwarded regardless. + * + * Is possible to disable the "line splitting" by calling `.disable()` and + * re-enable it by calling `.enable()`. + * + * If the line splitting is disabled the stream will behave like + * the proxied `tty.ReadStream`, forwarding all the characters. + */ +export class LineByLineInput { + private _emitter: EventEmitter; + private _originalInput: NodeJS.ReadStream; + private _forwarding: boolean; + private _enabled: boolean; + private _charQueue: string[]; + private _decoder: StringDecoder; + + constructor(readable: NodeJS.ReadStream) { + this._emitter = new EventEmitter(); + this._originalInput = readable; + this._forwarding = true; + this._enabled = true; + this._charQueue = []; + this._decoder = new StringDecoder('utf-8'); + + readable.on('data', this._onData); + + const proxy = new Proxy(readable, { + get: (target: NodeJS.ReadStream, property: string): any => { + if (typeof property === 'string' && + !property.startsWith('_') && + typeof this[property] === 'function' + ) { + return this[property].bind(this); + } + + return target[property]; + } + }); + + return (proxy as unknown) as LineByLineInput; + } + + on(event: string, handler: (...args: any[]) => void): void { + if (event === 'data') { + this._emitter.on('data', handler); + return; + } + + this._originalInput.on(event, handler); + return; + } + + nextLine(): void { + this._resumeForwarding(); + this._flush(); + } + + enable(): void { + this._enabled = true; + } + + disable(): void { + this._enabled = false; + this._flush(); + } + + private _onData = (chunk: Buffer): void => { + const chars = this._decoder.write(chunk); + + for (const char of chars) { + if (this._isCtrlC(char) || this._isCtrlD(char)) { + this._emitChar(char); + } else { + this._charQueue.push(char); + } + } + + this._flush(); + }; + + private _pauseForwarding(): void { + this._forwarding = false; + } + + private _resumeForwarding(): void { + this._forwarding = true; + } + + private _shouldForward(): boolean { + // If we are not splitting the input by line + // we just forward everything as is, + // otherwise we forward only if the forwarding + // is not paused. + + return !this._enabled || this._forwarding; + } + + private _emitChar(char): void { + this._emitter.emit('data', Buffer.from(char, 'utf8')); + } + + private _flush(): void { + while ( + this._charQueue.length && + this._shouldForward() && + + // We don't forward residual characters we could + // have in the buffer if in the meanwhile something + // downstream explicitly called pause(), as that may cause + // unexpected behaviors. + !this._originalInput.isPaused() + ) { + const char = this._charQueue.shift(); + + if (this._isLineEnding(char)) { + this._pauseForwarding(); + } + + this._emitChar(char); + } + } + + private _isLineEnding(char: string): boolean { + return LINE_ENDING_RE.test(char); + } + + private _isCtrlD(char: string): boolean { + return char === CTRL_D; + } + + private _isCtrlC(char: string): boolean { + return char === CTRL_C; + } +} From 9a16d7aeba0324776b8ede2648986f7117151722 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Sun, 31 May 2020 17:05:11 +0200 Subject: [PATCH 2/9] fix: readd missing enableBlockOnNewLine --- packages/cli-repl/src/cli-repl.ts | 4 +++- packages/cli-repl/src/line-by-line-input.ts | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index cee1d20184..e25440ada6 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -125,7 +125,7 @@ class CliRepl { const originalEditorAction = this.repl.commands.editor.action.bind(this.repl); this.repl.commands.editor.action = (): any => { - this.lineByLineInput.disable(); + this.lineByLineInput.disableBlockOnNewline(); return originalEditorAction(); }; @@ -139,6 +139,8 @@ class CliRepl { const originalEval = util.promisify(this.repl.eval); const customEval = async(input, context, filename, callback): Promise => { + this.lineByLineInput.enableBlockOnNewLine(); + let result; try { diff --git a/packages/cli-repl/src/line-by-line-input.ts b/packages/cli-repl/src/line-by-line-input.ts index 95b674edac..c9b95a2d3a 100644 --- a/packages/cli-repl/src/line-by-line-input.ts +++ b/packages/cli-repl/src/line-by-line-input.ts @@ -15,8 +15,8 @@ const CTRL_D = '\u0004'; * NOTE: the control sequences Ctrl+C and Ctrl+D are not buffered and instead * are forwarded regardless. * - * Is possible to disable the "line splitting" by calling `.disable()` and - * re-enable it by calling `.enable()`. + * Is possible to disable the "line splitting" by calling `.disableBlockOnNewline()` and + * re-enable it by calling `.enableBlockOnNewLine()`. * * If the line splitting is disabled the stream will behave like * the proxied `tty.ReadStream`, forwarding all the characters. @@ -25,7 +25,7 @@ export class LineByLineInput { private _emitter: EventEmitter; private _originalInput: NodeJS.ReadStream; private _forwarding: boolean; - private _enabled: boolean; + private _blockOnNewLineEnabled: boolean; private _charQueue: string[]; private _decoder: StringDecoder; @@ -33,7 +33,7 @@ export class LineByLineInput { this._emitter = new EventEmitter(); this._originalInput = readable; this._forwarding = true; - this._enabled = true; + this._blockOnNewLineEnabled = true; this._charQueue = []; this._decoder = new StringDecoder('utf-8'); @@ -70,12 +70,12 @@ export class LineByLineInput { this._flush(); } - enable(): void { - this._enabled = true; + enableBlockOnNewLine(): void { + this._blockOnNewLineEnabled = true; } - disable(): void { - this._enabled = false; + disableBlockOnNewline(): void { + this._blockOnNewLineEnabled = false; this._flush(); } @@ -102,12 +102,12 @@ export class LineByLineInput { } private _shouldForward(): boolean { - // If we are not splitting the input by line + // If we are not blocking on new lines // we just forward everything as is, // otherwise we forward only if the forwarding // is not paused. - return !this._enabled || this._forwarding; + return !this._blockOnNewLineEnabled || this._forwarding; } private _emitChar(char): void { From f2d389528f3f5a23a3f4fab73cd793d66aa709c6 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Sun, 31 May 2020 17:31:20 +0200 Subject: [PATCH 3/9] fix displayPrompt output --- packages/cli-repl/src/cli-repl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index e25440ada6..d64970d23e 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -118,8 +118,8 @@ class CliRepl { const originalDisplayPrompt = this.repl.displayPrompt.bind(this.repl); this.repl.displayPrompt = (...args: any[]): any => { + originalDisplayPrompt(...args); this.lineByLineInput.nextLine(); - return originalDisplayPrompt(...args); }; const originalEditorAction = this.repl.commands.editor.action.bind(this.repl); From 09d08d36b3a3a948ece6f604bd0e4fe0b01da17a Mon Sep 17 00:00:00 2001 From: mcasimir Date: Sun, 31 May 2020 17:34:01 +0200 Subject: [PATCH 4/9] avoid to run bootstrap on each commit --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 72ce476e44..ef1de30c5f 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,8 @@ "prebootstrap": "npm install", "bootstrap": "lerna bootstrap", "clean": "lerna clean -y && rm -Rf node_modules", - "check": "lerna bootstrap", - "check-ci": "lerna bootstrap", + "check": "echo 'check is disabled temporarily'", + "check-ci": "echo 'check is disabled temporarily'", "test": "lerna run test", "test-ci": "lerna run test-ci", "compile-ts": "lerna run compile-ts", From 17f51c06f1f1a7dabb91360dc28c98494dd6f287 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Tue, 2 Jun 2020 16:55:31 +0200 Subject: [PATCH 5/9] test: add unit tests for line-by-line-input --- package.json | 2 +- .../cli-repl/src/line-by-line-input.spec.ts | 65 +++++++++++++++++++ packages/cli-repl/src/line-by-line-input.ts | 24 +++++-- 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 packages/cli-repl/src/line-by-line-input.spec.ts diff --git a/package.json b/package.json index ef1de30c5f..4770c2444a 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "bin": "packages/cli-repl/bin/mongosh.js", "scripts": { "prebootstrap": "npm install", - "bootstrap": "lerna bootstrap", + "bootstrap": "lerna bootstrap --concurrency=1", "clean": "lerna clean -y && rm -Rf node_modules", "check": "echo 'check is disabled temporarily'", "check-ci": "echo 'check is disabled temporarily'", diff --git a/packages/cli-repl/src/line-by-line-input.spec.ts b/packages/cli-repl/src/line-by-line-input.spec.ts new file mode 100644 index 0000000000..80a3c53c4d --- /dev/null +++ b/packages/cli-repl/src/line-by-line-input.spec.ts @@ -0,0 +1,65 @@ +import { expect } from 'chai'; +import { StringDecoder } from 'string_decoder'; +import { EventEmitter } from 'events'; +import { LineByLineInput } from './line-by-line-input'; + +describe('LineByLineInput', () => { + let stdinMock: NodeJS.ReadStream; + let decoder: StringDecoder; + let forwardedChunks: string[]; + let lineByLineInput: LineByLineInput; + + beforeEach(() => { + stdinMock = new EventEmitter() as NodeJS.ReadStream; + stdinMock.isPaused = (): boolean => false; + decoder = new StringDecoder(); + forwardedChunks = []; + lineByLineInput = new LineByLineInput(stdinMock); + lineByLineInput.on('data', (chunk) => { + const decoded = decoder.write(chunk); + if (decoded) { + forwardedChunks.push(decoded); + } + }); + }); + + context('when block on newline is enabled (default)', () => { + it('does not forward characters after newline', () => { + stdinMock.emit('data', Buffer.from('ab\nc')); + expect(forwardedChunks).to.deep.equal(['a', 'b', '\n']); + }); + + it('forwards CTRL-C anyway and as soon as is received', () => { + stdinMock.emit('data', Buffer.from('\n\u0003')); + expect(forwardedChunks).to.contain('\u0003'); + }); + + it('forwards CTRL-D anyway and as soon as is received', () => { + stdinMock.emit('data', Buffer.from('\n\u0004')); + expect(forwardedChunks).to.contain('\u0004'); + }); + + it('unblocks on nextline', () => { + stdinMock.emit('data', Buffer.from('ab\nc')); + lineByLineInput.nextLine(); + expect(forwardedChunks).to.deep.equal(['a', 'b', '\n', 'c']); + }); + }); + + context('when block on newline is disabled', () => { + it('does forwards all the characters', () => { + lineByLineInput.disableBlockOnNewline(); + stdinMock.emit('data', Buffer.from('ab\nc')); + expect(forwardedChunks).to.deep.equal(['ab\nc']); + }); + }); + + context('when block on newline is disabled and re-enabled', () => { + it('does forwards all the characters', () => { + lineByLineInput.disableBlockOnNewline(); + lineByLineInput.enableBlockOnNewLine(); + stdinMock.emit('data', Buffer.from('ab\nc')); + expect(forwardedChunks).to.deep.equal(['a', 'b', '\n']); + }); + }); +}); diff --git a/packages/cli-repl/src/line-by-line-input.ts b/packages/cli-repl/src/line-by-line-input.ts index c9b95a2d3a..f8a241b9f6 100644 --- a/packages/cli-repl/src/line-by-line-input.ts +++ b/packages/cli-repl/src/line-by-line-input.ts @@ -80,8 +80,15 @@ export class LineByLineInput { } private _onData = (chunk: Buffer): void => { - const chars = this._decoder.write(chunk); + if (this._blockOnNewLineEnabled) { + return this._forwardAndBlockOnNewline(chunk); + } + + return this._forwardWithoutBlocking(chunk); + }; + private _forwardAndBlockOnNewline(chunk: Buffer): void { + const chars = this._decoder.write(chunk); for (const char of chars) { if (this._isCtrlC(char) || this._isCtrlD(char)) { this._emitChar(char); @@ -89,9 +96,14 @@ export class LineByLineInput { this._charQueue.push(char); } } - this._flush(); - }; + } + + private _forwardWithoutBlocking(chunk: Buffer): void { + // keeps decoding state consistent + this._decoder.write(chunk); + this._emitChunk(chunk); + } private _pauseForwarding(): void { this._forwarding = false; @@ -111,7 +123,11 @@ export class LineByLineInput { } private _emitChar(char): void { - this._emitter.emit('data', Buffer.from(char, 'utf8')); + this._emitChunk(Buffer.from(char, 'utf8')); + } + + private _emitChunk(chunk: Buffer): void { + this._emitter.emit('data', chunk); } private _flush(): void { From 4bab78a41f98fdc8edf3e8b135d3d240f8eb8b1b Mon Sep 17 00:00:00 2001 From: mcasimir Date: Tue, 2 Jun 2020 23:54:57 +0200 Subject: [PATCH 6/9] test: fix arg-parser tests --- packages/cli-repl/src/arg-parser.spec.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/cli-repl/src/arg-parser.spec.ts b/packages/cli-repl/src/arg-parser.spec.ts index 1b045dfce0..f932009bb5 100644 --- a/packages/cli-repl/src/arg-parser.spec.ts +++ b/packages/cli-repl/src/arg-parser.spec.ts @@ -1,5 +1,6 @@ import parse, { getLocale } from './arg-parser'; import { expect } from 'chai'; +import stripAnsi from 'strip-ansi'; const NODE = 'node'; const MONGOSH = 'mongosh'; @@ -237,8 +238,14 @@ describe('arg-parser', () => { const argv = [ ...baseArgv, uri, '--what' ]; it('raises an error', () => { - expect(parse.bind(null, argv)).to. - throw('Error parsing command line: unrecognized option: --what'); + try { + parse(argv); + throw new Error('should have thrown'); + } catch (err) { + expect( + stripAnsi(err.message) + ).to.contain('Error parsing command line: unrecognized option: --what'); + } }); }); }); @@ -731,8 +738,14 @@ describe('arg-parser', () => { const argv = [ ...baseArgv, uri, '--what' ]; it('raises an error', () => { - expect(parse.bind(null, argv)).to. - throw('Error parsing command line: unrecognized option: --what'); + try { + parse(argv); + throw new Error('should have thrown'); + } catch (err) { + expect( + stripAnsi(err.message) + ).to.contain('Error parsing command line: unrecognized option: --what'); + } }); }); }); From 3a906fd61c45157595f33038402f6a371bba2d5c Mon Sep 17 00:00:00 2001 From: mcasimir Date: Wed, 3 Jun 2020 11:55:04 +0200 Subject: [PATCH 7/9] test: fix some e2e tests --- packages/cli-repl/package.json | 2 + packages/cli-repl/src/line-by-line-input.ts | 7 + packages/cli-repl/test/e2e.spec.ts | 88 +++++-------- packages/cli-repl/test/helpers.ts | 45 +------ packages/cli-repl/test/test-shell.ts | 138 ++++++++++++++++++++ 5 files changed, 181 insertions(+), 99 deletions(-) create mode 100644 packages/cli-repl/test/test-shell.ts diff --git a/packages/cli-repl/package.json b/packages/cli-repl/package.json index fdebda7f4b..90cbc97bf9 100644 --- a/packages/cli-repl/package.json +++ b/packages/cli-repl/package.json @@ -20,6 +20,8 @@ "start-async": "node --experimental-repl-await bin/mongosh.js start --async", "test": "mocha --timeout 15000 --colors -r ts-node/register \"./{src,test}/**/*.spec.ts\"", "test-ci": "mocha --timeout 15000 -r ts-node/register \"./{src,test}/**/*.spec.ts\"", + "pretest-e2e": "npm run compile-ts", + "test-e2e": "mocha --timeout 15000 --colors -r ts-node/register \"./test/e2e.spec.ts\"", "lint": "eslint \"**/*.{js,ts,tsx}\"", "check": "npm run lint", "prepublish": "npm run compile-ts" diff --git a/packages/cli-repl/src/line-by-line-input.ts b/packages/cli-repl/src/line-by-line-input.ts index f8a241b9f6..7cff5c0580 100644 --- a/packages/cli-repl/src/line-by-line-input.ts +++ b/packages/cli-repl/src/line-by-line-input.ts @@ -58,6 +58,8 @@ export class LineByLineInput { on(event: string, handler: (...args: any[]) => void): void { if (event === 'data') { this._emitter.on('data', handler); + // we may have buffered data for the first listener + this._flush(); return; } @@ -131,6 +133,11 @@ export class LineByLineInput { } private _flush(): void { + // there is nobody to flush for + if (this._emitter.listenerCount('data') === 0) { + return; + } + while ( this._charQueue.length && this._shouldForward() && diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts index f77e884548..350d67e7aa 100644 --- a/packages/cli-repl/test/e2e.spec.ts +++ b/packages/cli-repl/test/e2e.spec.ts @@ -1,19 +1,20 @@ -import { expect } from 'chai'; import { MongoClient } from 'mongodb'; -import { eventually, startShell, killOpenShells } from './helpers'; +import { eventually } from './helpers'; +import { TestShell } from './test-shell'; describe('e2e', function() { before(require('mongodb-runner/mocha/before')({ port: 27018, timeout: 60000 })); after(require('mongodb-runner/mocha/after')({ port: 27018 })); - afterEach(() => killOpenShells()); + afterEach(() => TestShell.killall()); describe('--version', () => { it('shows version', async() => { - const shell = startShell('--version'); + const shell = TestShell.start({ args: [ '--version' ] }); + await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - expect(shell.stdio.stdout).to.contain( + shell.assertNoErrors(); + shell.assertContainsOutput( require('../package.json').version ); }); @@ -23,20 +24,23 @@ describe('e2e', function() { describe('with connection string', () => { let db; let client; - let shell; + let shell: TestShell; let dbName; beforeEach(async() => { dbName = `test-${Date.now()}`; const connectionString = `mongodb://localhost:27018/${dbName}`; + shell = TestShell.start({ args: [ connectionString ] }); - shell = startShell(connectionString); client = await (MongoClient as any).connect( connectionString, - { useNewUrlParser: true } + { useNewUrlParser: true, useUnifiedTopology: true } ); db = client.db(dbName); + + await shell.waitForPrompt(); + shell.assertNoErrors(); }); afterEach(async() => { @@ -45,68 +49,40 @@ describe('e2e', function() { client.close(); }); - it.skip('connects to the right database', async() => { - shell.stdio.stdin.write('db\n'); - - await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - expect(shell.stdio.stdout).to.contain(`> ${dbName}\n`); - }); - }); - it('throws multiline input with a single line string', async() => { // this is an unterminated string constant and should throw, since it does // not pass: https://www.ecma-international.org/ecma-262/#sec-line-terminators - shell.stdio.stdin.write('"this is a multi\nline string"\n'); - - await eventually(() => { - expect(shell.stdio.stderr).to.exist; - }); + await shell.executeLine('"this is a multi\nline string'); + shell.assertContainsError('SyntaxError: Invalid or unexpected token'); }); it('throws when a syntax error is encountered', async() => { - shell.stdio.stdin.write('\n'); - - await eventually(() => { - expect(shell.stdio.stderr).to.exist; - }); - }); - - it('does not throw for a repl await function', async() => { - shell.stdio.stdin.write('await Promise.resolve(\'Nori-cat\');'); - - await eventually(() => { - expect(shell.stdio.stderr).to.be.equal(''); - }); + await shell.executeLine(' { - shell.stdio.stdin.write('function x () {\nconsole.log(\'y\')\n }\n'); - - await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - }); + await shell.writeInputLine('function x () {\nconsole.log(\'y\')\n }'); + shell.assertNoErrors(); }); it('runs an unterminated function', async() => { - shell.stdio.stdin.write('function x () {\n'); - - await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - }); + await shell.writeInputLine('function x () {'); + shell.assertNoErrors(); }); it('runs help command', async() => { - shell.stdio.stdin.write('help\n'); + await shell.executeLine('help'); await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - expect(shell.stdio.stdout).to.contain('Shell Help'); + shell.assertContainsOutput('Shell Help'); }); + + shell.assertNoErrors(); }); it('allows to find documents', async() => { - shell.stdio.stdin.write(`use ${dbName}\n`); + await shell.writeInputLine(`use ${dbName}`); await db.collection('test').insertMany([ { doc: 1 }, @@ -114,14 +90,16 @@ describe('e2e', function() { { doc: 3 } ]); - shell.stdio.stdin.write('db.test.find()\n'); + await shell.writeInputLine('db.test.find()'); await eventually(() => { - expect(shell.stdio.stderr).to.be.empty; - expect(shell.stdio.stdout).to.contain('doc: 1'); - expect(shell.stdio.stdout).to.contain('doc: 2'); - expect(shell.stdio.stdout).to.contain('doc: 3'); + shell.assertContainsOutput('doc: 1'); + shell.assertContainsOutput('doc: 2'); + shell.assertContainsOutput('doc: 3'); }); + + shell.assertNoErrors(); }); }); }); + diff --git a/packages/cli-repl/test/helpers.ts b/packages/cli-repl/test/helpers.ts index dc427c916a..cd9f27015a 100644 --- a/packages/cli-repl/test/helpers.ts +++ b/packages/cli-repl/test/helpers.ts @@ -1,8 +1,4 @@ -import { spawn } from 'child_process'; -import path from 'path'; -import stripAnsi from 'strip-ansi'; - -export async function eventually(fn, options: { frequency?: number; timeout?: number } = {}): Promise { +export async function eventually(fn: Function, options: { frequency?: number; timeout?: number } = {}): Promise { options = { frequency: 100, timeout: 10000, @@ -28,42 +24,3 @@ export async function eventually(fn, options: { frequency?: number; timeout?: nu throw err; } -const openShells = []; - -export function startShell(...args): any { - const execPath = path.resolve(__dirname, '..', 'bin', 'mongosh.js'); - - const shell = spawn('node', [execPath, ...args], { - stdio: [ 'pipe', 'pipe', 'pipe' ] - }); - - const stdio = { - stdin: shell.stdin, - stdout: '', - stderr: '' - }; - - shell.stdout.on('data', (chunk) => { - const plainChunk = stripAnsi(chunk.toString()); - stdio.stdout += plainChunk; - }); - - shell.stderr.on('data', (chunk) => { - const plainChunk = stripAnsi(chunk.toString()); - stdio.stderr += plainChunk; - }); - - openShells.push(shell); - - return { - process: shell, - stdio, - }; -} - -export function killOpenShells(): any { - while (openShells.length) { - openShells.pop().kill(); - } -} - diff --git a/packages/cli-repl/test/test-shell.ts b/packages/cli-repl/test/test-shell.ts new file mode 100644 index 0000000000..389c54af07 --- /dev/null +++ b/packages/cli-repl/test/test-shell.ts @@ -0,0 +1,138 @@ +import { StringDecoder } from 'string_decoder'; +import { eventually } from './helpers'; + +import { spawn, ChildProcess } from 'child_process'; + +import path from 'path'; +import stripAnsi from 'strip-ansi'; +import assert from 'assert'; + +const PROMPT_PATTERN = /^> /m; +const ERROR_PATTERN = /Thrown:\n([^>]*)/m; + +/** + * Test shell helper class. + */ +export class TestShell { + private static _openShells: TestShell[] = []; + + static start(options: { args: string[] } = { args: [] }): TestShell { + const execPath = path.resolve(__dirname, '..', 'bin', 'mongosh.js'); + + const process = spawn('node', [execPath, ...options.args], { + stdio: [ 'pipe', 'pipe', 'pipe' ] + }); + + const shell = new TestShell(process); + TestShell._openShells.push(shell); + + return shell; + } + + static killall(): void { + while (TestShell._openShells.length) { + TestShell._openShells.pop().kill(); + } + } + + private _process: ChildProcess; + + private _output: string; + + constructor(shellProcess: ChildProcess) { + this._process = shellProcess; + this._output = ''; + + const stdoutDecoder = new StringDecoder(); + + shellProcess.stdout.on('data', (chunk) => { + this._output += stripAnsi(stdoutDecoder.write(chunk)); + }); + + const stderrDecoder = new StringDecoder(); + + shellProcess.stderr.on('data', (chunk) => { + this._output += stripAnsi(stderrDecoder.write(chunk)); + }); + } + + get output(): string { + return this._output; + } + + async waitForPrompt(start = 0): Promise { + await eventually(() => { + if (!this._output.slice(start).match(PROMPT_PATTERN)) { + throw new assert.AssertionError({ + message: 'expected prompt', + expected: PROMPT_PATTERN.toString(), + actual: this._output + }); + } + }); + } + + kill(): void { + this._process.kill(); + } + + writeInput(chars: string): void { + this._process.stdin.write(chars); + } + + writeInputLine(chars: string): void { + this.writeInput(`${chars}\n`); + } + + async executeLine(line: string): Promise { + const previousOutputLength = this._output.length; + this.writeInputLine(line); + await this.waitForPrompt(previousOutputLength); + return this._output.slice(previousOutputLength); + } + + assertNoErrors(): void { + const allErrors = this._getAllErrors(); + + if (allErrors.length) { + throw new assert.AssertionError({ + message: `Expected no errors in stdout but got: ${allErrors[0]}`, + expected: '', + actual: this._output + }); + } + } + + assertContainsOutput(expectedOutput: string): void { + const onlyOutputLines = this._getOutputLines(); + if (!onlyOutputLines.join('\n').includes(expectedOutput)) { + throw new assert.AssertionError({ + message: `Expected shell output to include ${JSON.stringify(expectedOutput)}`, + actual: this._output, + expected: expectedOutput + }); + } + } + + assertContainsError(expectedError: string): void { + const allErrors = this._getAllErrors(); + + if (!allErrors.find((error) => error.includes(expectedError))) { + throw new assert.AssertionError({ + message: `Expected shell errors to include ${JSON.stringify(expectedError)}`, + actual: this._output, + expected: expectedError + }); + } + } + + private _getOutputLines(): string[] { + return this._output.split('\n') + .filter((line) => !line.match(PROMPT_PATTERN)); + } + + private _getAllErrors(): string[] { + return [...(this._output as any).matchAll(ERROR_PATTERN)] + .map(m => m[1].trim()); + } +} From 1f29695bcaa9e939ce5a0809252c9193b2501447 Mon Sep 17 00:00:00 2001 From: aherlihy Date: Wed, 3 Jun 2020 14:50:03 +0200 Subject: [PATCH 8/9] .editor test fix --- packages/cli-repl/src/cli-repl.ts | 3 ++- packages/cli-repl/test/e2e.spec.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index d64970d23e..b03f9aab19 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -113,6 +113,7 @@ class CliRepl { prompt: '> ', writer: this.writer, completer: completer.bind(null, version), + terminal: true }); const originalDisplayPrompt = this.repl.displayPrompt.bind(this.repl); @@ -149,7 +150,7 @@ class CliRepl { if (isRecoverableError(input)) { return callback(new Recoverable(err)); } - result = err; + return callback(err); } callback(null, result); }; diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts index 350d67e7aa..5f63bcb3c1 100644 --- a/packages/cli-repl/test/e2e.spec.ts +++ b/packages/cli-repl/test/e2e.spec.ts @@ -53,12 +53,20 @@ describe('e2e', function() { // this is an unterminated string constant and should throw, since it does // not pass: https://www.ecma-international.org/ecma-262/#sec-line-terminators await shell.executeLine('"this is a multi\nline string'); - shell.assertContainsError('SyntaxError: Invalid or unexpected token'); + shell.assertContainsError('SyntaxError: Unterminated string constant'); }); + it('does not throw for valid input', async() => { + await shell.executeLine('1'); + shell.assertNoErrors(); + + await eventually(() => { + shell.assertContainsOutput('1'); + }); + }); it('throws when a syntax error is encountered', async() => { await shell.executeLine(' { From 132ef8439858af5fd6c70aa16bfb85173651cfe0 Mon Sep 17 00:00:00 2001 From: aherlihy Date: Wed, 3 Jun 2020 14:52:27 +0200 Subject: [PATCH 9/9] put checks back --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 4770c2444a..72ce476e44 100644 --- a/package.json +++ b/package.json @@ -5,10 +5,10 @@ "bin": "packages/cli-repl/bin/mongosh.js", "scripts": { "prebootstrap": "npm install", - "bootstrap": "lerna bootstrap --concurrency=1", + "bootstrap": "lerna bootstrap", "clean": "lerna clean -y && rm -Rf node_modules", - "check": "echo 'check is disabled temporarily'", - "check-ci": "echo 'check is disabled temporarily'", + "check": "lerna bootstrap", + "check-ci": "lerna bootstrap", "test": "lerna run test", "test-ci": "lerna run test-ci", "compile-ts": "lerna run compile-ts",