Merge branch 'master' into patch-1

CHANGELOG.md

@@ -17,7 +17,10 @@
 
 - Add `DANGER_DISABLE_TSC` environment variable to disable transpiling with tsc, providing a way to force transpiling
   with Babel - [@ozzieorca]
-  <!-- Your comment above this -->
+- Adds options `--newComment` and `--removePreviousComments` - [@davidhouweling]
+- Add support for a file path filter when calculation lines of code - [@melvinvermeer]
+
+<!-- Your comment above this -->
 
 # 10.5.4
 

source/commands/ci/_tests/runner.test.ts

@@ -93,3 +93,51 @@ 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 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 })
+
+  // 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)
+})

source/commands/ci/runner.ts

@@ -69,6 +69,8 @@ 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,
       }
 
       const processName = (app.process && addSubprocessCallAguments(app.process.split(" "))) || undefined

source/commands/utils/sharedDangerfileArgs.ts

@@ -29,6 +29,10 @@ 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
 }
 
 export default (command: any) =>
@@ -46,3 +50,9 @@ 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",
+      false
+    )

source/dsl/GitDSL.ts

@@ -185,6 +185,8 @@ export interface GitDSL extends GitJSONDSL {
 
   /**
    * Offers the overall lines of code added/removed in the diff
+   *
+   * @param {string} pattern an option glob pattern to filer files that will considered for lines of code.
    */
-  linesOfCode(): Promise<number | null>
+  linesOfCode(pattern?: string): Promise<number | null>
 }

source/platforms/git/gitJSONToGitDSL.ts

@@ -10,6 +10,8 @@ import * as jsonDiff from "fast-json-patch"
 import jsonpointer from "jsonpointer"
 import JSON5 from "json5"
 
+import micromatch from "micromatch"
+
 import { GitDSL, JSONPatchOperation, GitJSONDSL, StructuredDiff } from "../../dsl/GitDSL"
 import chainsmoker from "../../commands/utils/chainsmoker"
 
@@ -155,11 +157,13 @@ export const gitJSONToGitDSL = (gitJSONRep: GitJSONDSL, config: GitJSONToGitDSLC
     }, Object.create(null))
   }
 
-  const linesOfCode = async () => {
+  const linesOfCode = async (pattern?: string) => {
+    const isPatternMatch = (path: string) => pattern === undefined || micromatch.isMatch(path, pattern)
+
     const [createdFilesDiffs, modifiedFilesDiffs, deletedFilesDiffs] = await Promise.all([
-      Promise.all(gitJSONRep.created_files.map(path => diffForFile(path))),
-      Promise.all(gitJSONRep.modified_files.map(path => diffForFile(path))),
-      Promise.all(gitJSONRep.deleted_files.map(path => diffForFile(path))),
+      Promise.all(gitJSONRep.created_files.filter(isPatternMatch).map(path => diffForFile(path))),
+      Promise.all(gitJSONRep.modified_files.filter(isPatternMatch).map(path => diffForFile(path))),
+      Promise.all(gitJSONRep.deleted_files.filter(isPatternMatch).map(path => diffForFile(path))),
     ])
 
     let additions = createdFilesDiffs

source/platforms/github/_tests/_github_git.test.ts

@@ -102,6 +102,16 @@ describe("the dangerfile gitDSL", () => {
     ])
   })
 
+  it("counts the lines of code", async () => {
+    const loc = await gitDSL.linesOfCode()
+    expect(loc).toBe(1151)
+  })
+
+  it("allows the lines of code to be filtered by file path", async () => {
+    const loc = await gitDSL.linesOfCode("lib/**")
+    expect(loc).toBe(720)
+  })
+
   it("show diff chunks for a specific file", async () => {
     const { chunks } = (await gitDSL.structuredDiffForFile("tsconfig.json"))!
 

source/runner/Executor.ts

@@ -60,6 +60,10 @@ 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
 }
 // This is still badly named, maybe it really should just be runner?
 
@@ -252,19 +256,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"
@@ -303,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}`)
       }
     }

source/runner/_tests/_executor.test.ts

@@ -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 = {
@@ -151,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 () => {
@@ -170,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())
@@ -185,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)
@@ -197,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()
   })
 
@@ -299,6 +343,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())
@@ -332,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()
 
@@ -343,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()
 
@@ -359,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()
 
@@ -378,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()
 
@@ -397,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()