Add Jenkins CI source #61
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves #56
macklinu
commented
Dec 27, 2016
| } | ||
|
|
||
| get isCI(): boolean { | ||
| return ensureEnvKeysExist(this.env, ["JENKINS_URL"]) |
There was a problem hiding this comment.
| @@ -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 | |||
| @@ -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) | |||
There was a problem hiding this comment.
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).
| @@ -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 | |||
There was a problem hiding this comment.
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.
orta
reviewed
Dec 27, 2016
| @@ -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) | |||
| @@ -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 | |||
| @@ -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 | |||
|
Will merge on 🍏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #56