Merge pull request #31 from danger/tests_ok

Support warning, messages and arbitrary markdown in the DSL

.flowconfig

@@ -4,7 +4,6 @@ distribution
 [include]
 
 [libs]
-interfaces/
 flow-typed/
 
 [options]

README.md

@@ -1,16 +1,8 @@
-# danger-js
-
 Danger on Node, wonder what's going on? see [VISION.md](VISION.md)
 
-### Get started?
-
-This is like, kinda early. If you can take a bit of heat, it's usable in production as of 0.0.10. Note: There is basically no error reporting ATM.
-
-### Early Adopters
-
 *Welcome!*
 
-So, what's the deal? Well, right now Danger JS does the MVP of the Ruby version. You can look at Git metadata, or GitHub metadata on Travis CI. 
+So, what's the deal? Well, right now Danger JS does the MVP of [the Ruby version](http://danger.systems). You can look at [Git](https://github.com/danger/danger-js/blob/master/source/dsl/GitDSL.js) metadata, or [GitHub](https://github.com/danger/danger-js/blob/master/source/dsl/GitHubDSL.js) metadata on Travis CI. 
 
 Danger can fail your build, write a comment on GitHub, edit it as your build changes and then delete it once you've passed review.
 
@@ -29,24 +21,37 @@ Then create a `dangerfile.js` in the project root with some rules:
 
 ```js
 import { danger, fail } from "danger"
+const fs = require("fs")
 
 // Make sure there are changelog entries
 const hasChangelog = danger.git.modified_files.includes("changelog.md")
 if (!hasChangelog) {
   fail("No Changelog changes!")
 }
+
+const jsFiles = danger.git.created_files.filter(path => path.endsWith("js"))
+
+// new js files should have `@flow` at the top
+const unFlowedFiles = jsFiles.filter(filepath => {
+  const content = fs.readFileSync(filepath)
+  return !content.includes("@flow")
+})
+
+if (unFlowedFiles.length > 0) {
+  warn(`These new JS files do not have Flow enabled: ${unFlowedFiles.join(", ")}`)
+}
 ```
 
 Then you add `npm run danger` to the end of your CI run, and Danger will run. Here's [an example](https://github.com/artsy/emission/pull/385). 👍
 
 
 Notes: 
 
