Merge pull request #377 from danger/tighten_commands

Tighten the typings on commands

.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"
+  ]
 }

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
 

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) {

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] <pr_url>").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))
-}

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<any> {
-  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<any> {
 
       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)
       }
     }
   }