[BUG] renaming master branch causes danger local to fail even when base branch is provided

[ahobson]

Describe the bug
We renamed our master branch to main and danger local -b main fails

To Reproduce
Steps to reproduce the behavior:

1. git checkout master
2. git branch -m main
3. git checkout -b testbranch
4. yarn run danger local -b main

Expected behavior
yarn run danger local -b main runs danger without error

Screenshots

yarn danger local -b main
yarn run v1.22.4
$ /opt/docker/react-uswds/node_modules/.bin/danger local -b main
Could not get diff from git between master and HEAD

Error: fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

    at Socket.<anonymous> (/opt/docker/react-uswds/node_modules/danger/distribution/platforms/git/localGetDiff.js:20:19)
    at Socket.emit (events.js:310:20)
    at addChunk (_stream_readable.js:286:12)
    at readableAddChunk (_stream_readable.js:268:9)
    at Socket.Readable.push (_stream_readable.js:209:10)
    at Pipe.onStreamRead (internal/stream_base_commons.js:186:23)

/opt/docker/react-uswds/node_modules/danger/distribution/commands/danger-runner.js:119
    d("Got md " + results.markdowns.length + " w " + results.warnings.length + " f " + results.fails.length + " m " + results.messages.length);
                          ^

TypeError: Cannot read property 'markdowns' of undefined
    at /opt/docker/react-uswds/node_modules/danger/distribution/commands/danger-runner.js:119:27
    at /opt/docker/react-uswds/node_modules/node-cleanup/node-cleanup.js:80:9
    at Array.forEach (<anonymous>)
    at process.exitHandler (/opt/docker/react-uswds/node_modules/node-cleanup/node-cleanup.js:79:21)
    at process.emit (events.js:310:20)
    at process.exit (internal/process/per_thread.js:166:15)
    at process.exceptionHandler (/opt/docker/react-uswds/node_modules/node-cleanup/node-cleanup.js:74:13)
    at process.emit (events.js:310:20)
    at process._fatalException (internal/process/execution.js:164:25)

Danger: ⅹ Failing the build, there is 1 fail.
## Failures
`node` failed.
## Markdowns
### Log


```sh

```


error Command failed with exit code 7.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Your Environment

software	version
danger.js	10.2.1
node	v12.16.3
npm	6.14.4
Operating System	Linux

Additional context
Add any other context about the problem here.

[fbartho]

@ahobson -- We don't run danger local but it appears that it doesn't break danger pr right? -- just the danger local command?

(I moved master -> main on my non-OpenSource project last week and we haven't seen this kind of crash)

[fbartho]

@steipete -- should we move master -> main on danger-js when we fix this bug?

[orta]

Looks like -b doesn't work or has an implicit dep on master somehow.

Sure, I'm not against moving danger to use main

[ahobson]

From some local debugging, I'm pretty sure the base branch settings is lost in the JSON serialization step when calling getPlatformGitRepresentation

[ahobson]

After some more spelunking, it seems that the danger runner is invoked with the right options:

  danger:process_runner Preparing to run: /usr/local/bin/node,/opt/docker/danger-js/distribution/commands/danger-runner.js,-b,main,-v +1ms

But sharedDangerfileArgs.ts does not support the --base argument. That means it is not parsed and so the option is not set.

The setting is in the JSON representation, but dangerDSLJSON.ts overrides the settings with the ones from the command line.

One way to fix it would be to merge the settings from the JSON with the command line

diff --git a/source/runner/dangerDSLJSON.ts b/source/runner/dangerDSLJSON.ts
index f607edb..d6a6a4a 100644
--- a/source/runner/dangerDSLJSON.ts
+++ b/source/runner/dangerDSLJSON.ts
@@ -38,6 +38,6 @@ export class DangerDSLJSON implements DangerDSLJSONType {
     Object.assign(this, parsedString.danger)
 
     // @ts-ignore
-    this.settings.cliArgs = cliArgs
+    Object.assign(this.settings.cliArgs, cliArgs)
   }
 }

Another way would be to add base and staging to the shared args.

diff --git a/source/commands/danger-local.ts b/source/commands/danger-local.ts
index 03f65d2..42abbec 100644
--- a/source/commands/danger-local.ts
+++ b/source/commands/danger-local.ts
@@ -7,22 +7,13 @@ import { runRunner } from "./ci/runner"
 import { LocalGit } from "../platforms/LocalGit"
 import { FakeCI } from "../ci_source/providers/Fake"
 
-interface App extends SharedCLI {
-  /** What should we compare against? */
-  base?: string
-  /** Should we run against current staged changes? */
-  staging?: boolean
-}
-
 program
   .usage("[options]")
   .description("Runs danger without PR metadata, useful for git hooks.")
-  .option("-s, --staging", "Just use staged changes.")
-  .option("-b, --base [branch_name]", "Use a different base branch")
   .allowUnknownOption(true)
 setSharedArgs(program).parse(process.argv)
 
-const app = (program as any) as App
+const app = (program as any) as SharedCLI
 const base = app.base || "master"
 
 const localPlatform = new LocalGit({ base, staging: app.staging })
diff --git a/source/commands/utils/sharedDangerfileArgs.ts b/source/commands/utils/sharedDangerfileArgs.ts
index 65f3fe6..d683a56 100644
--- a/source/commands/utils/sharedDangerfileArgs.ts
+++ b/source/commands/utils/sharedDangerfileArgs.ts
@@ -7,6 +7,10 @@ process.on("unhandledRejection", function(reason: string, _p: any) {
 })
 
 export interface SharedCLI extends program.CommanderStatic {
+  /** What should we compare against? */
+  base?: string
+  /** Should we run against current staged changes? */
+  staging?: boolean
   /** Should we be posting as much info as possible? */
   verbose: boolean
   /** Output to STDOUT instead of leaving a comment */
@@ -43,3 +47,5 @@ export default (command: any) =>
     .option("-u, --passURLForDSL", "[dev] Use a custom URL to send the Danger DSL into the sub-process")
     .option("-f, --failOnErrors", "Fail if there are fails in the danger report", false)
     .option("--use-github-checks", "Use GitHub Checks", false)
+    .option("-s, --staging", "Just use staged changes.")
+    .option("-b, --base [branch_name]", "Use a different base branch")

Any preference?

[fbartho]

@orta Probably has an opinion on which should win (json or Args)

But generally, I bet that the args should work as they are being passed!