From c669396a8f453d3598fb63d9829909f3483dd795 Mon Sep 17 00:00:00 2001 From: Marcos Ojeda Date: Fri, 18 Nov 2016 15:15:17 -0800 Subject: [PATCH 1/4] add circle ci detection for danger This is very much similar to the travis tests, but with env vars updated to match circle env var settings (and not the almighty word of Josh). Adds jest tests for a circle env. --- source/ci_source/Circle.js | 53 ++++++++++++++++ source/ci_source/_tests/_circle.test.js | 83 +++++++++++++++++++++++++ source/ci_source/ci_source.js | 5 ++ 3 files changed, 141 insertions(+) create mode 100644 source/ci_source/Circle.js create mode 100644 source/ci_source/_tests/_circle.test.js diff --git a/source/ci_source/Circle.js b/source/ci_source/Circle.js new file mode 100644 index 000000000..cb883d072 --- /dev/null +++ b/source/ci_source/Circle.js @@ -0,0 +1,53 @@ +// @flow +"use strict" + +import type { Env } from "./ci_source" +import { ensureEnvKeysExist, ensureEnvKeysAreInt } from "./ci_source_helpers" +export default class Circle { + env: Env + constructor(env: Env) { this.env = env } + + get name(): string { return "Circle CI" } + + get isCI() : boolean { + return ensureEnvKeysExist(this.env, ["CIRCLE_BUILD_NUM"]) + } + + get isPR() : boolean { + if (ensureEnvKeysExist(this.env, ["CI_PULL_REQUEST"])) { + return true + } + + const mustHave = ["CIRCLE_CI_API_TOKEN", "CIRCLE_PROJECT_USERNAME", "CIRCLE_PROJECT_REPONAME", "CIRCLE_BUILD_NUM"] + return ensureEnvKeysExist(this.env, mustHave) && ensureEnvKeysAreInt(this.env, ["CIRCLE_PR_NUMBER"]) + } + + _prParseURL(): {owner?: string, reponame?: string, id?: string} { + const pr_url = this.env.CI_PULL_REQUEST || ''; + const split_slug = pr_url.split('/') + if (split_slug.length === 7) { + const owner = split_slug[3] + const reponame = split_slug[4] + const id = split_slug[6] + return {owner, reponame, id} + } + return {} + } + + get pullRequestID(): string { + if (this.env.CIRCLE_PR_NUMBER) { + return this.env.CIRCLE_PR_NUMBER + } else { + const {id} = this._prParseURL() + return id ? id : '' + } + } + + get repoSlug(): string { + const {owner, reponame} = this._prParseURL() + return (owner && reponame) ? `${owner}/${reponame}` : '' + } + + get repoURL(): string { return this.env.CIRCLE_REPOSITORY_URL } + get supportedPlatforms() : string[] { return ["github"] } +} diff --git a/source/ci_source/_tests/_circle.test.js b/source/ci_source/_tests/_circle.test.js new file mode 100644 index 000000000..a397fc207 --- /dev/null +++ b/source/ci_source/_tests/_circle.test.js @@ -0,0 +1,83 @@ +import Circle from "../Circle" + +const correctEnv = { + "CIRCLE_CI_API_TOKEN":"xxx", + "CIRCLE_PROJECT_USERNAME":"circle_org", + "CIRCLE_PROJECT_REPONAME":"someproject", + "CIRCLE_BUILD_NUM": "1501", + "CIRCLE_PR_NUMBER": "800", + "CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800" +} + +describe(".isCI", () => { + test("validates when all Circle environment vars are set", () => { + const circle = new Circle(correctEnv) + expect(circle.isCI).toBeTruthy() + }) + + test("does not validate without josh", () => { + const circle = new Circle({}) + expect(circle.isCI).toBeFalsy() + }) +}) + +describe(".isPR", () => { + test("validates when all circle environment vars are set", () => { + const circle = new Circle(correctEnv) + expect(circle.isPR).toBeTruthy() + }) + + test("does not validate outside of circle", () => { + const circle = new Circle({}) + expect(circle.isPR).toBeFalsy() + }) + + const envs = ["CIRCLE_CI_API_TOKEN", "CIRCLE_PROJECT_USERNAME", "CIRCLE_PROJECT_REPONAME", "CIRCLE_BUILD_NUM"] + envs.forEach((key: string) => { + var env = { + "CIRCLE_CI_API_TOKEN":"xxx", + "CIRCLE_PROJECT_USERNAME":"circle_org", + "CIRCLE_PROJECT_REPONAME":"someproject", + "CIRCLE_BUILD_NUM": "1501", + "CIRCLE_PR_NUMBER": "800", + "CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800" + } + env[key] = null + + test(`does not validate when ${key} is missing`, () => { + const circle = new Circle({}) + expect(circle.isPR).toBeFalsy() + }) + }) + + it("needs to have a PR number", () => { + var env = { + "CIRCLE_PR_NUMBER": "asdasd", + "CIRCLE_REPO_SLUG": "artsy/eigen" + } + const circle = new Circle(env) + expect(circle.isPR).toBeFalsy() + }) +}) + +describe(".pullReuestID", () => { + it("pulls it out of the env", () => { + const circle = new Circle({CIRCLE_PR_NUMBER: "800"}) + expect(circle.pullRequestID).toEqual("800") + }) + + it("can derive it from PR Url", () => { + var env = { + "CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/23" + } + const circle = new Circle(env) + expect(circle.pullRequestID).toEqual("23") + }) +}) + +describe(".repoSlug", () => { + it("derives it from the PR Url", () => { + const circle = new Circle(correctEnv) + expect(circle.repoSlug).toEqual("artsy/eigen") + }) +}) diff --git a/source/ci_source/ci_source.js b/source/ci_source/ci_source.js index 0c3dc6601..4c02b169d 100644 --- a/source/ci_source/ci_source.js +++ b/source/ci_source/ci_source.js @@ -35,6 +35,7 @@ export interface CISource { } import Travis from "./Travis" +import Circle from "./Circle" import Fake from "./Fake" /** @@ -46,8 +47,12 @@ import Fake from "./Fake" export function getCISourceForEnv(env: Env) : ?CISource { // Fake is what I'm using during dev for the minute const travis = new Travis(env) + const circle = new Circle(env) + if (travis.isCI) { return travis + } else if (circle.isCI) { + return circle } else { return new Fake() } From 272f64e8318d1f2ca814c86e63dcc729159af8b1 Mon Sep 17 00:00:00 2001 From: Marcos Ojeda Date: Fri, 18 Nov 2016 16:14:02 -0800 Subject: [PATCH 2/4] adds circle.yml file for circleci --- circle.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 circle.yml diff --git a/circle.yml b/circle.yml new file mode 100644 index 000000000..3db508af2 --- /dev/null +++ b/circle.yml @@ -0,0 +1,12 @@ +machine: + node: + version: 6.1 + +test: + override: + - npm run build + - npm run link + - danger + post: + - npm run flow + - npm run lint From deada875fbc91f75bfed1b599a106bfed31db643 Mon Sep 17 00:00:00 2001 From: Marcos Ojeda Date: Fri, 18 Nov 2016 16:26:17 -0800 Subject: [PATCH 3/4] adds changelog and loosens flow checks for test.js files --- changelog.md | 20 +++++++++++++------- dangerfile.js | 8 +++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/changelog.md b/changelog.md index 92aa60bd1..971a588ba 100644 --- a/changelog.md +++ b/changelog.md @@ -2,12 +2,18 @@ // Add your own contribution below +### 0.6.0 + +* omits flow requirement for new test files +* adds support for circleci +* defines CISource properties in flow as read-only + ### 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 +* 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 @@ -22,7 +28,7 @@ * Danger will post a comment on a GitHub PR with any Fails - orta -### 0.0.2 +### 0.0.2 OK, first usable for others version. Only supports GitHub and Travis CI. @@ -30,7 +36,7 @@ You can run by doing: ```sh danger -``` +``` Make sure you set a `DANGER_GITHUB_API_TOKEN` on your CI - [see the Ruby guide](http://danger.systems/guides/getting_started.html#setting-up-danger-to-run-on-your-ci) for that. @@ -42,13 +48,13 @@ git fail(message: string) ``` -`pr` _probably_ won't be sticking around for the long run, but if you're using a `0.0.2` release, you should be OK with that. It's the full metadata of the PR, so [this JSON file](https://raw.githubusercontent.com/danger/danger/master/spec/fixtures/github_api/pr_response.json). +`pr` _probably_ won't be sticking around for the long run, but if you're using a `0.0.2` release, you should be OK with that. It's the full metadata of the PR, so [this JSON file](https://raw.githubusercontent.com/danger/danger/master/spec/fixtures/github_api/pr_response.json). `git` currently has: ```sh git.modified_file git.created_files -git.deleted_files +git.deleted_files ``` which are string arrays of files. @@ -66,10 +72,10 @@ if (!hasChangelog) { } ``` -That should do ya. I think. This doens't support babel, and I haven't explored using other modules etc, so +That should do ya. I think. This doens't support babel, and I haven't explored using other modules etc, so ./ ### 0.0.1 -Not usable for others, only stubs of classes etc. \ No newline at end of file +Not usable for others, only stubs of classes etc. diff --git a/dangerfile.js b/dangerfile.js index e0b315dca..2a3d9aa0b 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -12,9 +12,11 @@ if (!hasChangelog) { 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") +// but exclude tests from being flow-ey +const unFlowedFiles = jsFiles.filter(path => !path.endsWith('test.js')) + .filter(filepath => { + const content = fs.readFileSync(filepath) + return !content.includes("@flow") }) if (unFlowedFiles.length > 0) { From 8f173e8da31ff512207510f98715b72918cd971a Mon Sep 17 00:00:00 2001 From: Marcos Ojeda Date: Fri, 18 Nov 2016 16:47:58 -0800 Subject: [PATCH 4/4] :shirt: fix lint issues --- dangerfile.js | 4 ++-- source/ci_source/Circle.js | 22 +++++++++++----------- source/ci_source/_tests/_circle.test.js | 12 ++++++------ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/dangerfile.js b/dangerfile.js index 2a3d9aa0b..90f35a092 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -13,11 +13,11 @@ const jsFiles = danger.git.created_files.filter(path => path.endsWith("js")) // new js files should have `@flow` at the top // but exclude tests from being flow-ey -const unFlowedFiles = jsFiles.filter(path => !path.endsWith('test.js')) +const unFlowedFiles = jsFiles.filter(path => !path.endsWith("test.js")) .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(", ")}`) diff --git a/source/ci_source/Circle.js b/source/ci_source/Circle.js index cb883d072..540b3e6f2 100644 --- a/source/ci_source/Circle.js +++ b/source/ci_source/Circle.js @@ -9,11 +9,11 @@ export default class Circle { get name(): string { return "Circle CI" } - get isCI() : boolean { + get isCI(): boolean { return ensureEnvKeysExist(this.env, ["CIRCLE_BUILD_NUM"]) } - get isPR() : boolean { + get isPR(): boolean { if (ensureEnvKeysExist(this.env, ["CI_PULL_REQUEST"])) { return true } @@ -23,12 +23,12 @@ export default class Circle { } _prParseURL(): {owner?: string, reponame?: string, id?: string} { - const pr_url = this.env.CI_PULL_REQUEST || ''; - const split_slug = pr_url.split('/') - if (split_slug.length === 7) { - const owner = split_slug[3] - const reponame = split_slug[4] - const id = split_slug[6] + const prUrl = this.env.CI_PULL_REQUEST || "" + const splitSlug = prUrl.split("/") + if (splitSlug.length === 7) { + const owner = splitSlug[3] + const reponame = splitSlug[4] + const id = splitSlug[6] return {owner, reponame, id} } return {} @@ -39,15 +39,15 @@ export default class Circle { return this.env.CIRCLE_PR_NUMBER } else { const {id} = this._prParseURL() - return id ? id : '' + return id || "" } } get repoSlug(): string { const {owner, reponame} = this._prParseURL() - return (owner && reponame) ? `${owner}/${reponame}` : '' + return (owner && reponame) ? `${owner}/${reponame}` : "" } get repoURL(): string { return this.env.CIRCLE_REPOSITORY_URL } - get supportedPlatforms() : string[] { return ["github"] } + get supportedPlatforms(): string[] { return ["github"] } } diff --git a/source/ci_source/_tests/_circle.test.js b/source/ci_source/_tests/_circle.test.js index a397fc207..4abf258f0 100644 --- a/source/ci_source/_tests/_circle.test.js +++ b/source/ci_source/_tests/_circle.test.js @@ -1,9 +1,9 @@ import Circle from "../Circle" const correctEnv = { - "CIRCLE_CI_API_TOKEN":"xxx", - "CIRCLE_PROJECT_USERNAME":"circle_org", - "CIRCLE_PROJECT_REPONAME":"someproject", + "CIRCLE_CI_API_TOKEN": "xxx", + "CIRCLE_PROJECT_USERNAME": "circle_org", + "CIRCLE_PROJECT_REPONAME": "someproject", "CIRCLE_BUILD_NUM": "1501", "CIRCLE_PR_NUMBER": "800", "CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800" @@ -35,9 +35,9 @@ describe(".isPR", () => { const envs = ["CIRCLE_CI_API_TOKEN", "CIRCLE_PROJECT_USERNAME", "CIRCLE_PROJECT_REPONAME", "CIRCLE_BUILD_NUM"] envs.forEach((key: string) => { var env = { - "CIRCLE_CI_API_TOKEN":"xxx", - "CIRCLE_PROJECT_USERNAME":"circle_org", - "CIRCLE_PROJECT_REPONAME":"someproject", + "CIRCLE_CI_API_TOKEN": "xxx", + "CIRCLE_PROJECT_USERNAME": "circle_org", + "CIRCLE_PROJECT_REPONAME": "someproject", "CIRCLE_BUILD_NUM": "1501", "CIRCLE_PR_NUMBER": "800", "CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800"