Skip to content

Commit

Permalink
Merge pull request #548 from danger/validate
Browse files Browse the repository at this point in the history
Adds support for a validation step on the danger results
  • Loading branch information
orta committed Mar 31, 2018
2 parents 0c7e7d7 + 02dd89c commit a31bdf0
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@

-- [@sunshinejr][]

* Adds a data validation step when Danger gets results back from a process . [@orta][]

## 3.3.2

* Adds support for TeamCity as a CI provider. [@fwal][]
Expand Down
32 changes: 32 additions & 0 deletions source/dsl/DangerResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,38 @@ export const emptyDangerResults = {
markdowns: [],
}

export function validateResults(results: DangerResults) {
// The data we get back is JSON sent by STDIN that can come from many
// consumers, let's take the time to ensure the data is how we think it is.
const { fails, warnings, messages, markdowns } = results
const props = { fails, warnings, messages, markdowns }
Object.keys(props).forEach(name => {
//
// Must include the key 4 types
if (!props[name]) {
throw new Error(`Results passed to Danger JS did not include ${name}.\n\n${JSON.stringify(results, null, " ")}`)
}

const violations: Violation[] = props[name]
violations.forEach(v => {
// They should always have a message
if (!v.message) {
throw new Error(
`A violation passed to Danger JS in ${name} did not include \`message\`.\n\n${JSON.stringify(v, null, " ")}`
)
}
// Warn if anything other than the initial API is on a violation
const officialAPI = ["message", "line", "file"]
const keys = Object.keys(v).filter(f => !officialAPI.includes(f))
if (keys.length) {
console.warn(
`Recieved unexpected key in Violation, expected only ${officialAPI} but recieved ${Object.keys(v)}`
)
}
})
})
}

/** Returns only the inline violations from Danger results */
export function inlineResults(results: DangerResults): DangerResults {
return {
Expand Down
25 changes: 25 additions & 0 deletions source/dsl/_tests/DangerResults.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
inlineResults,
regularResults,
sortInlineResults,
validateResults,
} from "../DangerResults"
import {
singleViolationSingleFileResults,
Expand Down Expand Up @@ -89,3 +90,27 @@ describe("DangerResults operations", () => {
expect(results).toMatchSnapshot()
})
})

describe("validation", () => {
it("validates the presence of all result types", () => {
const badResults = { fails: [], markdowns: [], other: [] }

expect(() => {
validateResults(badResults as any)
}).toThrowErrorMatchingSnapshot()
})

it("validates the presence of message in a violation result types", () => {
const badResults = { fails: [{}], markdowns: [], warnings: [], messages: [] }

expect(() => {
validateResults(badResults as any)
}).toThrowErrorMatchingSnapshot()
})

it("does not throw for correct results", () => {
expect(() => {
validateResults(singleViolationSingleFileResults)
}).not.toThrow()
})
})
16 changes: 16 additions & 0 deletions source/dsl/_tests/__snapshots__/DangerResults.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,19 @@ Array [
},
]
`;

exports[`validation validates the presence of all result types 1`] = `
"Results passed to Danger JS did not include warnings.
{
\\"fails\\": [],
\\"markdowns\\": [],
\\"other\\": []
}"
`;

exports[`validation validates the presence of message in a violation result types 1`] = `
"A violation passed to Danger JS in fails did not include \`message\`.
{}"
`;
2 changes: 0 additions & 2 deletions source/platforms/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ export interface Platform {
/** Creates a comment on the PR */
createComment: (dangerID: string, body: string) => Promise<any>
/** Creates an inline comment on the PR if possible */
// Here we pass GitDSL because platforms have different endpoints for inline comments
// Wasn't sure if passing the dsl is the best way of achieving this, though
createInlineComment: (git: GitDSL, comment: string, path: string, line: number) => Promise<any>
/** Updates an inline comment */
updateInlineComment: (comment: string, commentId: string) => Promise<any>
Expand Down
3 changes: 3 additions & 0 deletions source/runner/Executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
inlineResultsIntoResults,
sortResults,
sortInlineResults,
validateResults,
} from "../dsl/DangerResults"
import {
template as githubResultsTemplate,
Expand Down Expand Up @@ -117,6 +118,8 @@ export class Executor {
* @param {DangerResults} results a JSON representation of the end-state for a Danger run
*/
async handleResults(results: DangerResults, git: GitDSL) {
validateResults(results)

this.d(`Got Results back, current settings`, this.options)
if (this.options.stdoutOnly || this.options.jsonOnly) {
await this.handleResultsPostingToSTDOUT(results)
Expand Down

0 comments on commit a31bdf0

Please sign in to comment.