fix: improve danger-pr

When danger-pr is called with a GitLab URL with no DANGER_GITLAB_API_TOKEN use the GitLab provider instead of defaulting to GitHub
danger · Jun 4, 2019 · 67662eb · 67662eb
1 parent 0952bac
commit 67662eb
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 22 deletions.
diff --git a/source/commands/danger-pr.ts b/source/commands/danger-pr.ts
@@ -14,6 +14,7 @@ import { prepareDangerDSL } from "./utils/runDangerSubprocess"
 import { runRunner } from "./ci/runner"
 import { Platform, getPlatformForEnv } from "../platforms/platform"
 import { CISource } from "../ci_source/ci_source"
+import { getGitLabAPICredentialsFromEnv } from "../platforms/gitlab/GitLabAPI"
 
 const d = debug("pr")
 const log = console.log
@@ -25,6 +26,8 @@ interface App extends SharedCLI {
   js?: boolean
 }
 
+const gitLabApiCredentials = getGitLabAPICredentialsFromEnv(process.env)
+
 program
   .usage("[options] <pr_url>")
   .description("Emulate running Danger against an existing GitHub Pull Request.")
@@ -37,10 +40,12 @@ program
     if (
       !process.env["DANGER_GITHUB_API_TOKEN"] &&
       !process.env["DANGER_BITBUCKETSERVER_HOST"] &&
-      !process.env["DANGER_GITLAB_HOST"]
+      !gitLabApiCredentials.token
     ) {
       log("")
-      log("     You don't have a DANGER_GITHUB_API_TOKEN set up, this is optional, but TBH, you want to do this.")
+      log(
+        "     You don't have a DANGER_GITHUB_API_TOKEN/DANGER_GITLAB_API_TOKEN set up, this is optional, but TBH, you want to do this."
+      )
       log("     Check out: http://danger.systems/js/guides/the_dangerfile.html#working-on-your-dangerfile")
       log("")
     }
