diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 9b15617d5..cacb491d5 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,3 +1,8 @@ { - "recommendations": ["Orta.vscode-jest"] + "recommendations": [ + "Orta.vscode-jest", + "esbenp.prettier-vscode", + "christian-kohler.path-intellisense", + "wayou.vscode-todo-highlight" + ] } diff --git a/changelog.md b/changelog.md index ae4bb2ac2..7daa830bb 100644 --- a/changelog.md +++ b/changelog.md @@ -7,6 +7,7 @@ Some UX fixes: - Don't show warnings about not setting a commit status (unless in verbose) - orta - Delete duplicate Danger message, due to fast Peril edits - orta - Show Peril in the commit status if inside Peril, not just Danger - orta +- [internal] Tightened the typings on the commands, and abstracted them to share some code - orta ### 2.0.0-alpha.15 diff --git a/source/ci_source/get_ci_source.ts b/source/ci_source/get_ci_source.ts index e8fc246cb..dbb3c2a2e 100644 --- a/source/ci_source/get_ci_source.ts +++ b/source/ci_source/get_ci_source.ts @@ -46,7 +46,7 @@ export function getCISourceForExternal(env: Env, modulePath: string): CISource | * @param {string} modulePath relative path to CI provider * @returns {?CISource} a CI source if module loaded successfully, undefined otherwise */ -export function getCISource(env: Env, modulePath: string): CISource | undefined { +export function getCISource(env: Env, modulePath: string | undefined): CISource | undefined { if (modulePath) { const external = getCISourceForExternal(env, modulePath) if (external) { diff --git a/source/commands/danger-pr.ts b/source/commands/danger-pr.ts index d6799eeba..27f4b35ca 100644 --- a/source/commands/danger-pr.ts +++ b/source/commands/danger-pr.ts @@ -1,7 +1,5 @@ import * as program from "commander" import * as debug from "debug" -import * as fs from "fs" -import * as repl from "repl" import * as jsome from "jsome" import { FakeCI } from "../ci_source/providers/Fake" @@ -10,16 +8,17 @@ import { GitHubAPI } from "../platforms/github/GitHubAPI" import { Executor } from "../runner/Executor" import { pullRequestParser } from "../platforms/github/pullRequestParser" import { runDangerfileEnvironment } from "../runner/DangerfileRunner" -import { DangerContext } from "../runner/Dangerfile" import { dangerfilePath } from "./utils/file-utils" +import validateDangerfileExists from "./utils/validateDangerfileExists" +import openRepl from "./utils/repl" +import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs" const d = debug("danger:pr") -program - .option("-v, --verbose", "Output more text to the stdout than a normal run") - .option("-d, --dangerfile [filePath]", "Specify custom dangerfile other than default dangerfile.js") - .option("-r, --repl", "Drop into a Node REPL after evaluating the dangerfile") - .parse(process.argv) +program.usage("[options] ").description("Emulate running Danger against an existing GitHub Pull Request.") +setSharedArgs(program).parse(process.argv) + +const app = (program as any) as SharedCLI const dangerFile = dangerfilePath(program) @@ -46,70 +45,19 @@ if (program.args.length === 0) { } } -function validateDangerfileExists(filePath: string): boolean { - let stat: fs.Stats | null = null - try { - stat = fs.statSync(filePath) - } catch (error) { - console.error(`Could not find a dangerfile at ${filePath}, not running against your PR.`) - process.exitCode = 1 - } - - if (!!stat && !stat.isFile()) { - console.error(`The resource at ${filePath} appears to not be a file, not running against your PR.`) - process.exitCode = 1 - } - - return !!stat && stat.isFile() -} - async function runDanger(source: FakeCI, platform: GitHub, file: string) { const config = { - stdoutOnly: program.textOnly, - verbose: program.verbose, + stdoutOnly: app.textOnly, + verbose: app.verbose, } const exec = new Executor(source, platform, config) const runtimeEnv = await exec.setupDanger() const results = await runDangerfileEnvironment(file, undefined, runtimeEnv) - if (program["repl"]) { + if (program.repl) { openRepl(runtimeEnv.sandbox) } else { jsome(results) } } - -function openRepl(dangerContext: DangerContext): void { - /** - * Injects a read-only, global variable into the REPL - * - * @param {repl.REPLServer} repl The Node REPL created via `repl.start()` - * @param {string} name The name of the global variable - * @param {*} value The value of the global variable - */ - function injectReadOnlyProperty(repl: repl.REPLServer, name: string, value: any) { - Object.defineProperty(repl["context"], name, { - configurable: false, - enumerable: true, - value, - }) - } - - /** - * Sets up the Danger REPL with `danger` and `results` global variables - * - * @param {repl.REPLServer} repl The Node REPL created via `repl.start()` - */ - function setup(repl: repl.REPLServer) { - injectReadOnlyProperty(repl, "danger", dangerContext.danger) - injectReadOnlyProperty(repl, "results", dangerContext.results) - } - - const dangerRepl = repl.start({ prompt: "> " }) - setup(dangerRepl) - dangerRepl.on("exit", () => process.exit()) - // Called when `.clear` is executed in the Node REPL - // This ensures that `danger` and `results` are not cleared from the REPL context - dangerRepl.on("reset", () => setup(dangerRepl)) -} diff --git a/source/commands/danger-process.ts b/source/commands/danger-process.ts index da77482d6..16fbc1545 100644 --- a/source/commands/danger-process.ts +++ b/source/commands/danger-process.ts @@ -1,20 +1,17 @@ +import * as chalk from "chalk" +import * as program from "commander" + +import { getPlatformForEnv } from "../platforms/platform" +import { Executor } from "../runner/Executor" +import runDangerSubprocess, { prepareDangerDSL } from "./utils/runDangerSubprocess" +import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs" +import getRuntimeCISource from "./utils/getRuntimeCISource" + // Given the nature of this command, it can be tricky to test, so I use a command like this: // // env DANGER_GITHUB_API_TOKEN='xxx' DANGER_FAKE_CI="YEP" DANGER_TEST_REPO='artsy/eigen' DANGER_TEST_PR='2408' // yarn ts-node -s -- source/commands/danger-process.ts ./scripts/danger_runner.rb // -// - -import { spawn } from "child_process" - -import * as program from "commander" -import { getCISource } from "../ci_source/get_ci_source" -import { getPlatformForEnv } from "../platforms/platform" -import { Executor } from "../runner/Executor" -import { providers } from "../ci_source/providers" -import { sentence } from "../runner/DangerUtils" -import * as chalk from "chalk" -import { markdownCode, resultsWithFailure } from "./utils/reporting" declare const global: any @@ -27,69 +24,28 @@ program "into another process expecting the process to eventually return results back as JSON. If you don't " + "provide another process, then it will output to STDOUT." ) - .option("-v, --verbose", "Verbose output of files") - .option("-c, --external-ci-provider [modulePath]", "Specify custom CI provider") - .option("-t, --text-only", "Provide an STDOUT only interface, Danger will not post to your PR") - .action(process_name => (subprocessName = process_name)) - .parse(process.argv) + +setSharedArgs(program) +program.action(process_name => (subprocessName = process_name)).parse(process.argv) // The dynamic nature of the program means typecasting a lot // use this to work with dynamic propeties -const app = program as any - -process.on("unhandledRejection", function(reason: string, _p: any) { - console.log(chalk.red("Error: "), reason) - process.exitCode = 1 -}) - -// const encoding = "utf-8" -// let data = "" - -// process.stdin.setEncoding(encoding) - -// process.stdin.on("readable", function() { -// var chunk -// while ((chunk = process.stdin.read())) { -// data += chunk -// } -// }) - -// process.stdin.on("end", function() { -// // There will be a trailing \n from the user hitting enter. Get rid of it. -// data = data.replace(/\n$/, "") -// processIncomingResults(data) -// }) +const app = (program as any) as SharedCLI if (process.env["DANGER_VERBOSE"] || app.verbose) { global.verbose = true } -// const processIncomingResults = (response: string) => response - // a dirty wrapper to allow async functionality in the setup -async function run(): Promise { - const source = getCISource(process.env, app.externalCiProvider || undefined) - - if (!source) { - console.log("Could not find a CI source for this run. Does Danger support this CI service?") - console.log(`Danger supports: ${sentence(providers.map(p => p.name))}.`) - - if (!process.env["CI"]) { - console.log("You may want to consider using `danger pr` to run Danger locally.") - } - - process.exitCode = 1 - } - // run the sources setup function, if it exists - if (source && source.setup) { - await source.setup() - } +async function run() { + const source = await getRuntimeCISource(app) + // This does not set a failing exit code if (source && !source.isPR) { - // This does not set a failing exit code console.log("Skipping Danger due to not this run not executing on a PR.") } + // The optimal path if (source && source.isPR) { const platform = getPlatformForEnv(process.env, source) if (!platform) { @@ -108,49 +64,13 @@ async function run(): Promise { const exec = new Executor(source, platform, config) const dangerDSL = await exec.dslForDanger() + const processInput = prepareDangerDSL(dangerDSL) - // Remove this to reduce STDOUT spam - if (dangerDSL.github && dangerDSL.github.api) { - delete dangerDSL.github.api - // Add an API token? - } - - const dslJSONString = JSON.stringify(dangerDSL, null, " ") + "\n" if (!subprocessName) { // Just pipe it out to the CLI - process.stdout.write(dslJSONString) + process.stdout.write(processInput) } else { - const child = spawn(subprocessName) - let allLogs = "" - - child.stdin.write(dslJSONString) - child.stdin.end() - - child.stdout.on("data", async data => { - data = data.toString() - const trimmed = data.trim() - if (trimmed.startsWith("{") && trimmed.endsWith("}") && trimmed.includes("markdowns")) { - const results = JSON.parse(trimmed) - await exec.handleResults(results) - } else { - console.log(`stdout: ${data}`) - allLogs += data - } - }) - - child.stderr.on("data", data => { - console.log(`stderr: ${data}`) - }) - - child.on("close", async code => { - console.log(`child process exited with code ${code}`) - - // Submit an error back to the PR - if (process.exitCode) { - const results = resultsWithFailure(`${subprocessName}\` failed.`, "### Log\n\n" + markdownCode(allLogs)) - await exec.handleResults(results) - } - }) + runDangerSubprocess(subprocessName, processInput, exec) } } } diff --git a/source/commands/danger-run.ts b/source/commands/danger-run.ts index ac94487bf..7ed373976 100755 --- a/source/commands/danger-run.ts +++ b/source/commands/danger-run.ts @@ -1,58 +1,29 @@ +import * as chalk from "chalk" import * as program from "commander" import * as debug from "debug" -import * as fs from "fs" -import { getCISource } from "../ci_source/get_ci_source" + import { getPlatformForEnv } from "../platforms/platform" import { Executor } from "../runner/Executor" import { dangerfilePath } from "./utils/file-utils" -import { providers } from "../ci_source/providers" -import { sentence } from "../runner/DangerUtils" -import * as chalk from "chalk" +import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs" +import validateDangerfileExists from "./utils/validateDangerfileExists" +import getRuntimeCISource from "./utils/getRuntimeCISource" const d = debug("danger:run") declare const global: any -// TODO: if we get more options around the dangerfile, we should -// support sharing `program` setup code with danger-pr.ts - -program - .option("-v, --verbose", "Verbose output of files") - .option("-c, --external-ci-provider [modulePath]", "Specify custom CI provider") - .option("-t, --text-only", "Provide an STDOUT only interface, Danger will not post to your PR") - .option("-d, --dangerfile [filePath]", "Specify a custom dangerfile path") - .parse(process.argv) +program.usage("[options]").description("Runs a Dangerfile in JavaScript or TypeScript.") +setSharedArgs(program).parse(process.argv) -// The dynamic nature of the program means typecasting a lot -// use this to work with dynamic propeties -const app = program as any - -process.on("unhandledRejection", function(reason: string, _p: any) { - console.log(chalk.red("Error: "), reason) - process.exitCode = 1 -}) +const app = (program as any) as SharedCLI if (process.env["DANGER_VERBOSE"] || app.verbose) { global.verbose = true } // a dirty wrapper to allow async functionality in the setup -async function run(): Promise { - const source = getCISource(process.env, app.externalCiProvider || undefined) - - if (!source) { - console.log("Could not find a CI source for this run. Does Danger support this CI service?") - console.log(`Danger supports: ${sentence(providers.map(p => p.name))}.`) - - if (!process.env["CI"]) { - console.log("You may want to consider using `danger pr` to run Danger locally.") - } - - process.exitCode = 1 - } - // run the sources setup function, if it exists - if (source && source.setup) { - await source.setup() - } +async function run() { + const source = await getRuntimeCISource(app) if (source && !source.isPR) { // This does not set a failing exit code @@ -73,27 +44,20 @@ async function run(): Promise { console.log(`${chalk.bold("OK")}, everything looks good: ${source.name} on ${platform.name}`) const dangerFile = dangerfilePath(program) - try { - const stat = fs.statSync(dangerFile) - - if (!!stat && stat.isFile()) { - d(`executing dangerfile at ${dangerFile}`) - - const config = { - stdoutOnly: app.textOnly, - verbose: app.verbose, - } + const exists = validateDangerfileExists(dangerFile) + if (!exists) { + console.error(chalk.red(`Looks like your path '${dangerFile}' is not a valid path for a Dangerfile.`)) + process.exitCode = 1 + } else { + d(`executing dangerfile at ${dangerFile}`) - const exec = new Executor(source, platform, config) - exec.setupAndRunDanger(dangerFile) - } else { - console.error(chalk.red(`Looks like your path '${dangerFile}' is not a valid path for a Dangerfile.`)) - process.exitCode = 1 + const config = { + stdoutOnly: app.textOnly, + verbose: app.verbose, } - } catch (error) { - process.exitCode = 1 - console.error(error.message) - console.error(error) + + const exec = new Executor(source, platform, config) + exec.setupAndRunDanger(dangerFile) } } } diff --git a/source/commands/danger.ts b/source/commands/danger.ts index 95b3c230d..b0639a8b3 100644 --- a/source/commands/danger.ts +++ b/source/commands/danger.ts @@ -3,11 +3,16 @@ import { version } from "../../package.json" import * as program from "commander" import * as debug from "debug" +import * as chalk from "chalk" const d = debug("danger:runner") - d(`argv: ${process.argv}`) +process.on("unhandledRejection", function(reason: string, _p: any) { + console.log(chalk.red("Error: "), reason) + process.exitCode = 1 +}) + // Provides the root node to the command-line architecture program .version(version) diff --git a/source/commands/utils/getRuntimeCISource.ts b/source/commands/utils/getRuntimeCISource.ts new file mode 100644 index 000000000..c0aec8149 --- /dev/null +++ b/source/commands/utils/getRuntimeCISource.ts @@ -0,0 +1,29 @@ +import { getCISource } from "../../ci_source/get_ci_source" +import { providers } from "../../ci_source/providers" +import { sentence } from "../../runner/DangerUtils" +import { SharedCLI } from "./sharedDangerfileArgs" +import { CISource } from "../../ci_source/ci_source" + +const getRuntimeCISource = async (app: SharedCLI): Promise => { + const source = getCISource(process.env, app.externalCiProvider || undefined) + + if (!source) { + console.log("Could not find a CI source for this run. Does Danger support this CI service?") + console.log(`Danger supports: ${sentence(providers.map(p => p.name))}.`) + + if (!process.env["CI"]) { + console.log("You may want to consider using `danger pr` to run Danger locally.") + } + + process.exitCode = 1 + } + + // run the sources setup function, if it exists + if (source && source.setup) { + await source.setup() + } + + return source +} + +export default getRuntimeCISource diff --git a/source/commands/utils/repl.ts b/source/commands/utils/repl.ts new file mode 100644 index 000000000..eded807d6 --- /dev/null +++ b/source/commands/utils/repl.ts @@ -0,0 +1,39 @@ +import * as repl from "repl" + +import { DangerContext } from "../../runner/Dangerfile" + +function openRepl(dangerContext: DangerContext): void { + /** + * Injects a read-only, global variable into the REPL + * + * @param {repl.REPLServer} repl The Node REPL created via `repl.start()` + * @param {string} name The name of the global variable + * @param {*} value The value of the global variable + */ + function injectReadOnlyProperty(repl: repl.REPLServer, name: string, value: any) { + Object.defineProperty(repl["context"], name, { + configurable: false, + enumerable: true, + value, + }) + } + + /** + * Sets up the Danger REPL with `danger` and `results` global variables + * + * @param {repl.REPLServer} repl The Node REPL created via `repl.start()` + */ + function setup(repl: repl.REPLServer) { + injectReadOnlyProperty(repl, "danger", dangerContext.danger) + injectReadOnlyProperty(repl, "results", dangerContext.results) + } + + const dangerRepl = repl.start({ prompt: "> " }) + setup(dangerRepl) + dangerRepl.on("exit", () => process.exit()) + // Called when `.clear` is executed in the Node REPL + // This ensures that `danger` and `results` are not cleared from the REPL context + dangerRepl.on("reset", () => setup(dangerRepl)) +} + +export default openRepl diff --git a/source/commands/utils/runDangerSubprocess.ts b/source/commands/utils/runDangerSubprocess.ts new file mode 100644 index 000000000..8aa175c8f --- /dev/null +++ b/source/commands/utils/runDangerSubprocess.ts @@ -0,0 +1,50 @@ +import { spawn } from "child_process" + +import { DangerDSL } from "../../dsl/DangerDSL" +import { Executor } from "../../runner/Executor" +import { markdownCode, resultsWithFailure } from "./reporting" + +// Sanitizes the DSL so for sending via STDOUT +export const prepareDangerDSL = (dangerDSL: DangerDSL) => { + if (dangerDSL.github && dangerDSL.github.api) { + delete dangerDSL.github.api + } + + return JSON.stringify(dangerDSL, null, " ") + "\n" +} + +// Runs the Danger process +const runDangerSubprocess = (subprocessName: string, dslJSONString: string, exec: Executor) => { + const child = spawn(subprocessName) + let allLogs = "" + + child.stdin.write(dslJSONString) + child.stdin.end() + + child.stdout.on("data", async data => { + data = data.toString() + const trimmed = data.trim() + if (trimmed.startsWith("{") && trimmed.endsWith("}") && trimmed.includes("markdowns")) { + const results = JSON.parse(trimmed) + await exec.handleResults(results) + } else { + console.log(`stdout: ${data}`) + allLogs += data + } + }) + + child.stderr.on("data", data => { + console.log(`stderr: ${data}`) + }) + + child.on("close", async code => { + console.log(`child process exited with code ${code}`) + // Submit an error back to the PR + if (process.exitCode) { + const results = resultsWithFailure(`${subprocessName}\` failed.`, "### Log\n\n" + markdownCode(allLogs)) + await exec.handleResults(results) + } + }) +} + +export default runDangerSubprocess diff --git a/source/commands/utils/sharedDangerfileArgs.ts b/source/commands/utils/sharedDangerfileArgs.ts new file mode 100644 index 000000000..88f9222cc --- /dev/null +++ b/source/commands/utils/sharedDangerfileArgs.ts @@ -0,0 +1,15 @@ +import * as program from "commander" + +export interface SharedCLI extends program.ICommand { + verbose: boolean + externalCiProvider: string + textOnly: boolean + dangerfile: string +} + +export default (command: program.ICommand) => + command + .option("-v, --verbose", "Verbose output of files") + .option("-c, --external-ci-provider [modulePath]", "Specify custom CI provider") + .option("-t, --text-only", "Provide an STDOUT only interface, Danger will not post to your PR") + .option("-d, --dangerfile [filePath]", "Specify a custom dangerfile path") diff --git a/source/commands/utils/validateDangerfileExists.ts b/source/commands/utils/validateDangerfileExists.ts new file mode 100644 index 000000000..45d352908 --- /dev/null +++ b/source/commands/utils/validateDangerfileExists.ts @@ -0,0 +1,20 @@ +import * as fs from "fs" + +const validateDangerfileExists = (filePath: string): boolean => { + let stat: fs.Stats | null = null + try { + stat = fs.statSync(filePath) + } catch (error) { + console.error(`Could not find a dangerfile at ${filePath}, not running against your PR.`) + process.exitCode = 1 + } + + if (!!stat && !stat.isFile()) { + console.error(`The resource at ${filePath} appears to not be a file, not running against your PR.`) + process.exitCode = 1 + } + + return !!stat && stat.isFile() +} + +export default validateDangerfileExists