danger local --base inconsistency

[adam-moss]

Doing some more testing with --base, looks like it is being ignored on the call tolocalGetDiff triggered in the subprocess:

DEBUG="*" ./node_modules/.bin/danger local --base develop
 danger:process_runner Starting sub-process run with  +0ms
 danger:localGetDiff git diff develop...HEAD +0ms
 danger:localGetDiff git log develop...HEAD --pretty=format:'{ "sha": "%H", "parents": "%p", "author": {"name": "%an", "email": "%ae", "date": "%ai" }, "committer": {"name": "%cn", "email": "%ce", "date": "%ci" }, "message": "+0ms"},'
 danger:process_runner Preparing to run: /Users/adam.moss/.nvm/versions/node/v9.4.0/bin/node,/Users/adam.moss/projects/project_name/node_modules/danger/distribution/commands/danger-runner.js,--base,develop +29ms
 danger:runDangerSubprocess Running subprocess: node - /Users/adam.moss/projects/project_name/node_modules/danger/distribution/commands/danger-runner.js,--base,develop +0ms
2018-01-24T14:00:53.419Z danger:runner Started Danger runner with

2018-01-24T14:00:53.426Z danger:runner Got STDIN for Danger Run

2018-01-24T14:00:53.441Z danger:localGetDiff git diff master...HEAD

2018-01-24T14:00:53.455Z danger:localGetDiff git log master...HEAD --pretty=format:'{ "sha": "%H", "parents": "%p", "author": {"name": "%an", "email": "%ae", "date": "%ai" }, "committer": {"name": "%cn", "email": "%ce", "date": "%ci" }, "message": "%s"},'

2018-01-24T14:00:53.469Z danger:runner Evaluating dangerfile.js

2018-01-24T14:00:53.470Z danger:inline_runner Started parsing Dangerfile:  dangerfile.js

2018-01-24T14:00:53.470Z danger:inline_runner Finished running dangerfile:  dangerfile.js

2018-01-24T14:00:53.471Z danger:inline_runner Scheduler waiting on: 0 tasks

2018-01-24T14:00:53.471Z danger:inline_runner Finished scheduled tasks

2018-01-24T14:00:54.424Z danger:runner Process has finished with 0 undefined, sending the results back to the host process

 danger:runDangerSubprocess Got JSON results from STDOUT +1s
 danger:runDangerSubprocess child process exited with code 0 +5ms
 danger:executor Got Results back, current settings { stdoutOnly: true,
 verbose: undefined,
 jsonOnly: false,
 dangerID: 'default' } +0ms
 danger:executor Writing to STDOUT: { fails: [],
 warnings: [],
 messages: [],
 markdowns: [],
 scheduled: [] } +2ms
Danger: Passed review, received no feedback.

[peterjgrainger]

Issue is here where base is hardcoded to master

danger-js/source/runner/jsonToDSL.ts

Line 18 in f7a5287

const localPlatform = new LocalGit({ base: "master" })

Will try and find where to change

[peterjgrainger]

Not 100% sure but I don't think the danger-runner.ts file should be ran if doing a local run

Here is where the file is imported

danger-js/source/commands/utils/dangerRunToRunnerCLI.ts

Line 16 in f7a5287

newJSFile = newJSFile.replace("-" + name, "-runner")

[orta]

Yep - it will run, that's the issue - the base needs to go from the local process to the separate runner process somehow

[orta]

I'd been wondering about expanding the JSON DSL sent to the sub-process to include individual command settings (it may already be doing this TBH) looks like this is probably the case

[orta]

I'm mid some pretty deep work on Peril so don't want to use my next spare time on this until the peril work is ready - so if someone wants to take a look at that PR that's fine with me

[peterjgrainger]

Yeah we can look at it. Might need a bit of validation before hand to make sure I'm doing the right thing

…

On 24 Jan 2018 3:10 pm, "Orta" ***@***.***> wrote: I'm mid some pretty deep work on Peril <danger/peril#189> so don't want to use my next spare time on this until the peril work is ready - so if someone wants to take a look at that PR that's fine with me — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABRUq2sYOmSh3ox8i5zC8sdNJQqZ7eCkks5tN0fJgaJpZM4RrU0S> .

[orta]

You want to put the fill the cliArgs in here, in the local somehow

danger-js/source/dsl/DangerDSL.ts

Lines 17 to 46 in f0a7cf9

	export interface DangerDSLJSONType {
	/** The data only version of Git DSL */
	git: GitJSONDSL
	/** The data only version of GitHub DSL */
	github: GitHubDSL
	/**
	* Used in the Danger JSON DSL to pass metadata between
	* processes. It will be undefined when used inside the Danger DSL
	*/
	settings: {
	/**
	* Saves each client re-implmenting logic to grab these vars
	* for their API clients
	*/
	github: {
	/** API token for the GitHub client to use */
	accessToken: string
	/** Optional URL for enterprise GitHub */
	baseURL: string | undefined
	/** Optional headers to add to a request */
	additionalHeaders: any
	}
	/**
	* This is still a bit of a WIP, but this should
	* pass args/opts from the original CLI call through
	* to the process.
	*/
	cliArgs: any
	}
	}

Then use that in the line you mentioned here:

danger-js/source/runner/jsonToDSL.ts

Line 18 in f7a5287

const localPlatform = new LocalGit({ base: "master" })

[peterjgrainger]

Ok. I think I get it

…

On 24 Jan 2018 3:15 pm, "Orta" ***@***.***> wrote: You want to put the fill the cliArgs in here, in the local somehow https://github.com/danger/danger-js/blob/f0a7cf9c452cc52755b4a0e04c7b8a 89a2539027/source/dsl/DangerDSL.ts#L17-L46 Then use that in the line you mentioned here: https://github.com/danger/ danger-js/blob/f7a528779623f8e2be74cc4f100c744f0ef2696f/source/runner/ jsonToDSL.ts#L18 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABRUq-6kmlV4C6crxdn0KuDeQjz3h9DNks5tN0kogaJpZM4RrU0S> .

[peterjgrainger]

I'm trying to add tests for the danger-runner file and saw your note at the top to run this command instead, as pretty fairly, it's hard to unit test a CLI call.

// yarn build; cat source/_tests/fixtures/danger-js-pr-395.json | env DANGER_FAKE_CI="YEP" DANGER_TEST_REPO='danger/danger-js' DANGER_TEST_PR='395' node distribution/commands/danger-runner.js --text-only
I'm having some issues running that command, I get 401 bad credentials response. I'm guessing because I don't have access to the danger repo.

Was just wondering if you mind me refactoring the non cli parts out into a separate file so I can test it a bit easier?

[orta]

Sure, - you probably don't have a DANGER_GITHUB_API_TOKEN set up, so your requests are going through un-auth'd

[peterjgrainger]

ah, that's probably it. I've made some unit tests and split it up a little bit.

I'm attaching the CLI args at the point the JSON is parsed.

not sure if you use it but the jest extension for vscode is really good :) Good choice on using jest, I really like it :)

[orta]

Thanks, I made that extension ;)