Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Jenkins CI source #61

Merged
merged 2 commits into from
Dec 27, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

// Add your own contribution below

* Add Jenkins CI source - macklinu

### 0.7.0


Expand Down
39 changes: 39 additions & 0 deletions source/ci_source/Jenkins.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// @flow
"use strict"

import type { Env } from "./ci_source"
import { ensureEnvKeysExist, ensureEnvKeysAreInt } from "./ci_source_helpers"

export default class Jenkins {
env: Env

constructor(env: Env) {
this.env = env
}

get name(): string {
return "Jenkins"
}

get isCI(): boolean {
return ensureEnvKeysExist(this.env, ["JENKINS_URL"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

get isPR(): boolean {
const mustHave = ["JENKINS_URL", "ghprbPullId", "ghprbGhRepository"]
const mustBeInts = ["ghprbPullId"]
return ensureEnvKeysExist(this.env, mustHave) && ensureEnvKeysAreInt(this.env, mustBeInts)
}

get pullRequestID(): string {
return this.env.ghprbPullId
}

get repoSlug(): string {
return this.env.ghprbGhRepository
}

get supportedPlatforms(): string[] {
return ["github"]
}
}
67 changes: 67 additions & 0 deletions source/ci_source/_tests/_jenkins.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import Jenkins from "../Jenkins"

const correctEnv = {
"ghprbGhRepository": "danger/danger-js",
"ghprbPullId": "50",
"JENKINS_URL": "https://danger.jenkins"
}

describe(".isCI", () => {
it("validates when JENKINS_URL is present in environment", () => {
const jenkins = new Jenkins(correctEnv)
expect(jenkins.isCI).toBeTruthy()
})

it("does not validate without JENKINS_URL", () => {
const jenkins = new Jenkins({})
expect(jenkins.isCI).toBeFalsy()
})
})

describe(".isPR", () => {
it("validates when all Jenkins environment variables are set", () => {
const jenkins = new Jenkins(correctEnv)
expect(jenkins.isPR).toBeTruthy()
})

it("does not validate with required environment variables", () => {
const jenkins = new Jenkins({})
expect(jenkins.isPR).toBeFalsy()
})

const envs = ["JENKINS_URL", "ghprbPullId", "ghprbGhRepository"]
envs.forEach((key: string) => {
const env = {
...correctEnv,
[key]: null
}

it(`does not validate when ${key} is missing`, () => {
const jenkins = new Jenkins(env)
expect(jenkins.isPR).toBeFalsy()
})
})

it("needs to have a PR number", () => {
const env = {
...correctEnv,
"ghprbPullId": "not a number"
}
const jenkins = new Jenkins(env)
expect(jenkins.isPR).toBeFalsy()
})
})

describe(".pullRequestID", () => {
it("pulls it out of environment", () => {
const jenkins = new Jenkins(correctEnv)
expect(jenkins.pullRequestID).toEqual("50")
})
})

describe(".repoSlug", () => {
it("pulls it out of environment", () => {
const jenkins = new Jenkins(correctEnv)
expect(jenkins.repoSlug).toEqual("danger/danger-js")
})
})
2 changes: 1 addition & 1 deletion source/ci_source/_tests/_travis.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe(".isPR", () => {
env[key] = null

test(`does not validate when ${key} is missing`, () => {
const travis = new Travis({})
const travis = new Travis(env)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Travis CI source test wasn't validating the env where env[key] was null. Updated this test to do so (which I think it intends to do).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

expect(travis.isPR).toBeFalsy()
})
})
Expand Down
9 changes: 5 additions & 4 deletions source/ci_source/ci_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type Env = any;

/** The shape of an object that represents an individual CI */
export interface CISource {
/** The name, mainly for showing errors */
/** The project name, mainly for showing errors */
+name: string,

/** The hash of environment variables */
Expand All @@ -26,14 +26,12 @@ export interface CISource {

/** What unique id can be found for the code review platform's PR */
+pullRequestID: string,

/** What is the project name */
+name: string
}

import Travis from "./Travis"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dupe, removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one

import Circle from "./Circle"
import Semaphore from "./Semaphore"
import Jenkins from "./Jenkins"
import Fake from "./Fake"

/**
Expand All @@ -46,6 +44,7 @@ export function getCISourceForEnv(env: Env): ?CISource {
const travis = new Travis(env)
const circle = new Circle(env)
const semaphore = new Semaphore(env)
const jenkins = new Jenkins(env)
const fake = new Fake(env)

if (travis.isCI) {
Expand All @@ -54,6 +53,8 @@ export function getCISourceForEnv(env: Env): ?CISource {
return circle
} else if (semaphore.isCI) {
return semaphore
} else if (jenkins.isCI) {
return jenkins
} else if (fake.isCI) {
return fake
}
Expand Down
2 changes: 1 addition & 1 deletion source/ci_source/ci_source_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { Env } from "./ci_source"
*/
export function ensureEnvKeysExist(env: Env, keys: string[]): boolean {
const hasKeys = keys.map((key: string): boolean => {
return env.hasOwnProperty(key) && env[key].length > 0
return env.hasOwnProperty(key) && env[key] != null && env[key].length > 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Travis (and now Jenkins) CI source tests set an env[key] to null in the tests (our way of saying the key exists but there is no value for that key). This function was attempting to access .length on null, which throws a TypeError, so I believe this null/undefined check is useful here in case the env object has a supplied property but its value is falsy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep 👍

})
return !hasKeys.includes(false)
}
Expand Down