From 5ea1d8b2aa7f35a81e46deb0833e09c003b1b70c Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 6 Feb 2019 16:45:11 -0600 Subject: [PATCH] Bit of cleanup, some test fixes, moving some funcs --- packages/ide/src/fill/electron.ts | 6 +- packages/ide/src/fill/net.ts | 1 - packages/ide/src/upload.ts | 6 +- packages/logger/src/logger.ts | 110 +++++++++--------- packages/protocol/src/browser/client.ts | 72 ++++++------ packages/protocol/test/command.test.ts | 147 +++++++++++++----------- 6 files changed, 180 insertions(+), 162 deletions(-) diff --git a/packages/ide/src/fill/electron.ts b/packages/ide/src/fill/electron.ts index 8ff64aea5..e016e21ab 100644 --- a/packages/ide/src/fill/electron.ts +++ b/packages/ide/src/fill/electron.ts @@ -13,8 +13,10 @@ import { clipboard } from "./clipboard"; return []; }; +// This is required to make the fill load in Node without erroring. if (typeof document === "undefined") { - (global).document = {} as any; + // tslint:disable-next-line no-any + (global as any).document = {} as any; } const oldCreateElement: ( @@ -52,7 +54,7 @@ const newCreateElement = (tagName: K): HT if (view.contentDocument) { view.contentDocument.body.id = frameID; view.contentDocument.body.parentElement!.style.overflow = "hidden"; - const script = document.createElement("script"); + const script = createElement("script"); script.src = url; view.contentDocument.head.appendChild(script); } diff --git a/packages/ide/src/fill/net.ts b/packages/ide/src/fill/net.ts index 26b267dae..92af470bb 100644 --- a/packages/ide/src/fill/net.ts +++ b/packages/ide/src/fill/net.ts @@ -13,7 +13,6 @@ class Net implements NodeNet { ) {} public get Socket(): typeof net.Socket { - // @ts-ignore return this.client.Socket; } diff --git a/packages/ide/src/upload.ts b/packages/ide/src/upload.ts index b088d17bc..abb70fdc3 100644 --- a/packages/ide/src/upload.ts +++ b/packages/ide/src/upload.ts @@ -221,7 +221,7 @@ export class Upload { await rm(); - reader.addEventListener("load", async () => { + const load = async (): Promise => { const buffer = new Uint8Array(reader.result as ArrayBuffer); let bufferOffset = 0; @@ -259,7 +259,9 @@ export class Upload { } seek(); - }); + }; + + reader.addEventListener("load", load); seek(); }); diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts index f56e62904..0564ff39f 100644 --- a/packages/logger/src/logger.ts +++ b/packages/logger/src/logger.ts @@ -56,57 +56,6 @@ export const field = (name: string, value: T): Field => { return new Field(name, value); }; -/** - * Hashes a string. - */ -const djb2 = (str: string): number => { - let hash = 5381; - for (let i = 0; i < str.length; i++) { - hash = ((hash << 5) + hash) + str.charCodeAt(i); /* hash * 33 + c */ - } - - return hash; -}; - -/** - * Convert rgb to hex. - */ -const rgbToHex = (r: number, g: number, b: number): string => { - const integer = ((Math.round(r) & 0xFF) << 16) - + ((Math.round(g) & 0xFF) << 8) - + (Math.round(b) & 0xFF); - - const str = integer.toString(16); - - return "#" + "000000".substring(str.length) + str; -}; - -/** - * Convert fully-formed hex to rgb. - */ -const hexToRgb = (hex: string): [number, number, number] => { - const integer = parseInt(hex.substr(1), 16); - - return [ - (integer >> 16) & 0xFF, - (integer >> 8) & 0xFF, - integer & 0xFF, - ]; -}; - -/** - * Generates a deterministic color from a string using hashing. - */ -const hashStringToColor = (str: string): string => { - const hash = djb2(str); - - return rgbToHex( - (hash & 0xFF0000) >> 16, - (hash & 0x00FF00) >> 8, - hash & 0x0000FF, - ); -}; - /** * This formats & builds text for logging. * It should only be used to build one log item at a time since it stores the @@ -207,7 +156,7 @@ export class BrowserFormatter extends Formatter { */ export class ServerFormatter extends Formatter { public tag(name: string, color: string): void { - const [r, g, b] = hexToRgb(color); + const [r, g, b] = this.hexToRgb(color); while (name.length < 5) { name += " "; } @@ -220,7 +169,7 @@ export class ServerFormatter extends Formatter { this.format += "\u001B[1m"; } if (color) { - const [r, g, b] = hexToRgb(color); + const [r, g, b] = this.hexToRgb(color); this.format += `\u001B[38;2;${r};${g};${b}m`; } this.format += this.getType(arg); @@ -241,6 +190,19 @@ export class ServerFormatter extends Formatter { this.args.push(JSON.stringify(obj)); console.log(...this.flush()); // tslint:disable-line no-console } + + /** + * Convert fully-formed hex to rgb. + */ + private hexToRgb(hex: string): [number, number, number] { + const integer = parseInt(hex.substr(1), 16); + + return [ + (integer >> 16) & 0xFF, + (integer >> 8) & 0xFF, + integer & 0xFF, + ]; + } } /** @@ -258,7 +220,7 @@ export class Logger { private readonly defaultFields?: FieldArray, ) { if (name) { - this.nameColor = hashStringToColor(name); + this.nameColor = this.hashStringToColor(name); } const envLevel = typeof global !== "undefined" && typeof global.process !== "undefined" ? global.process.env.LOG_LEVEL : process.env.LOG_LEVEL; if (envLevel) { @@ -401,7 +363,7 @@ export class Logger { const green = expPer < 1 ? max : min; const red = expPer >= 1 ? max : min; this._formatter.push(` ${time.identifier}=`, "#3794ff"); - this._formatter.push(`${diff}ms`, rgbToHex(red > 0 ? red : 0, green > 0 ? green : 0, 0)); + this._formatter.push(`${diff}ms`, this.rgbToHex(red > 0 ? red : 0, green > 0 ? green : 0, 0)); }); } @@ -413,6 +375,44 @@ export class Logger { } // tslint:enable no-console } + + /** + * Hashes a string. + */ + private djb2(str: string): number { + let hash = 5381; + for (let i = 0; i < str.length; i++) { + hash = ((hash << 5) + hash) + str.charCodeAt(i); /* hash * 33 + c */ + } + + return hash; + } + + /** + * Convert rgb to hex. + */ + private rgbToHex(r: number, g: number, b: number): string { + const integer = ((Math.round(r) & 0xFF) << 16) + + ((Math.round(g) & 0xFF) << 8) + + (Math.round(b) & 0xFF); + + const str = integer.toString(16); + + return "#" + "000000".substring(str.length) + str; + } + + /** + * Generates a deterministic color from a string using hashing. + */ + private hashStringToColor(str: string): string { + const hash = this.djb2(str); + + return this.rgbToHex( + (hash & 0xFF0000) >> 16, + (hash & 0x00FF00) >> 8, + hash & 0x0000FF, + ); + } } export const logger = new Logger( diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 43ccc1f8c..9fb8a6e6d 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -1,15 +1,16 @@ import { ReadWriteConnection, InitData, OperatingSystem, ISharedProcessData } from "../common/connection"; import { NewEvalMessage, ServerMessage, EvalDoneMessage, EvalFailedMessage, TypedValue, ClientMessage, NewSessionMessage, TTYDimensions, SessionOutputMessage, CloseSessionInputMessage, WorkingInitMessage, EvalEventMessage } from "../proto"; -import { Emitter, Event } from "@coder/events"; +import { Emitter } from "@coder/events"; import { logger, field } from "@coder/logger"; import { ChildProcess, SpawnOptions, ForkOptions, ServerProcess, ServerSocket, Socket, ServerListener, Server, ActiveEval } from "./command"; import { EventEmitter } from "events"; +import { Socket as NetSocket } from "net"; /** * Client accepts an arbitrary connection intended to communicate with the Server. */ export class Client { - public readonly Socket: typeof ServerSocket; + public readonly Socket: typeof NetSocket; private evalId = 0; private readonly evalDoneEmitter = new Emitter(); @@ -47,12 +48,11 @@ export class Client { }); const that = this; + // @ts-ignore NOTE: this doesn't fully implement net.Socket. this.Socket = class extends ServerSocket { - public constructor() { super(that.connection, that.connectionId++, that.registerConnection); } - }; this.initDataPromise = new Promise((resolve): void => { @@ -151,39 +151,39 @@ export class Client { }); const d1 = this.evalDoneEmitter.event((doneMsg) => { - if (doneMsg.getId() === id) { - d1.dispose(); - d2.dispose(); - - const resp = doneMsg.getResponse(); - if (!resp) { - res(); - - return; - } - - const rt = resp.getType(); - // tslint:disable-next-line - let val: any; - switch (rt) { - case TypedValue.Type.BOOLEAN: - val = resp.getValue() === "true"; - break; - case TypedValue.Type.NUMBER: - val = parseInt(resp.getValue(), 10); - break; - case TypedValue.Type.OBJECT: - val = JSON.parse(resp.getValue()); - break; - case TypedValue.Type.STRING: - val = resp.getValue(); - break; - default: - throw new Error(`unsupported typed value ${rt}`); - } - - res(val); + if (doneMsg.getId() !== id) { + return; } + + d1.dispose(); + d2.dispose(); + + const resp = doneMsg.getResponse(); + if (!resp) { + return res(); + } + + const rt = resp.getType(); + // tslint:disable-next-line no-any + let val: any; + switch (rt) { + case TypedValue.Type.BOOLEAN: + val = resp.getValue() === "true"; + break; + case TypedValue.Type.NUMBER: + val = parseInt(resp.getValue(), 10); + break; + case TypedValue.Type.OBJECT: + val = JSON.parse(resp.getValue()); + break; + case TypedValue.Type.STRING: + val = resp.getValue(); + break; + default: + throw new Error(`unsupported typed value ${rt}`); + } + + res(val); }); const d2 = this.evalFailedEmitter.event((failedMsg) => { diff --git a/packages/protocol/test/command.test.ts b/packages/protocol/test/command.test.ts index ec369c2dd..a3fffdbcf 100644 --- a/packages/protocol/test/command.test.ts +++ b/packages/protocol/test/command.test.ts @@ -4,7 +4,7 @@ import * as os from "os"; import * as path from "path"; import { TextEncoder, TextDecoder } from "text-encoding"; import { createClient } from "./helpers"; -import { Net } from "../src/browser/modules/net"; +import { ChildProcess } from "../src/browser/command"; (global as any).TextDecoder = TextDecoder; // tslint:disable-line no-any (global as any).TextEncoder = TextEncoder; // tslint:disable-line no-any @@ -13,6 +13,7 @@ describe("spawn", () => { const client = createClient({ dataDirectory: "", workingDirectory: "", + builtInExtensionsDirectory: "", forkProvider: (msg): cp.ChildProcess => { return cp.spawn(msg.getCommand(), msg.getArgsList(), { stdio: [null, null, null, "ipc"], @@ -20,6 +21,37 @@ describe("spawn", () => { }, }); + /** + * Returns a function that when called returns a promise that resolves with + * the next chunk of data from the process. + */ + const promisifyData = (proc: ChildProcess): (() => Promise) => { + // Use a persistent callback instead of creating it in the promise since + // otherwise we could lose data that comes in while no promise is listening. + let onData: (() => void) | undefined; + let buffer: string | undefined; + proc.stdout.on("data", (data) => { + // Remove everything that isn't a letter, number, or $ to avoid issues + // with ANSI escape codes printing inside the test output. + buffer = (buffer || "") + data.toString().replace(/[^a-zA-Z0-9$]/g, ""); + if (onData) { + onData(); + } + }); + + return (): Promise => new Promise((resolve): void => { + onData = (): void => { + if (typeof buffer !== "undefined") { + const data = buffer; + buffer = undefined; + onData = undefined; + resolve(data); + } + }; + onData(); + }); + }; + it("should execute command and return output", (done) => { const proc = client.spawn("echo", ["test"]); proc.stdout.on("data", (data) => { @@ -30,25 +62,30 @@ describe("spawn", () => { }); }); - it("should create shell", (done) => { - const proc = client.spawn("/bin/bash", [], { + it("should create shell", async () => { + // Setting the config file to something that shouldn't exist so the test + // isn't affected by custom configuration. + const proc = client.spawn("/bin/bash", ["--rcfile", "/tmp/test/nope/should/not/exist"], { tty: { columns: 100, rows: 10, }, }); - let first = true; - proc.stdout.on("data", (data) => { - if (first) { - // First piece of data is a welcome msg. Second is the prompt - first = false; - return; - } - expect(data.toString().endsWith("$ ")).toBeTruthy(); - proc.kill(); + const getData = promisifyData(proc); + + // First it outputs @hostname:cwd + expect((await getData()).length).toBeGreaterThan(1); + + // Then it seems to overwrite that with a shorter prompt in the format of + // [hostname@user]$ + expect((await getData())).toContain("$"); + + proc.kill(); + + await new Promise((resolve): void => { + proc.on("exit", resolve); }); - proc.on("exit", () => done()); }); it("should cat", (done) => { @@ -74,68 +111,46 @@ describe("spawn", () => { }); }); - it("should resize", (done) => { - // Requires the `tput lines` cmd to be available - - const proc = client.spawn("/bin/bash", [], { + it("should resize", async () => { + // Requires the `tput lines` cmd to be available. + // Setting the config file to something that shouldn't exist so the test + // isn't affected by custom configuration. + const proc = client.spawn("/bin/bash", ["--rcfile", "/tmp/test/nope/should/not/exist"], { tty: { columns: 10, rows: 10, }, }); - let output: number = 0; // Number of outputs parsed - proc.stdout.on("data", (data) => { - output++; - if (output === 1) { - // First is welcome msg - return; - } + const getData = promisifyData(proc); - if (output === 2) { - proc.send("tput lines\n"); + // We've already tested these first two bits of output; see shell test. + await getData(); + await getData(); - return; - } + proc.send("tput lines\n"); + expect(await getData()).toContain("tput"); - if (output === 3) { - // Echo of tput lines - return; - } - - if (output === 4) { - expect(data.toString().trim()).toEqual("10"); - proc.resize!({ - columns: 10, - rows: 50, - }); - - return; - } - - if (output === 5) { - // Primpt - return; - } - - if (output === 6) { - proc.send("tput lines\n"); - - return; - } - - if (output === 7) { - // Echo of tput lines - return; - } - - if (output === 8) { - expect(data.toString().trim()).toEqual("50"); - proc.kill(); - expect(proc.killed).toBeTruthy(); - } + expect((await getData()).trim()).toContain("10"); + proc.resize!({ + columns: 10, + rows: 50, + }); + + // The prompt again. + await getData(); + await getData(); + + proc.send("tput lines\n"); + expect(await getData()).toContain("tput"); + + expect((await getData())).toContain("50"); + + proc.kill(); + expect(proc.killed).toBeTruthy(); + await new Promise((resolve): void => { + proc.on("exit", resolve); }); - proc.on("exit", () => done()); }); it("should fork and echo messages", (done) => { @@ -176,7 +191,7 @@ describe("createConnection", () => { }); await new Promise((resolve): void => { - const socket = new (new Net(client)).Socket(); + const socket = new client.Socket(); socket.connect(tmpPath, () => { socket.end(); socket.addListener("close", () => {