-* the `Dangerfile.js` needs to be able to run on node without transpiling right now.
+* The `Dangerfile.js` needs to be able to run on node without running through babel right now.
 * The shape of the API is [`git`](https://github.com/danger/danger-js/blob/master/source/dsl/git.js) and [`pr`](https://raw.githubusercontent.com/danger/danger/master/spec/fixtures/github_api/pr_response.json)
 
 
-#### This thing is broken, I should help improve it
+#### This thing is broken, I should help improve it!
 
 Awesommmmee.
 

changelog.md

@@ -2,8 +2,13 @@
 
 //  Add your own contribution below
 
-* Started turning this into a real project by adding tests - orta
+### 0.5.0
+
 * `danger.pr` -> `danger.github.pr`, I've also created interfaces for them - orta
+* `warn`, `message`, `markdown` are all ported over to DangerJS - orta
+* Shows a HTML table for Danger message - orta
+* Now offers a Flow-typed definition file, it's not shipped to their repo yet, you can make it by `npm run export-flowtype` - orta 
+* Started turning this into a real project by adding tests - orta
 
 ### 0.0.5-0.0.10
 

dangerfile.js

@@ -0,0 +1,22 @@
+// @flow
+
+import { danger, fail, warn } from "danger"
+const fs = require("fs")
+
+// warn on changes in Package.json and not in shrinkwrap
+const hasChangelog = danger.git.modified_files.includes("changelog.md")
+if (!hasChangelog) {
+  fail("No Changelog changes!")
+}
+
+const jsFiles = danger.git.created_files.filter(path => path.endsWith("js"))
+
+// new js files should have `@flow` at the top
+const unFlowedFiles = jsFiles.filter(filepath => {
+  const content = fs.readFileSync(filepath)
+  return !content.includes("@flow")
+})
+
+if (unFlowedFiles.length > 0) {
+  warn(`These new JS files do not have Flow enabled: ${unFlowedFiles.join(", ")}`)
+}

docs/js_glossary.md

@@ -141,6 +141,8 @@ If you have a special case example of a literal, you should consider using a typ
 * An `Env` type, that maps to `any`.
 * An `APIToken` type, that maps to `strings`.
 
+Note: If you think you're being clever by having a both ES6 classes, and Flow interfaces exported from the same file - don't, it's not worked well for me so far. Odd transpilation errors. 
+
 #### Tests + Types
 
 Realistically, it's a bit too early for me to be writing about that. Here as a stub for later.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "danger",
-  "version": "0.0.10",
+  "version": "0.5.0",
   "description": "Automate your culture",
   "main": "distribution/danger.js",
   "bin": {
@@ -16,15 +16,14 @@
   "scripts": {
     "test": "jest",
     "testwatch": "jest --watch",
-    "flow": "flow; test $? -eq 0 -o $? -eq 2",
+    "flow": "flow check --ignore '.*/_tests/.*, dangerfile.js'",
     "lint": "eslint ./source",
     "fix": "eslint ./source --fix",
     "prepublish": "npm run build",
     "build": "babel source --out-dir distribution --source-maps",
     "buildwatch": "babel source --watch --out-dir distribution",
     "link": "npm run build ; chmod +x distribution/commands/danger.js ; npm link",
     "export-flowtype": "node scripts/create-flow-typed-export.js"
-
   },
   "repository": {
     "type": "git",
@@ -53,7 +52,7 @@
     "babel-preset-stage-3": "^6.17.0",
     "eslint": "^3.3.1",
     "eslint-config-standard": "^6.0.0-beta.3",
-    "eslint-plugin-flowtype": "^2.20.0",
+    "eslint-plugin-flowtype": "^2.25.0",
     "eslint-plugin-promise": "^3.3.0",
     "eslint-plugin-standard": "^2.0.0",
     "flow-bin": "^0.32.0",

scripts/create-flow-typed-export.js

@@ -22,15 +22,13 @@ fs.readdir("source/dsl", (err, files) => {
   // we need to add either `declare function` or `declare var` to the interface
   const context = moduleContext.split("\n").map((line) => {
     if ((line.length === 0) || (line.includes("*"))) { return line }
-    if (line.includes("(")) { return "    declare function " + line.trim() }
-    if (line.includes(":")) { return "    declare var " + line.trim() }
+    if (line.includes("(")) { return "  declare function " + line.trim() }
+    if (line.includes(":")) { return "  declare var " + line.trim() }
   }).join("\n")
 
   fileOutput += `
 declare module "danger" {
-  declare module.exports: {
-    ${context}
-  };
+  ${context}
 }
 `
   // Remove all JS-y bits

source/ci_source/ci_source.js

@@ -7,7 +7,7 @@ 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,
+    name: string,
 
     /** The hash of environment variables */
     env: Env,

source/commands/danger-run.js

@@ -8,18 +8,25 @@ import { getPlatformForEnv } from "../platforms/platform"
 import Executor from "../runner/Executor"
 
 program
-  .option("-h, --head [commitish]", "TODO: Set the head commitish")
-  .option("-b, --base [commitish]", "TODO: Set the base commitish")
   .option("-f, --fail-on-errors", "TODO: Fail on errors")
   .parse(process.argv)
 
+process.on("unhandledRejection", function(reason: string, p: any) {
+  console.log("Error: ", reason)
+  process.exitCode = 1
+})
+
 const source = getCISourceForEnv(process.env)
 if (!source) {
   console.log("Could not find a CI source for this run")
   // Check for ENV["CI"] and wanr they might want a local command instead?
   process.exitCode = 1
 }
 
+if (source && !source.isPR) {
+  console.log("Skipping due to not being a PR")
+}
+
 if (source && source.isPR) {
   const platform = getPlatformForEnv(process.env, source)
   if (!platform) {
@@ -29,9 +36,12 @@ if (source && source.isPR) {
 
   if (platform) {
     console.log(`OK, looks good ${source.name} on ${platform.name}`)
-    const exec = new Executor(source, platform)
-    exec.setup()
-    const results = exec.run()
-    exec.handleResults(results)
+    try {
+      const exec = new Executor(source, platform)
+      exec.runDanger()
+    } catch (error) {
+      console.error(error.message)
+      console.error(error)
+    }
   }
 }

source/commands/danger.js

@@ -1,8 +1,5 @@
 #! /usr/bin/env node
 
-console.log("OK")
-
-// @flow
 // This is needed so that other files can use async funcs
 import "babel-polyfill"
 

source/danger.js

@@ -2,24 +2,4 @@
 // This file represents the module that is exposed as the danger API
 import "babel-polyfill"
 
-type GitDSL = {
-  modified_files: string[],
-  created_files: string[],
-  deleted_files: string[]
-}
-
-type DangerDSL = {
-  git: GitDSL
-}
-
-/** Fails the build */
-export function fail(message: string) {
-}
-
-export const danger: DangerDSL = {
-  git: {
-    modified_files: ["hello world"],
-    created_files: ["other file"],
-    deleted_files: ["last file"]
-  }
-}
+// Decide what to do with this file in the future.

source/dsl/Aliases.js

@@ -0,0 +1,2 @@
+// @flow
+export type MarkdownString = string;

source/platforms/FakePlatform.js

@@ -0,0 +1,43 @@
+// @flow
+"use strict"
+
+import type { GitDSL } from "../dsl/GitDSL"
+import type { CISource } from "../ci_source/ci_source"
+
+export default class FakePlatform {
+  name: string
+  ciSource: CISource
+
+  constructor() {
+    this.name = "Fake"
+  }
+
+  async getReviewInfo(): Promise<any> {
+
+  }
+
+  async getReviewDiff(): Promise<GitDSL> {
+    return {
+      modified_files: [],
+      created_files: [],
+      deleted_files: []
+    }
+  }
+
+  async updateOrCreateComment(newComment: string): Promise<bool> {
+    return true
+  }
+
+  async createComment(comment: string): Promise<any> {
+    return true
+  }
+
+  async deleteMainComment(): Promise<bool> {
+    return true
+  }
+
+  async editMainComment(comment: string): Promise<bool> {
+    return true
+  }
+}
+

source/platforms/GitHub.js

@@ -1,7 +1,7 @@
 // @flow
 "use strict"
 
-import type { GitDSL } from "../dsl/git"
+import type { GitDSL } from "../dsl/GitDSL"
 import type { CISource } from "../ci_source/ci_source"
 import parseDiff from "parse-diff"
 
@@ -31,7 +31,7 @@ export class GitHub {
    *
    * @returns {Promise<any>} JSON representation
    */
-  async getReviewInfo() : Promise<any> {
+  async getReviewInfo(): Promise<any> {
     const deets = await this.getPullRequestInfo()
     return await deets.json()
   }
@@ -59,13 +59,6 @@ export class GitHub {
     }
   }
 
-  async updateOrCreateComment(newComment: string): Promise<bool> {
-    const commentID = await this.getDangerCommentID()
-    if (commentID) { await this.updateCommentWithID(commentID, newComment) }
-    else { await this.createComment(newComment) }
-    return true
-  }
-
   /**
    * Returns the response for the new comment
    *
@@ -91,6 +84,19 @@ export class GitHub {
     return commentID !== null
   }
 
+  /**
+   * Either updates an existing comment, or makes a new one
+   *
+   * @param {string} newComment string value of comment
+   * @returns {Promise<bool>} success of posting comment
+   */
+  async updateOrCreateComment(newComment: string): Promise<bool> {
+    const commentID = await this.getDangerCommentID()
+    if (commentID) { await this.updateCommentWithID(commentID, newComment) }
+    else { await this.createComment(newComment) }
+    return true
+  }
+
   /**
    * Updates the main Danger comment, when Danger has run
    * more than once

source/platforms/_tests/GitHub.test.js

@@ -3,11 +3,12 @@
 import { GitHub } from "../GitHub"
 import Fake from "../../ci_source/Fake"
 
-/** Gets a mocked out GitHub class for checking a get path */
+// Gets a mocked out GitHub class for checking a get path
 const mockGitHubWithGetForPath = (expectedPath): GitHub => {
   const mockSource = new Fake({})
   const github = new GitHub("Token", mockSource)
 
+  // $FlowFixMe: Ignore an error from setting an unexistant func
   github.get = (path: string, headers: any = {}, body: any = {}, method: string = "GET"): Promise<any> => {
     return new Promise((resolve: any, reject: any) => {
       expect(path).toBe(expectedPath)
@@ -26,9 +27,8 @@ describe("API results", () => {
   })
 
   it("sets the correct paths for getPullRequestDiff", () => {
-    const expectedPath = "repos/artsy/emission/issues/327/comments"
+    const expectedPath = "repos/artsy/emission/pulls/327"
     const github = mockGitHubWithGetForPath(expectedPath)
     expect(github.getPullRequestDiff())
   })
-
 })

source/platforms/platform.js

@@ -1,7 +1,7 @@
 // @flow
 
 import type { Env, CISource } from "../ci_source/ci_source"
-import type { GitDSL } from "../dsl/git"
+import type { GitDSL } from "../dsl/GitDSL"
 
 /** A type that represents the downloaded metadata about a code review session */
 export type Metadata = any;
@@ -38,7 +38,7 @@ export interface Platform {
     /** Pulls in the Code Review Diff, and offers a succinct user-API for it */
     getReviewDiff: () => Promise<GitDSL>;
     /** Creates a comment on the PR */
-    createComment: (body: string) => Promise<?Comment>;
+    createComment: (body: string) => Promise<any>;
     /** Delete the main Danger comment */
     deleteMainComment: () => Promise<bool>;
     /** Replace the main Danger comment */