Skip to content

Commit

Permalink
feat: add option 'removePreviousComments'
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhouweling committed Dec 25, 2020
1 parent f502880 commit 1fe474b
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 3 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 @@ -93,3 +93,27 @@ it.skip("passes the dangerID from args into the executor config", async () => {
const executor: Executor = mockRunDangerSubprocess.mock.calls[0][2]
expect(executor.options.dangerID).toEqual("test-danger-run")
})

it("sets removePreviousComments 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.removePreviousComments).toEqual(false)
})

it("passes removePreviousComments option if requested", async () => {
const customArgs = {
...defaultAppArgs,
removePreviousComments: 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.removePreviousComments).toEqual(true)
})
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,
removePreviousComments: app.removePreviousComments || false,
}

const processName = (app.process && addSubprocessCallAguments(app.process.split(" "))) || undefined
Expand Down
7 changes: 7 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
/** Removes all previous comment and create a new one in the end of the list */
removePreviousComments?: boolean
}

export default (command: any) =>
Expand All @@ -46,3 +48,8 @@ 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(
"--removePreviousComments",
"Removes all previous comment and create a new one in the end of the list",
false
)
15 changes: 12 additions & 3 deletions 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
/** Removes all previous comment and create a new one in the end of the list */
removePreviousComments?: boolean
}
// This is still badly named, maybe it really should just be runner?

Expand Down Expand Up @@ -252,19 +254,26 @@ export class Executor {

const dangerID = this.options.dangerID
const failed = fails.length > 0
const hasMessages = failureCount + messageCount > 0

let issueURL = undefined

if (failureCount + messageCount === 0) {
this.log(`Found no issues or messages from Danger. Removing any existing messages on ${this.platform.name}.`)
if (!hasMessages || this.options.removePreviousComments) {
if (!hasMessages) {
this.log(`Found no issues or messages from Danger. Removing any existing messages on ${this.platform.name}.`)
} else {
this.log(`'removePreviousComments' option specified. Removing any existing messages on ${this.platform.name}.`)
}
await this.platform.deleteMainComment(dangerID)
const previousComments = await this.platform.getInlineComments(dangerID)
for (const comment of previousComments) {
if (comment && comment.ownedByDanger) {
await this.deleteInlineComment(comment)
}
}
} else {
}

if (hasMessages) {
if (fails.length > 0) {
const s = fails.length === 1 ? "" : "s"
const are = fails.length === 1 ? "is" : "are"
Expand Down
37 changes: 37 additions & 0 deletions source/runner/_tests/_executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,22 @@ describe("setup", () => {
expect(platform.deleteMainComment).toBeCalled()
})

it("Deletes a post when 'removePreviousComments' option has been specified", async () => {
const platform = new FakePlatform()
const exec = new Executor(
new FakeCI({}),
platform,
inlineRunner,
{ ...defaultConfig, removePreviousComments: true },
new FakeProcces()
)
const dsl = await defaultDsl(platform)
platform.deleteMainComment = jest.fn()

await exec.handleResults(warnResults, dsl.git)
expect(platform.deleteMainComment).toBeCalled()
})

it("Fails if the failOnErrors option is true and there are fails on the build", async () => {
const platform = new FakePlatform()
const strictConfig: ExecutorOptions = {
Expand Down Expand Up @@ -299,6 +315,27 @@ describe("setup", () => {
expect(platform.deleteInlineComment).toHaveBeenCalledTimes(2)
})

it("Deletes all old inline comments when 'removePreviousComments' option has been specified", async () => {
const platform = new FakePlatform()
const exec = new Executor(
new FakeCI({}),
platform,
inlineRunner,
{ ...defaultConfig, removePreviousComments: true },
new FakeProcces()
)
const dsl = await defaultDsl(platform)
const previousComments = mockPayloadForResults(inlineMultipleWarnResults)

platform.getInlineComments = jest.fn().mockResolvedValueOnce(previousComments).mockResolvedValueOnce([])
platform.updateInlineComment = jest.fn()
platform.createInlineComment = jest.fn()
platform.deleteInlineComment = jest.fn()

await exec.handleResults(warnResults, dsl.git)
expect(platform.deleteInlineComment).toHaveBeenCalledTimes(3)
})

it("Deletes old inline comment when not applicable in new results", async () => {
const platform = new FakePlatform()
const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig, new FakeProcces())
Expand Down

0 comments on commit 1fe474b

Please sign in to comment.