From aa9ef9a69b61fd70d470c9309c3440869a474cd1 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Sun, 16 Oct 2016 16:50:23 +0100 Subject: [PATCH] Add support for evaling and running Danger on CI --- .eslintrc.json | 9 ++++++ dangerfile.js | 7 ++++- package.json | 4 +-- source/ci_source/ci_source.js | 24 ++++++++++++++++ source/ci_source/ci_source_helpers.js | 14 +++++++-- source/ci_source/ci_source_selector.js | 11 ------- source/ci_source/fake.js | 1 + source/ci_source/travis.js | 2 ++ source/commands/danger-run.js | 38 ++++++++++++------------ source/commands/danger.js | 8 +++--- source/danger.js | 4 +-- source/dsl/DangerDSL.js | 15 ++++++++++ source/dsl/git.js | 6 ++++ source/platforms/github.js | 26 ++++++++++++----- source/platforms/platform.js | 23 ++++++++++++++- source/runner/Dangerfile.js | 40 ++++++++++++++++++++++++++ source/runner/Executor.js | 23 +++++++++++++++ 17 files changed, 205 insertions(+), 50 deletions(-) create mode 100644 source/dsl/DangerDSL.js create mode 100644 source/dsl/git.js create mode 100644 source/runner/Dangerfile.js create mode 100644 source/runner/Executor.js diff --git a/.eslintrc.json b/.eslintrc.json index f7215c0a5..3096ee257 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -11,7 +11,16 @@ "flowtype" ], "presets": ["es2015"], + "rules": { + "valid-jsdoc": 1, + "require-jsdoc": ["error", { + "require": { + "FunctionDeclaration": true, + "MethodDefinition": false, + "ClassDeclaration": false + } + }], "space-before-function-paren": ["error", "never"], "quotes": [1, "double", "avoid-escape"], "flowtype/define-flow-type": 1, diff --git a/dangerfile.js b/dangerfile.js index f593630c2..72e32a251 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -1,9 +1,14 @@ // @flow +// import danger from "danger" import danger from "./source/danger" // warn on changes in Package.json and not in shrinkwrap // warn on changelog +// console.log(danger) +// console.log(danger.git) +// console.log(danger.git.created_files) + +console.log("Eval'd:", danger.git) -danger.git.created_files.join(" ") diff --git a/package.json b/package.json index d2ed49b92..624526190 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,8 @@ "babel-plugin-transform-flow-strip-types": "^6.8.0", "babel-polyfill": "^6.16.0", "babel-preset-es2015": "^6.16.0", + "babel-plugin-syntax-async-functions": "^6.13.0", + "babel-plugin-transform-regenerator": "^6.16.1", "babel-preset-stage-3": "^6.17.0", "eslint": "^3.3.1", "eslint-config-standard": "^6.0.0-beta.3", @@ -55,8 +57,6 @@ "jest-cli": "^16.0.0" }, "dependencies": { - "babel-plugin-syntax-async-functions": "^6.13.0", - "babel-plugin-transform-regenerator": "^6.16.1", "commander": "^2.9.0", "node-fetch": "^1.6.3", "parse-diff": "^0.4.0" diff --git a/source/ci_source/ci_source.js b/source/ci_source/ci_source.js index 7ef73d44b..d1afffbc8 100644 --- a/source/ci_source/ci_source.js +++ b/source/ci_source/ci_source.js @@ -1,4 +1,5 @@ // @flow +"strict mode" /** A json object that represents the outer ENV */ export type Env = any; @@ -6,6 +7,9 @@ export type Env = any; /** The shape of an object that represents an individual CI */ export interface CISource { + /** The name, mainly for showing errors */ + env: string, + /** The hash of environment variables */ env: Env, @@ -27,3 +31,23 @@ export interface CISource { /** What is the URL for the repo */ repoURL: string, } + +import Travis from "./travis" +import Fake from "./fake" + +/** + * Gets a CI Source form the current environment, by asking all known + * sources if they can be represented in this environment. + * @param {Env} env The environment. + * @returns {?CISource} a CI source if it's OK, otherwise Danger can't run. +*/ +export function getCISourceForEnv(env: Env) : ?CISource { + // Fake is what I'm using during dev for the minute + let travis = new Travis(env) + if (travis.isCI) { + return travis + } else { + return new Fake() + } +} + diff --git a/source/ci_source/ci_source_helpers.js b/source/ci_source/ci_source_helpers.js index 9fceb3f3e..f070066eb 100644 --- a/source/ci_source/ci_source_helpers.js +++ b/source/ci_source/ci_source_helpers.js @@ -3,7 +3,12 @@ import type { Env } from "./ci_source" -/** Validates that all ENV keys exist and have a length */ +/** + * Validates that all ENV keys exist and have a length + * @param {Env} env The environment. + * @param {[string]} keys Keys to ensure existence of + * @returns {bool} true if they exist, false if not +*/ export function ensureEnvKeysExist(env: Env, keys: string[]) : boolean { let hasKeys = keys.map((key: string) : boolean => { return env.hasOwnProperty(key) && env[key].length > 0 @@ -11,7 +16,12 @@ export function ensureEnvKeysExist(env: Env, keys: string[]) : boolean { return !hasKeys.includes(false) } -/** Validates that all ENV keys exist and can be turned into ints */ +/** + * Validates that all ENV keys exist and can be turned into ints + * @param {Env} env The environment. + * @param {[string]} keys Keys to ensure existence and number-ness of + * @returns {bool} true if they are all good, false if not +*/ export function ensureEnvKeysAreInt(env: Env, keys: string[]) : boolean { let hasKeys = keys.map((key: string) : boolean => { return env.hasOwnProperty(key) && !isNaN(parseInt(env.TRAVIS_PULL_REQUEST)) diff --git a/source/ci_source/ci_source_selector.js b/source/ci_source/ci_source_selector.js index 6ed517139..e69de29bb 100644 --- a/source/ci_source/ci_source_selector.js +++ b/source/ci_source/ci_source_selector.js @@ -1,11 +0,0 @@ -// @flow -"strict mode" - -import type { Env, CISource } from "./ci_source" -import Travis from "./travis" - -/** Here is some docs for the function */ -export function getCISourceForEnv(env: Env) : ?CISource { - let travis = new Travis(env) - return travis -} diff --git a/source/ci_source/fake.js b/source/ci_source/fake.js index bf7821175..1b5e41600 100644 --- a/source/ci_source/fake.js +++ b/source/ci_source/fake.js @@ -6,6 +6,7 @@ import type { Env } from "./ci_source" export default class FakeCI { env: Env constructor(env: Env) { this.env = env } + get name(): string { return "Fake Testing CI" } get isCI() : boolean { return true } get isPR() : boolean { return true } diff --git a/source/ci_source/travis.js b/source/ci_source/travis.js index 55616b47e..e67ab94e1 100644 --- a/source/ci_source/travis.js +++ b/source/ci_source/travis.js @@ -7,6 +7,8 @@ export default class Travis { env: Env constructor(env: Env) { this.env = env } + get name(): string { return "Travis CI" } + get isCI() : boolean { return ensureEnvKeysExist(this.env, ["HAS_JOSH_K_SEAL_OF_APPROVAL"]) } diff --git a/source/commands/danger-run.js b/source/commands/danger-run.js index ec3352c71..57d2cf954 100755 --- a/source/commands/danger-run.js +++ b/source/commands/danger-run.js @@ -3,12 +3,9 @@ import "babel-polyfill" var program = require("commander") -import { getCISourceForEnv } from "../ci_source/ci_source_selector" -import { GitHub } from "../platforms/github" - -// import type { Platform } from "../platforms/platform" -// import type { CISource } from "../ci_source/ci_source" -import FakeCI from "../ci_source/fake" +import { getCISourceForEnv } from "../ci_source/ci_source" +import { getPlatformForEnv } from "../platforms/platform" +import Executor from "../runner/Executor" program .option("-h, --head [commitish]", "TODO: Set the head commitish") @@ -16,21 +13,22 @@ program .option("-f, --fail-on-errors", "TODO: Fail on errors") .parse(process.argv) -// function setupPlatformWithSource(platform:Platform, source: CISource): void { - -// } - let source = getCISourceForEnv(process.env) -let fake = new FakeCI(process.env) -let github = new GitHub("OK", fake) -github.getInfo() +if (!source) { + console.log("Could not find a CI source for this run") + process.exitCode = 1 +} if (source) { - console.log("OK?") - console.log(source.isCI) - console.log("Is PR?") - console.log(source.isPR) -} else { - console.log("Could not find a CI source for this run") - process.exit(0) + const platform = getPlatformForEnv(process.env, source) + if (!platform) { + console.log(`Could not find a source code hosting platform for ${source.name}`) + process.exitCode = 1 + } + + if (platform) { + console.log(`OK, looks good ${source.name} on ${platform.name}`) + const exec = new Executor(source, platform) + exec.run() + } } diff --git a/source/commands/danger.js b/source/commands/danger.js index 924b00e2c..e0d15af41 100644 --- a/source/commands/danger.js +++ b/source/commands/danger.js @@ -1,4 +1,7 @@ #! /usr/bin/env node + +console.log("OK") + // @flow // This is needed so that other files can use async funcs import "babel-polyfill" @@ -14,7 +17,4 @@ program .command("run", "Runs danger on your local system", {isDefault: true}) .command("init", "Creates a new Dangerfile.js") .command("local", "Runs your changes against ") - -console.log("pre?") -program.parse(process.argv) -console.log("post?") + .parse(process.argv) diff --git a/source/danger.js b/source/danger.js index 2afa2463f..7b9a64da6 100644 --- a/source/danger.js +++ b/source/danger.js @@ -2,14 +2,14 @@ // This file represents the module that is exposed as the danger API import "babel-polyfill" -type DangerGit = { +type GitDSL = { modified_files: string[], created_files: string[], deleted_files: string[] } type DangerDSL = { - git: DangerGit + git: GitDSL } const danger: DangerDSL = { diff --git a/source/dsl/DangerDSL.js b/source/dsl/DangerDSL.js new file mode 100644 index 000000000..55390880e --- /dev/null +++ b/source/dsl/DangerDSL.js @@ -0,0 +1,15 @@ +// @flow +"use strict" + +// import type { Platform } from "../platforms/platform" +import type { GitDSL } from "../dsl/Git" + +export default class DangerDSL { + git: GitDSL + pr: any + + constructor(pr: any, git: GitDSL) { + this.git = git + this.pr = pr + } +} diff --git a/source/dsl/git.js b/source/dsl/git.js new file mode 100644 index 000000000..19594fccf --- /dev/null +++ b/source/dsl/git.js @@ -0,0 +1,6 @@ +export interface GitDSL { + modified_files: string[], + created_files: string[], + deleted_files: string[] +} + diff --git a/source/platforms/github.js b/source/platforms/github.js index 36dbea98c..e110bb857 100644 --- a/source/platforms/github.js +++ b/source/platforms/github.js @@ -1,6 +1,7 @@ // @flow "use strict" +import type { GitDSL } from "../danger" import type { CISource } from "../ci_source/ci_source" import parseDiff from "parse-diff" @@ -12,6 +13,8 @@ export type APIToken = string; export type GraphQLQuery = string; export type GraphQLResponse = any; +/** This represent the GitHub API, and conforming to the Platform Interface */ + export class GitHub { token: APIToken ciSource: CISource @@ -21,24 +24,33 @@ export class GitHub { this.ciSource = ciSource } - async getInfo() : void { + name: "GitHub" + + async getReviewInfo() : Promise { let deets = await this.getPullRequestInfo() - let pr = await deets.json() - console.log(pr) + return await deets.json() + } + async getReviewDiff() : Promise { let diffReq = await this.getPullRequestDiff() let diff = await diffReq.text() + + // Worth trying to add a flow-typed for this as a tester? let fileDiffs: [any] = parseDiff(diff) let addedDiffs = fileDiffs.filter((diff: any) => diff["new"]) let removedDiffs = fileDiffs.filter((diff: any) => diff["deleted"]) - let modified = fileDiffs.filter((diff: any) => !addedDiffs.includes(diff) && !removedDiffs.includes(diff)) + let modifiedDiffs = fileDiffs.filter((diff: any) => !addedDiffs.includes(diff) && !removedDiffs.includes(diff)) - console.log("Added: ", addedDiffs) - console.log("Removed: ", removedDiffs) - console.log("Modified: ", modified) + return { + modified_files: modifiedDiffs.map((d: any) => d.to), + created_files: addedDiffs.map((d: any) => d.to), + deleted_files: removedDiffs.map((d: any) => d.from) + } } + // The above is the API for Platform + getUserInfo(): Promise { return this.get("user") } diff --git a/source/platforms/platform.js b/source/platforms/platform.js index 41206d88f..7d9aedf32 100644 --- a/source/platforms/platform.js +++ b/source/platforms/platform.js @@ -12,8 +12,15 @@ export type Comment = { } export interface Platform { - ciSource: CISource + /** Mainly for logging and error reporting */ + name: String, + ciSource: CISource, + /** Pulls in the Code Review Metadata for inspection */ + getReviewInfo: () => Promise, + /** Pulls in the Code Review Diff, and offers a succinct user-API for it */ + fetchDiffInfo: () => Promise } + // envVars: () => string[]; // optionalEnvVars: () => string[]; @@ -32,3 +39,17 @@ export interface Platform { // /** Edit an existing comment */ // async editComment: (comment: Comment, newBody: string) => Promise; // } + +import { GitHub } from "./github" + +/** + * Pulls out a platform for Danger to communicate on based on the environment + * @param {Env} env The environment. + * @param {CISource} source The existing source, to ensure they can run against each other + * @returns {?Platform} returns a platform if it can be supported +*/ +export function getPlatformForEnv(env: Env, source: CISource) : ?Platform { + let github = new GitHub(env["DANGER_GITHUB_API_TOKEN"], source) + return github +} + diff --git a/source/runner/Dangerfile.js b/source/runner/Dangerfile.js new file mode 100644 index 000000000..3eda24a89 --- /dev/null +++ b/source/runner/Dangerfile.js @@ -0,0 +1,40 @@ +// https://nodejs.org/api/vm.html +// https://60devs.com/executing-js-code-with-nodes-vm-module.html + +import fs from "fs" +import vm from "vm" +import DangerDSL from "../dsl/DangerDSL" + +export default class Dangerfile { + dsl: DangerDSL + constructor(dsl: DangerDSL) { this.dsl = dsl } + + run(file: string) { + fs.readFile(file, "utf8", (err: Error, data: string) => { + if (err) { return console.error(err.message) } + + // comment out imports of 'danger' + // e.g `import danger from` + // then user get typed data, and we fill it in + // via the VM context + + const cleaned = data.replace(/import danger from/gi, "// import danger from") + + const script = new vm.Script(cleaned, { + filename: file, + lineOffset: 1, + columnOffset: 1, + displayErrors: true, + timeout: 1000 // ms + }) + + const context: any = { + console, + danger: this.dsl + } + + script.runInNewContext(context) + }) + } +} + diff --git a/source/runner/Executor.js b/source/runner/Executor.js new file mode 100644 index 000000000..174fb856c --- /dev/null +++ b/source/runner/Executor.js @@ -0,0 +1,23 @@ +import Dangerfile from "../runner/Dangerfile" +import DangerDSL from "../dsl/DangerDSL" +import { Platform } from "../platforms/platform" + +// This is still badly named, maybe it really sbhould just be runner? + +export default class Executor { + ciSource: CISource + platform: Platform + + constructor(ciSource: CISource, platform: Platform) { + this.ciSource = ciSource + this.platform = platform + } + + async run() { + const git = await this.platform.getReviewDiff() + const pr = await this.platform.getReviewInfo() + let dsl = new DangerDSL(pr, git) + const dangerfile = new Dangerfile(dsl) + dangerfile.run("dangerfile.js") + } +}