@@ -65,13 +70,10 @@ if (program.args.length === 0) {
   process.exitCode = 1
 } else {
   const customHost =
-    process.env["DANGER_GITHUB_HOST"] ||
-    process.env["DANGER_BITBUCKETSERVER_HOST"] ||
-    process.env["DANGER_GITLAB_HOST"] ||
-    "github"
+    process.env["DANGER_GITHUB_HOST"] || process.env["DANGER_BITBUCKETSERVER_HOST"] || gitLabApiCredentials.host // this defaults to https://gitlab.com
 
   // Allow an ambiguous amount of args to find the PR reference
-  const findPR = program.args.find(a => a.includes(customHost))
+  const findPR = program.args.find(a => a.includes(customHost) || a.includes("github"))
 
   if (!findPR) {
     console.error(`Could not find an arg which mentioned GitHub, BitBucket Server, or GitLab.`)
@@ -94,7 +96,18 @@ if (program.args.length === 0) {
           d(`executing dangerfile at ${dangerfilePath(program)}`)
         }
         const source = new FakeCI({ DANGER_TEST_REPO: pr.repo, DANGER_TEST_PR: pr.pullRequestNumber })
-        const platform = getPlatformForEnv(process.env, source, /* requireAuth */ false)
+        const platform = getPlatformForEnv(
+          {
+            ...process.env,
+            // Inject a platform hint, its up to getPlatformForEnv to decide if the environment is suitable for the
+            // requested platform. Because we have a URL we can determine with greater accuracy what platform that the
+            // user is attempting to test. This complexity is required because danger-pr defaults to using
+            // un-authenticated GitHub where typically when using FakeCI we want to use Fake(Platform) e.g. when running
+            // danger-local
+            DANGER_PR_PLATFORM: pr.platform,
+          },
+          source
+        )
 
         if (isJSON) {
           d("getting just the JSON/JS DSL")

diff --git a/source/platforms/_tests/_pull_request_parser.test.ts b/source/platforms/_tests/_pull_request_parser.test.ts
@@ -9,6 +9,7 @@ describe("parsing urls", () => {
     expect(pullRequestParser("https://github.com/facebook/jest/pull/2555")).toEqual({
       pullRequestNumber: "2555",
       repo: "facebook/jest",
+      platform: "GitHub",
     })
   })
 
@@ -17,27 +18,31 @@ describe("parsing urls", () => {
     expect(pullRequestParser(longPR)).toEqual({
       pullRequestNumber: "406",
       repo: "artsy/emission",
+      platform: "GitHub",
     })
   })
 
   it("handles bitbucket server PRs", () => {
     expect(pullRequestParser("http://localhost:7990/projects/PROJ/repos/repo/pull-requests/1")).toEqual({
       pullRequestNumber: "1",
       repo: "projects/PROJ/repos/repo",
+      platform: "BitBucketServer",
     })
   })
 
   it("handles bitbucket server PRs (overview)", () => {
     expect(pullRequestParser("http://localhost:7990/projects/PROJ/repos/repo/pull-requests/1/overview")).toEqual({
       pullRequestNumber: "1",
       repo: "projects/PROJ/repos/repo",
+      platform: "BitBucketServer",
     })
   })
 
   it("handles bitbucket server PRs (overview) with dashes in name", () => {
     expect(pullRequestParser("http://localhost:7990/projects/PROJ/repos/super-repo/pull-requests/1/overview")).toEqual({
       pullRequestNumber: "1",
       repo: "projects/PROJ/repos/super-repo",
+      platform: "BitBucketServer",
     })
   })
 
@@ -47,6 +52,7 @@ describe("parsing urls", () => {
     ).toEqual({
       pullRequestNumber: "1",
       repo: "projects/PROJ/repos/super-strong.repo_name",
+      platform: "BitBucketServer",
     })
   })
 
@@ -56,43 +62,49 @@ describe("parsing urls", () => {
         expect(pullRequestParser("https://gitlab.com/GROUP/PROJ/merge_requests/123")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/PROJ",
+          platform: "GitLab",
         })
       })
 
       it("handles PRs sub-pages", () => {
         expect(pullRequestParser("https://gitlab.com/GROUP/PROJ/merge_requests/123/commits")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/PROJ",
+          platform: "GitLab",
         })
       })
 
       it("handles sub-groups", () => {
         expect(pullRequestParser("https://gitlab.com/GROUP/SUBGROUP/PROJ/merge_requests/123")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/SUBGROUP/PROJ",
+          platform: "GitLab",
         })
       })
     })
 
-    describe("hosted", () => {
+    describe("CE/EE", () => {
       it("handles PRs", () => {
         expect(pullRequestParser("https://localdomain/GROUP/PROJ/merge_requests/123")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/PROJ",
+          platform: "GitLab",
         })
       })
 
       it("handles PRs sub-pages", () => {
         expect(pullRequestParser("https://localdomain/GROUP/PROJ/merge_requests/123/commits")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/PROJ",
+          platform: "GitLab",
         })
       })
 
       it("handles sub-groups", () => {
         expect(pullRequestParser("https://localdomain/GROUP/SUBGROUP/PROJ/merge_requests/123")).toEqual({
           pullRequestNumber: "123",
           repo: "GROUP/SUBGROUP/PROJ",
+          platform: "GitLab",
         })
       })
     })

diff --git a/source/platforms/platform.ts b/source/platforms/platform.ts
@@ -1,5 +1,5 @@
-import { Env, CISource } from "../ci_source/ci_source"
-import { GitJSONDSL, GitDSL } from "../dsl/GitDSL"
+import { CISource, Env } from "../ci_source/ci_source"
+import { GitDSL, GitJSONDSL } from "../dsl/GitDSL"
 import { GitHub } from "./GitHub"
 import { GitHubAPI } from "./github/GitHubAPI"
 import { BitBucketServer } from "./BitBucketServer"
