[Inline mode] Fix a bug with multiple updates for the inline comment.

CHANGELOG.md

@@ -13,7 +13,7 @@
 
 ## Master
 
-* Item. [@name][]
+* Fixed a bug where updating multiple inline comments caused a Javascript error [@sunshinejr][]
 
 ## 3.4.2
 

source/runner/Executor.ts

@@ -268,7 +268,7 @@ export class Executor {
       let promise: Promise<any>
       if (index != -1) {
         let previousComment = deleteComments[index]
-        delete deleteComments[index]
+        deleteComments.splice(index, 1)
         promise = this.updateInlineComment(inlineResult, previousComment)
       } else {
         promise = this.sendInlineComment(git, inlineResult)

source/runner/_tests/_executor.test.ts

@@ -9,6 +9,8 @@ import {
   inlineRegularResults,
   inlineFailResults,
   inlineMessageResults,
+  inlineMultipleWarnResults,
+  inlineMultipleWarnResults2,
 } from "./fixtures/ExampleDangerResults"
 import inlineRunner from "../runners/inline"
 import { jsonDSLGenerator } from "../dslGenerator"
@@ -162,6 +164,24 @@ describe("setup", () => {
     expect(platform.createInlineComment).not.toBeCalled()
   })
 
+  it("Updates multiple inline comments", async () => {
+    const platform = new FakePlatform()
+    const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig)
+    const dsl = await defaultDsl(platform)
+    const previousResults = inlineMultipleWarnResults
+    const newResults = inlineMultipleWarnResults2
+    const previousComments = mockPayloadForResults(previousResults)
+    platform.getInlineComments = jest.fn().mockReturnValue(new Promise(r => r(previousComments)))
+    platform.updateInlineComment = jest.fn()
+    platform.createInlineComment = jest.fn()
+    platform.deleteInlineComment = jest.fn()
+
+    await exec.handleResults(newResults, dsl.git)
+    expect(platform.updateInlineComment).toHaveBeenCalledTimes(3)
+    expect(platform.createInlineComment).not.toBeCalled()
+    expect(platform.deleteInlineComment).not.toBeCalled()
+  })
+
   it("Doesn't update/create an inline comment as the old was the same as the new", async () => {
     const platform = new FakePlatform()
     const exec = new Executor(new FakeCI({}), platform, inlineRunner, defaultConfig)

source/runner/_tests/fixtures/ExampleDangerResults.ts

@@ -28,6 +28,28 @@ export const inlineWarnResults: DangerResults = {
   markdowns: [],
 }
 
+export const inlineMultipleWarnResults: DangerResults = {
+  messages: [],
+  warnings: [
+    { message: "Test message", file: "File.swift", line: 10 },
+    { message: "Test message", file: "File.swift", line: 11 },
+    { message: "Test message", file: "File2.swift", line: 10 },
+  ],
+  fails: [],
+  markdowns: [],
+}
+
+export const inlineMultipleWarnResults2: DangerResults = {
+  messages: [],
+  warnings: [
+    { message: "Test message2", file: "File.swift", line: 10 },
+    { message: "Test message2", file: "File.swift", line: 11 },
+    { message: "Test message2", file: "File2.swift", line: 10 },
+  ],
+  fails: [],
+  markdowns: [],
+}
+
 export const inlineFailResults: DangerResults = {
   messages: [],
   warnings: [],