fix: use fallbacks to get PR id for CodeBuild

CHANGELOG.md

@@ -15,7 +15,7 @@
 
 <!-- Your comment below this -->
 
-- Bitbucket Cloud: Fix bug when Danger updating inline comment with summary comment. - [@hellocore]
+- Fall back to alternative methods for establishing the PR number from CodeBuild - [@alexandermendes]
 
 <!-- Your comment above this -->
 
@@ -1845,3 +1845,4 @@ Not usable for others, only stubs of classes etc. - [@orta]
 [@soyn]: https://github.com/Soyn
 [@tim3trick]: https://github.com/tim3trick
 [@doniyor2109]: https://github.com/doniyor2109
+[@alexandermendes]: https://github.com/alexandermendes

source/ci_source/providers/CodeBuild.ts

@@ -1,5 +1,5 @@
 import { Env, CISource } from "../ci_source"
-import { ensureEnvKeysExist } from "../ci_source_helpers"
+import { ensureEnvKeysExist, getPullRequestIDForBranch } from "../ci_source_helpers"
 
 /**
  * CI Setup
@@ -10,10 +10,24 @@ import { ensureEnvKeysExist } from "../ci_source_helpers"
  *
  * Add your `DANGER_GITHUB_API_TOKEN` to your project. Edit -> Environment -> Additional configuration -> Create a parameter
  *
+ * Note that currently, there seems to be no totally reliable way to get the branch
+ * name from CodeBuild. Sometimes `CODEBUILD_SOURCE_VERSION` contains the
+ * PR number in the format pr/123, but not always. Other times it may contain
+ * a commit hash. `CODEBUILD_WEBHOOK_TRIGGER` will contain the pr number on the
+ * same format, but only for the first event, for subsequent events it should
+ * contain the branch number in the format branch/my-branch. So here we attempt
+ * to determine the PR number from one of the environment variables and if
+ * unsuccessful fall back to calling the API to find the PR for the branch.
  */
 export class CodeBuild implements CISource {
+  private default = { prID: "0" }
   constructor(private readonly env: Env) {}
 
+  async setup(): Promise<any> {
+    const prID = await this._getPrId()
+    this.default.prID = prID.toString()
+  }
+
   get name(): string {
     return "CodeBuild"
   }
@@ -23,12 +37,12 @@ export class CodeBuild implements CISource {
   }
 
   get isPR(): boolean {
-    const mustHave = ["CODEBUILD_BUILD_ID", "CODEBUILD_SOURCE_VERSION", "CODEBUILD_SOURCE_REPO_URL"]
+    const mustHave = ["CODEBUILD_BUILD_ID", "CODEBUILD_SOURCE_REPO_URL"]
     return ensureEnvKeysExist(this.env, mustHave) && this._isPRRequest()
   }
 
   get pullRequestID(): string {
-    return this.env.CODEBUILD_SOURCE_VERSION.split("/")[1]
+    return this.default.prID
   }
 
   get repoSlug(): string {
@@ -40,9 +54,7 @@ export class CodeBuild implements CISource {
   }
 
   private _isPRRequest(): boolean {
-    const isPRSource = this.env.CODEBUILD_SOURCE_VERSION.split("/")[0] === "pr" ? true : false
-    const isPRIdInt = !isNaN(parseInt(this.env.CODEBUILD_SOURCE_VERSION.split("/")[1]))
-    return isPRSource && isPRIdInt
+    return this.default.prID !== "0"
   }
 
   private _prParseUrl(): string {
@@ -51,4 +63,26 @@ export class CodeBuild implements CISource {
     const matches = prUrl.match(regexp)
     return matches ? matches[2] : ""
   }
+
+  private async _getPrId(): Promise<string> {
+    const sourceParts = (this.env.CODEBUILD_SOURCE_VERSION || "").split("/")
+    const triggerParts = (this.env.CODEBUILD_WEBHOOK_TRIGGER || "").split("/")
+
+    const branchName = triggerParts[0] === "branch" ? triggerParts[1] : null
+    let prId = sourceParts[0] === "pr" ? sourceParts[1] : null
+
+    if (!prId) {
+      prId = triggerParts[0] === "pr" ? triggerParts[1] : null
+    }
+
+    if (!prId && branchName) {
+      prId = await getPullRequestIDForBranch(this, this.env, branchName)
+    }
+
+    if (isNaN(parseInt(prId))) {
+      return "0"
+    }
+
+    return prId
+  }
 }

source/ci_source/providers/_tests/_codebuild.test.ts

@@ -1,5 +1,6 @@
 import { CodeBuild } from "../CodeBuild"
 import { getCISourceForEnv } from "../../get_ci_source"
+import { getPullRequestIDForBranch } from "../../ci_source_helpers"
 
 const correctEnv = {
   CODEBUILD_BUILD_ID: "123",
@@ -8,6 +9,18 @@ const correctEnv = {
   DANGER_GITHUB_API_TOKEN: "xxx",
 }
 
+const setupCodeBuildSource = async (env: Object) => {
+  const source = new CodeBuild(env)
+  await source.setup()
+
+  return source
+}
+
+jest.mock("../../ci_source_helpers", () => ({
+  ...jest.requireActual("../../ci_source_helpers"),
+  getPullRequestIDForBranch: jest.fn(),
+}))
+
 describe("being found when looking for CI", () => {
   it("finds CodeBuild with the right ENV", () => {
     const cb = getCISourceForEnv(correctEnv)
@@ -16,57 +29,82 @@ describe("being found when looking for CI", () => {
 })
 
 describe(".isCI", () => {
-  it("validates when all CodeBuild environment vars are set", () => {
-    const codebuild = new CodeBuild(correctEnv)
+  it("validates when all CodeBuild environment vars are set", async () => {
+    const codebuild = await setupCodeBuildSource(correctEnv)
     expect(codebuild.isCI).toBeTruthy()
   })
 
-  it("does not validate without env", () => {
-    const codebuild = new CodeBuild({})
+  it("does not validate without env", async () => {
+    const codebuild = await setupCodeBuildSource({})
     expect(codebuild.isCI).toBeFalsy()
   })
 })
 
 describe(".isPR", () => {
-  it("validates when all CodeBuild environment vars are set", () => {
-    const codebuild = new CodeBuild(correctEnv)
+  it("validates when all CodeBuild environment vars are set", async () => {
+    const codebuild = await setupCodeBuildSource(correctEnv)
     expect(codebuild.isPR).toBeTruthy()
   })
 
-  it("does not validate outside of CodeBuild", () => {
-    const codebuild = new CodeBuild({})
+  it("does not validate outside of CodeBuild", async () => {
+    const codebuild = await setupCodeBuildSource({})
     expect(codebuild.isPR).toBeFalsy()
   })
 
-  const envs = ["CODEBUILD_BUILD_ID", "CODEBUILD_SOURCE_VERSION", "CODEBUILD_SOURCE_REPO_URL"]
-  envs.forEach((key: string) => {
-    let env = Object.assign({}, correctEnv)
-    env[key] = null
-
-    it(`does not validate when ${key} is missing`, () => {
-      const codebuild = new CodeBuild({})
-      expect(codebuild.isPR).toBeFalsy()
-    })
+  it.each(["CODEBUILD_BUILD_ID", "CODEBUILD_SOURCE_REPO_URL"])(`does not validate when %s is missing`, async key => {
+    const copiedEnv = { ...correctEnv }
+    delete copiedEnv[key]
+    const codebuild = await setupCodeBuildSource(copiedEnv)
+    expect(codebuild.isPR).toBeFalsy()
   })
 
-  it("needs to have a PR number", () => {
+  it("needs to have a PR number", async () => {
     let env = Object.assign({}, correctEnv)
     env.CODEBUILD_SOURCE_VERSION = "pr/abc"
-    const codebuild = new CodeBuild(env)
+    const codebuild = await setupCodeBuildSource(env)
     expect(codebuild.isPR).toBeFalsy()
   })
 })
 
 describe(".pullRequestID", () => {
-  it("splits it from the env", () => {
-    const codebuild = new CodeBuild({ CODEBUILD_SOURCE_VERSION: "pr/2" })
+  beforeEach(() => {
+    jest.resetAllMocks()
+  })
+
+  it.each(["CODEBUILD_SOURCE_VERSION", "CODEBUILD_WEBHOOK_TRIGGER"])("splits it from %s", async key => {
+    const codebuild = await setupCodeBuildSource({ [key]: "pr/2" })
+    await codebuild.setup()
     expect(codebuild.pullRequestID).toEqual("2")
   })
+
+  it("calls the API to get the PR number if not available in the env vars", async () => {
+    ;(getPullRequestIDForBranch as jest.Mock).mockResolvedValue(1)
+    const env = {
+      CODEBUILD_SOURCE_REPO_URL: "https://github.com/sharkysharks/some-repo",
+      CODEBUILD_WEBHOOK_TRIGGER: "branch/my-branch",
+      DANGER_GITHUB_API_TOKEN: "xxx",
+    }
+    const codebuild = await setupCodeBuildSource(env)
+    expect(codebuild.pullRequestID).toBe("1")
+    expect(getPullRequestIDForBranch).toHaveBeenCalledTimes(1)
+    expect(getPullRequestIDForBranch).toHaveBeenCalledWith(codebuild, env, "my-branch")
+  })
+
+  it("does not call the API if no PR number or branch name available in the env vars", async () => {
+    const env = {
+      CODEBUILD_SOURCE_REPO_URL: "https://github.com/sharkysharks/some-repo",
+      CODEBUILD_WEBHOOK_TRIGGER: "tag/my-tag",
+      DANGER_GITHUB_API_TOKEN: "xxx",
+    }
+    const codebuild = await setupCodeBuildSource(env)
+    expect(codebuild.pullRequestID).toBe("0")
+    expect(getPullRequestIDForBranch).not.toHaveBeenCalled()
+  })
 })
 
 describe(".repoSlug", () => {
-  it("parses it from the repo url", () => {
-    const codebuild = new CodeBuild(correctEnv)
+  it("parses it from the repo url", async () => {
+    const codebuild = await setupCodeBuildSource(correctEnv)
     expect(codebuild.repoSlug).toEqual("sharkysharks/some-repo")
   })
 })

source/platforms/bitbucket_server/_tests/_bitbucket_server_git.test.ts

@@ -129,12 +129,12 @@ describe("the dangerfile gitDSL - BitBucket Server", () => {
 
   it("checks promise rejection for line not in the diff for inline comment", async () => {
     const promise = bbs.findTypeOfLine(gitDSL, 2, "banana")
-    await expect(promise).rejects
+    await expect(promise).rejects.toBeUndefined()
   })
 
   it("checks promise rejection for `del` line for inline comment for deleted file", async () => {
     const promise = bbs.findTypeOfLine(gitDSL, 0, "jest.eslint.config.js")
-    await expect(promise).rejects
+    await expect(promise).rejects.toBeUndefined()
   })
 
   it("writes a JSON DSL fixture", async () => {