Skip to content

Commit

Permalink
feat: add option 'newComment'
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhouweling committed Dec 25, 2020
1 parent 1fe474b commit a572368
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 4 deletions.
24 changes: 24 additions & 0 deletions source/commands/ci/_tests/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,30 @@ it.skip("passes the dangerID from args into the executor config", async () => {
expect(executor.options.dangerID).toEqual("test-danger-run")
})

it("sets newComment to false by default", async () => {
await runRunner(defaultAppArgs as SharedCLI, { platform, source })

// Pull the executor out of the call to runDangerSubprocess
const executor: Executor = mockRunDangerSubprocess.mock.calls[0][2]
expect(executor.ciSource).toEqual(source)
expect(executor.platform).toEqual(platform)
expect(executor.options.newComment).toEqual(false)
})

it("passes newComment option if requested", async () => {
const customArgs = {
...defaultAppArgs,
newComment: true,
} as SharedCLI
await runRunner(customArgs, { platform, source })

// Pull the executor out of the call to runDangerSubprocess
const executor: Executor = mockRunDangerSubprocess.mock.calls[0][2]
expect(executor.ciSource).toEqual(source)
expect(executor.platform).toEqual(platform)
expect(executor.options.newComment).toEqual(true)
})

it("sets removePreviousComments to false by default", async () => {
await runRunner(defaultAppArgs as SharedCLI, { platform, source })

Expand Down
1 change: 1 addition & 0 deletions source/commands/ci/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const runRunner = async (app: SharedCLI, config?: Partial<RunnerConfig>)
failOnErrors: app.failOnErrors,
noPublishCheck: !app.publishCheck,
ignoreOutOfDiffComments: app.ignoreOutOfDiffComments,
newComment: app.newComment || false,
removePreviousComments: app.removePreviousComments || false,
}

Expand Down
3 changes: 3 additions & 0 deletions source/commands/utils/sharedDangerfileArgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export interface SharedCLI extends program.CommanderStatic {
useGithubChecks: boolean
/** Ignore inline-comments that are in lines which were not changed */
ignoreOutOfDiffComments: boolean
/** Makes Danger post a new comment instead of editing its previous one */
newComment?: boolean
/** Removes all previous comment and create a new one in the end of the list */
removePreviousComments?: boolean
}
Expand All @@ -48,6 +50,7 @@ export default (command: any) =>
.option("-f, --failOnErrors", "Fail if there are fails in the danger report", false)
.option("--use-github-checks", "Use GitHub Checks", false)
.option("--ignoreOutOfDiffComments", "Ignore inline-comments that are in lines which were not changed", false)
.option("--newComment", "Makes Danger post a new comment instead of editing its previous one", false)
.option(
"--removePreviousComments",
"Removes all previous comment and create a new one in the end of the list",
Expand Down
8 changes: 7 additions & 1 deletion source/runner/Executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export interface ExecutorOptions {
noPublishCheck?: boolean
/** Ignore inline-comments that are in lines which were not changed */
ignoreOutOfDiffComments: boolean
/** Makes Danger post a new comment instead of editing its previous one */
newComment?: boolean
/** Removes all previous comment and create a new one in the end of the list */
removePreviousComments?: boolean
}
Expand Down Expand Up @@ -312,7 +314,11 @@ export class Executor {
comment = githubResultsTemplate(dangerID, mergedResults, commitID)
}

issueURL = await this.platform.updateOrCreateComment(dangerID, comment)
if (this.options.newComment) {
issueURL = await this.platform.createComment(dangerID, comment)
} else {
issueURL = await this.platform.updateOrCreateComment(dangerID, comment)
}
this.log(`Feedback: ${issueURL}`)
}
}
Expand Down
39 changes: 36 additions & 3 deletions source/runner/_tests/_executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,22 @@ describe("setup", () => {
expect(platform.updateOrCreateComment).toBeCalled()
})

it("Updates or Creates comments for warnings", async () => {
it("Creates comments (rather than update or create) for warnings when newComment option is passed", async () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
const exec = new Executor(
new FakeCI({}),
platform,
inlineRunner,
{ ...defaultConfig, newComment: true },
new FakeProcces()
)
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()

await exec.handleResults(warnResults, dsl.git)
expect(platform.updateOrCreateComment).toBeCalled()
expect(platform.createComment).toBeCalled()
expect(platform.updateOrCreateComment).not.toBeCalled()
})

it("Updates or Creates comments for warnings, without GitDSL", async () => {
Expand All @@ -186,6 +194,23 @@ describe("setup", () => {
expect(platform.updateOrCreateComment).toBeCalled()
})

it("Creates comments (rather than update or create) for warnings, without GitDSL, when newComment option is passed", async () => {
const platform = new FakePlatform()
const exec = new Executor(
new FakeCI({}),
platform,
inlineRunner,
{ ...defaultConfig, newComment: true },
new FakeProcces()
)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()

await exec.handleResults(warnResults)
expect(platform.createComment).toBeCalled()
expect(platform.updateOrCreateComment).not.toBeCalled()
})

it("Sends inline comments and returns regular results for failures", async () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
Expand All @@ -201,6 +226,7 @@ describe("setup", () => {
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createInlineComment = jest.fn()
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()

await exec.handleResults(inlineWarnResults, dsl.git)
Expand All @@ -213,9 +239,11 @@ describe("setup", () => {
const exec = new Executor(new FakeCI({}), platform, inlineRunner, config, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createInlineComment = jest.fn().mockReturnValue(new Promise<any>((_, reject) => reject()))
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()

await exec.handleResults(inlineWarnResults, dsl.git)
expect(platform.createComment).not.toBeCalled()
expect(platform.updateOrCreateComment).not.toBeCalled()
})

Expand Down Expand Up @@ -369,6 +397,7 @@ describe("setup", () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()
platform.updateStatus = jest.fn()

Expand All @@ -380,6 +409,7 @@ describe("setup", () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()
platform.updateStatus = jest.fn()

Expand All @@ -396,6 +426,7 @@ describe("setup", () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()
platform.updateStatus = jest.fn()

Expand All @@ -415,6 +446,7 @@ describe("setup", () => {

const exec = new Executor(ci, platform, inlineRunner, defaultConfig, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()
platform.updateStatus = jest.fn()

Expand All @@ -434,6 +466,7 @@ describe("setup", () => {

const exec = new Executor(ci, platform, inlineRunner, config, new FakeProcces())
const dsl = await defaultDsl(platform)
platform.createComment = jest.fn()
platform.updateOrCreateComment = jest.fn()
platform.updateStatus = jest.fn()

Expand Down

0 comments on commit a572368

Please sign in to comment.