More work on peril support

source/commands/danger-run.js

@@ -38,7 +38,7 @@ if (source && source.isPR) {
     console.log(`OK, looks good ${source.name} on ${platform.name}`)
     try {
       const exec = new Executor(source, platform)
-      exec.runDanger("dangerfile.js")
+      exec.setupAndRunDanger("dangerfile.js")
     } catch (error) {
       console.error(error.message)
       console.error(error)

source/platforms/GitHub.js

@@ -23,12 +23,17 @@ export class GitHub {
   token: APIToken
   ciSource: CISource
   name: string
-  additionalHeaders: any
+  // A fetch-api shaped function
+  fetch: any
 
   constructor(token: APIToken, ciSource: CISource) {
     this.token = token
     this.ciSource = ciSource
     this.name = "GitHub"
+
+    // This allows Peril to DI in a new Fetch function
+    // which can handle unique API edge-cases around integrations
+    this.fetch = fetch
   }
 
   /**
@@ -137,12 +142,12 @@ export class GitHub {
     // Use head of PR (current state of PR) if no ref passed
     if (!ref) {
       const prJSON = await this.getReviewInfo()
-      console.log(prJSON)
       ref = prJSON.head.ref
     }
     const fileMetadata = await this.getFileContents(path, ref)
     const data = await fileMetadata.json()
-    return data.content
+    const buffer = new Buffer(data.content, "base64")
+    return buffer.toString()
   }
 
   // The above is the API for Platform
@@ -208,47 +213,42 @@ export class GitHub {
 
   getFileContents(path: string, ref: string): Promise<Response> {
     const repo = this.ciSource.repoSlug
-    return this.get(`repos/${repo}/contents/${path}`, {
-      ref: ref
-    })
+    return this.get(`repos/${repo}/contents/${path}?ref=${ref}`, {})
   }
 
   // maybe this can move into the stuff below
   post(path: string, headers: any = {}, body: any = {}, method: string = "POST"): Promise<Response> {
-    return fetch(`https://api.github.com/${path}`, {
+    return this.fetch(`https://api.github.com/${path}`, {
       method: method,
       body: JSON.stringify(body),
       headers: {
         "Authorization": `token ${this.token}`,
         "Content-Type": "application/json",
-        ...headers,
-        ...this.additionalHeaders
+        ...headers
       }
     })
   }
 
   get(path: string, headers: any = {}, body: any = {}, method: string = "GET"): Promise<Response> {
-    return fetch(`https://api.github.com/${path}`, {
+    return this.fetch(`https://api.github.com/${path}`, {
       method: method,
       body: body,
       headers: {
         "Authorization": `token ${this.token}`,
         "Content-Type": "application/json",
-        ...headers,
-        ...this.additionalHeaders
+        ...headers
       }
     })
   }
 
   patch(path: string, headers: any = {}, body: any = {}, method: string = "PATCH"): Promise<Response> {
-    return fetch(`https://api.github.com/${path}`, {
+    return this.fetch(`https://api.github.com/${path}`, {
       method: method,
       body: JSON.stringify(body),
       headers: {
         "Authorization": `token ${this.token}`,
         "Content-Type": "application/json",
-        ...headers,
-        ...this.additionalHeaders
+        ...headers
       }
     })
   }

source/runner/DangerfileRunner.js

@@ -1,60 +1,21 @@
 // @flow
 
-type Path = string
-
-interface ResolverConfig {
-  browser?: boolean,
-  defaultPlatform: ?string,
-  extensions: Array<string>,
-  hasCoreModules: boolean,
-  moduleDirectories: Array<string>,
-  moduleNameMapper: ?{[key: string]: RegExp},
-  modulePaths: Array<Path>,
-  platforms?: Array<string>,
-};
-
-interface HasteConfig {
-  providesModuleNodeModules: Array<string>,
-  defaultPlatform?: ?string,
-  platforms?: Array<string>,
-};
-
-interface Environment {
-  constructor(config: any): void;
-  dispose(): void;
-  runScript(script: any): any;
-  global: any;
-  fakeTimers: {
-    clearAllTimers(): void;
-    runAllImmediates(): void;
-    runAllTicks(): void;
-    runAllTimers(): void;
-    runTimersToTime(): void;
-    runOnlyPendingTimers(): void;
-    runWithRealTimers(callback: any): void;
-    useFakeTimers(): void;
-    useRealTimers(): void;
-  };
-  testFilePath: string;
-  moduleMocker: any;
-};
-
 import Runtime from "jest-runtime"
 import NodeEnvironment from "jest-environment-node"
 import os from "os"
 
 import type { DangerResults } from "../dsl/DangerResults"
 import type { DangerContext } from "../runner/Dangerfile"
+import type { DangerfileRuntimeEnv, Environment, Path } from "./types"
 
 /**
  * Executes a Dangerfile at a specific path, with a context.
  * The values inside a Danger context are applied as globals to the Dangerfiles runtime.
  *
- * @param {string} filename the file path for the dangerfile
  * @param {DangerContext} dangerfileContext the gloabl danger context
- * @returns {DangerResults} the results of the run
+ * @returns {any} the results of the run
  */
-export async function runDangerfile(filename: string, dangerfileContext: DangerContext): Promise<DangerResults> {
+export async function createDangerfileRuntimeEnvironment(dangerfileContext: DangerContext): Promise<DangerfileRuntimeEnv> {
   const config = {
     cacheDirectory: os.tmpdir(),
     setupFiles: [],
@@ -69,7 +30,8 @@ export async function runDangerfile(filename: string, dangerfileContext: DangerC
     cache: null
   }
 
-  const environment:Environment = new NodeEnvironment(config)
+  const environment: Environment = new NodeEnvironment(config)
+
   const runnerGlobal = environment.global
   const context = dangerfileContext
 
@@ -87,9 +49,25 @@ export async function runDangerfile(filename: string, dangerfileContext: DangerC
   const resolver = Runtime.createResolver(config, hasteMap.moduleMap)
   const runtime = new Runtime(config, environment, resolver)
 
+  return {
+    context,
+    environment,
+    runtime
+  }
+}
+
+/**
+ * Executes a Dangerfile at a specific path, with a context.
+ * The values inside a Danger context are applied as globals to the Dangerfiles runtime.
+ *
+ * @param {string} filename the file path for the dangerfile
+ * @param {any} environment the results of createDangerfileRuntimeEnvironment
+ * @returns {DangerResults} the results of the run
+ */
+export async function runDangerfileEnvironment(filename: Path, environment: DangerfileRuntimeEnv): Promise<DangerResults> {
+  const runtime = environment.runtime
   // Require our dangerfile
-  // TODO: This needs to be able to support arbitrary strings for Peril
   runtime.requireModule(filename)
 
-  return context.results
+  return environment.context.results
 }

source/runner/Executor.js

@@ -6,7 +6,8 @@ import type { CISource } from "../ci_source/ci_source"
 import { Platform } from "../platforms/platform"
 import type { DangerResults } from "../dsl/DangerResults"
 import githubResultsTemplate from "./templates/github-issue-template"
-import { runDangerfile } from "./DangerfileRunner"
+import { createDangerfileRuntimeEnvironment, runDangerfileEnvironment } from "./DangerfileRunner"
+import type { DangerfileRuntimeEnv } from "./types"
 
 // This is still badly named, maybe it really should just be runner?
 
@@ -19,14 +20,32 @@ export default class Executor {
     this.platform = platform
   }
 
+  /** Mainly just a dumb helper because I can't do
+   * async functions in danger-run.js
+   */
+  async setupAndRunDanger(file: string) {
+    const runtimeEnv = await this.setupDanger()
+    await this.runDanger(file, runtimeEnv)
+  }
+
   /**
    *  Runs all of the operations for a running just Danger
-   * @returns {void} It's a promise, so a void promise
+   * @returns {DangerfileRuntimeEnv} A runtime environment to run Danger in
    */
-  async runDanger(file: string) {
+  async setupDanger(): DangerfileRuntimeEnv {
     const dsl = await this.dslForDanger()
     const context = contextForDanger(dsl)
-    const results = await runDangerfile(file, context)
+    return await createDangerfileRuntimeEnvironment(context)
+  }
+
+  /**
+   *  Runs all of the operations for a running just Danger
+   * @param {string} file the filepath to the Dangerfile
+   * @returns {void} It's a promise, so a void promise
+   */
+
+  async runDanger(file: string, runtime: DangerfileRuntimeEnv) {
+    const results = await runDangerfileEnvironment(file, runtime)
     await this.handleResults(results)
   }
 

source/runner/_tests/DangerRunner.test.js

@@ -1,7 +1,7 @@
 // @flow
 
 import { contextForDanger } from "../Dangerfile"
-import {runDangerfile} from "../DangerfileRunner"
+import {createDangerfileRuntimeEnvironment, runDangerfileEnvironment} from "../DangerfileRunner"
 import Fake from "../../ci_source/Fake"
 import FakePlatform from "../../platforms/FakePlatform"
 import Executor from "../Executor"
@@ -27,7 +27,8 @@ async function setupDangerfileContext() {
 describe("with fixtures", () => {
   it("handles a blank Dangerfile", async () => {
     const context = await setupDangerfileContext()
-    const results = await runDangerfile(`${fixtures}/__DangerfileEmpty.js`, context)
+    const runtime = await createDangerfileRuntimeEnvironment(context)
+    const results = await runDangerfileEnvironment(`${fixtures}/__DangerfileEmpty.js`, runtime)
 
     expect(results).toEqual({
       fails: [],
@@ -39,7 +40,9 @@ describe("with fixtures", () => {
 
   it("handles a full set of  messages", async () => {
     const context = await setupDangerfileContext()
-    const results = await runDangerfile(`${fixtures}/__DangerfileFullMessages.js`, context)
+    const runtime = await createDangerfileRuntimeEnvironment(context)
+
+    const results = await runDangerfileEnvironment(`${fixtures}/__DangerfileFullMessages.js`, runtime)
 
     expect(results).toEqual({
       fails: [{"message": "this is a failure"}],
@@ -51,8 +54,10 @@ describe("with fixtures", () => {
 
   it("handles a failing dangerfile", async () => {
     const context = await setupDangerfileContext()
+    const runtime = await createDangerfileRuntimeEnvironment(context)
+
     try {
-      await runDangerfile(`${fixtures}/__DangerfileBadSyntax.js`, context)
+      await runDangerfileEnvironment(`${fixtures}/__DangerfileBadSyntax.js`, runtime)
       throw new Error("Do not get to this")
     }
     catch (e) {
@@ -63,6 +68,8 @@ describe("with fixtures", () => {
 
   it("handles relative imports correctly", async () => {
     const context = await setupDangerfileContext()
-    await runDangerfile(`${fixtures}/__DangerfileImportRelative.js`, context)
+    const runtime = await createDangerfileRuntimeEnvironment(context)
+
+    await runDangerfileEnvironment(`${fixtures}/__DangerfileImportRelative.js`, runtime)
   })
 })

source/runner/types.js

@@ -0,0 +1,44 @@
+export type Path = string
+
+export interface ResolverConfig {
+  browser?: boolean,
+  defaultPlatform: ?string,
+  extensions: Array<string>,
+  hasCoreModules: boolean,
+  moduleDirectories: Array<string>,
+  moduleNameMapper: ?{[key: string]: RegExp},
+  modulePaths: Array<Path>,
+  platforms?: Array<string>,
+};
+
+export interface HasteConfig {
+  providesModuleNodeModules: Array<string>,
+  defaultPlatform?: ?string,
+  platforms?: Array<string>,
+};
+
+export interface Environment {
+  constructor(config: any): void;
+  dispose(): void;
+  runScript(script: any): any;
+  global: any;
+  fakeTimers: {
+    clearAllTimers(): void;
+    runAllImmediates(): void;
+    runAllTicks(): void;
+    runAllTimers(): void;
+    runTimersToTime(): void;
+    runOnlyPendingTimers(): void;
+    runWithRealTimers(callback: any): void;
+    useFakeTimers(): void;
+    useRealTimers(): void;
+  };
+  testFilePath: string;
+  moduleMocker: any;
+};
+
+export interface DangerfileRuntimeEnv {
+  context: DangerContext,
+  environment: Environment,
+  runtime: any
+}