@@ -89,32 +89,29 @@ export interface PlatformCommunicator {
  * @param {CISource} source The existing source, to ensure they can run against each other
  * @returns {Platform} returns a platform if it can be supported
  */
-export function getPlatformForEnv(env: Env, source: CISource, requireAuth = true): Platform {
+export function getPlatformForEnv(env: Env, source: CISource): Platform {
   // BitBucket Server
-  const bbsHost = env["DANGER_BITBUCKETSERVER_HOST"]
-  if (bbsHost) {
+  if (env["DANGER_BITBUCKETSERVER_HOST"] || env["DANGER_PR_PLATFORM"] === BitBucketServer.name) {
     const api = new BitBucketServerAPI(
       {
         pullRequestID: source.pullRequestID,
         repoSlug: source.repoSlug,
       },
       bitbucketServerRepoCredentialsFromEnv(env)
     )
-    const bbs = new BitBucketServer(api)
-    return bbs
+    return new BitBucketServer(api)
   }
 
   // GitLab
-  if (env["DANGER_GITLAB_API_TOKEN"]) {
+  if (env["DANGER_GITLAB_API_TOKEN"] || env["DANGER_PR_PLATFORM"] === GitLab.name) {
     const api = new GitLabAPI(
       {
         pullRequestID: source.pullRequestID,
         repoSlug: source.repoSlug,
       },
       getGitLabAPICredentialsFromEnv(env)
     )
-    const gitlab = new GitLab(api)
-    return gitlab
+    return new GitLab(api)
   }
 
   // They need to set the token up for GitHub actions to work
@@ -128,15 +125,14 @@ export function getPlatformForEnv(env: Env, source: CISource, requireAuth = true
 
   // GitHub Platform
   const ghToken = env["DANGER_GITHUB_API_TOKEN"] || env["GITHUB_TOKEN"]
-  if (ghToken || !requireAuth) {
+  if (ghToken || env["DANGER_PR_PLATFORM"] === GitHub.name) {
     if (!ghToken) {
       console.log("You don't have a DANGER_GITHUB_API_TOKEN set up, this is optional, but TBH, you want to do this")
       console.log("Check out: http://danger.systems/js/guides/the_dangerfile.html#working-on-your-dangerfile")
     }
 
     const api = new GitHubAPI(source, ghToken)
-    const github = GitHub(api)
-    return github
+    return GitHub(api)
   }
 
   // Support automatically returning a fake platform if you pass a Fake CI

diff --git a/source/platforms/pullRequestParser.ts b/source/platforms/pullRequestParser.ts
@@ -1,9 +1,13 @@
 import * as url from "url"
 import includes from "lodash.includes"
+import { BitBucketServer } from "./BitBucketServer"
+import { GitHub } from "./GitHub"
+import GitLab from "./GitLab"
 
 export interface PullRequestParts {
   pullRequestNumber: string
   repo: string
+  platform: string
 }
 
 export function pullRequestParser(address: string): PullRequestParts | null {
@@ -14,6 +18,7 @@ export function pullRequestParser(address: string): PullRequestParts | null {
     const parts = components.path.match(/(projects\/\w+\/repos\/[\w-_.]+)\/pull-requests\/(\d+)/)
     if (parts) {
       return {
+        platform: BitBucketServer.name,
         repo: parts[1],
         pullRequestNumber: parts[2],
       }
@@ -22,6 +27,7 @@ export function pullRequestParser(address: string): PullRequestParts | null {
     // shape: http://github.com/proj/repo/pull/1
     if (includes(components.path, "pull")) {
       return {
+        platform: GitHub.name,
         repo: components.path.split("/pull")[0].slice(1),
         pullRequestNumber: components.path.split("/pull/")[1],
       }
@@ -33,6 +39,7 @@ export function pullRequestParser(address: string): PullRequestParts | null {
       const parts = components.path.match(regex)
       if (parts) {
         return {
+          platform: GitLab.name,
           repo: parts[1],
           pullRequestNumber: parts[2],
         }