diff --git a/.travis.yml b/.travis.yml index 98583ddd1..795314334 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,6 +29,14 @@ matrix: - yarn build - node scripts/run-fixtures.js + # Runs both the CI and PR with a custom process to ensure apps like swift/rust work + - node_js: "10.13" + script: + - yarn build + - node distribution/commands/danger-ci.js --process "ruby scripts/danger_runner.rb" + - node distribution/commands/danger-pr.js --process "ruby scripts/danger_runner.rb" + https://github.com/danger/danger-js/pull/760 + # Does the real danger run - node_js: "9" script: diff --git a/.vscode/settings.json b/.vscode/settings.json index cb5ef8426..ec4140aac 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -12,5 +12,6 @@ "distribution/": true }, "editor.formatOnSave": true, + "debug.node.autoAttach": "on", "cSpell.words": ["APIPR", "BITBUCKETSERVER", "Commenter", "JSONDSL", "PRDSL", "bitbucket", "jsome", "repo's"] } diff --git a/CHANGELOG.md b/CHANGELOG.md index a004c7f2d..d0442f87d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ ## Master +- Improves debug logging, messaging on CI about status updates - [@orta][] +- Better detection of json results: {} from a sub-process - [@orta][] +- CLI Args are actually passed to subprocess instead of an empty object - [@orta][] - Fix Netlify integration when repo url includes an access token - [@imorente][] # 6.1.0 diff --git a/scripts/danger_runner.rb b/scripts/danger_runner.rb index 36727bd3f..baa8ac3f1 100755 --- a/scripts/danger_runner.rb +++ b/scripts/danger_runner.rb @@ -5,6 +5,7 @@ # Have a dumb fake response require "json" +puts "Hello from ruby!" results = { fails: [], warnings: [], messages: [], markdowns: [] }.to_json STDOUT.write(results) diff --git a/source/commands/ci/runner.ts b/source/commands/ci/runner.ts index 10fd377ad..e19729930 100644 --- a/source/commands/ci/runner.ts +++ b/source/commands/ci/runner.ts @@ -57,7 +57,7 @@ export const runRunner = async (app: SharedCLI, config?: Partial) } if (platform) { - const dangerJSONDSL = await jsonDSLGenerator(platform, source) + const dangerJSONDSL = await jsonDSLGenerator(platform, source, app) const execConfig: ExecutorOptions = { stdoutOnly: !platform.supportsCommenting() || app.textOnly, verbose: app.verbose, diff --git a/source/commands/danger-pr.ts b/source/commands/danger-pr.ts index ef4956e68..3057b900c 100644 --- a/source/commands/danger-pr.ts +++ b/source/commands/danger-pr.ts @@ -104,7 +104,7 @@ if (program.args.length === 0) { // Run the first part of a Danger Process and output the JSON to CLI async function runHalfProcessJSON(platform: Platform, source: CISource) { - const dangerDSL = await jsonDSLGenerator(platform, source) + const dangerDSL = await jsonDSLGenerator(platform, source, program) const processInput = prepareDangerDSL(dangerDSL) const output = JSON.parse(processInput) const dsl = { danger: output } diff --git a/source/commands/danger-process.ts b/source/commands/danger-process.ts index 10bf7aa6b..13edca1c0 100644 --- a/source/commands/danger-process.ts +++ b/source/commands/danger-process.ts @@ -25,11 +25,13 @@ const d = debug("process") declare const global: any let subprocessName: string | undefined +let deprecationWarning = chalk.bold("Semi-deprecated, as ci/pr/local all support --process") program .usage("[options] ") .description( - "Does a Danger run, but instead of handling the execution of a Dangerfile it will pass the DSL " + + deprecationWarning + + "\n\nDoes a Danger run, but instead of handling the execution of a Dangerfile it will pass the DSL " + "into another process expecting the process to eventually return results back as JSON. If you don't " + "provide another process, then it will output to STDOUT." ) @@ -70,7 +72,7 @@ getRuntimeCISource(app).then(source => { } if (platform) { - jsonDSLGenerator(platform, source).then(dangerJSONDSL => { + jsonDSLGenerator(platform, source, app).then(dangerJSONDSL => { if (!subprocessName) { // Just pipe it out to the CLI const processInput = prepareDangerDSL(dangerJSONDSL) diff --git a/source/commands/utils/runDangerSubprocess.ts b/source/commands/utils/runDangerSubprocess.ts index b5638c749..473dedf46 100644 --- a/source/commands/utils/runDangerSubprocess.ts +++ b/source/commands/utils/runDangerSubprocess.ts @@ -37,11 +37,13 @@ export const runDangerSubprocess = ( const dslJSONString = prepareDangerDSL(dslJSON) d(`Running subprocess: ${processDisplayName} - ${args}`) const child = spawn(processName, args, { env: { ...process.env, ...config.additionalEnvVars } }) - let allLogs = "" + d(`Started passing in STDIN`) child.stdin.write(dslJSONString) child.stdin.end() + d(`Passed in STDIN`) + let allLogs = "" child.stdout.on("data", async data => { const stdout = data.toString() allLogs += stdout @@ -50,15 +52,21 @@ export const runDangerSubprocess = ( const maybeJSON = getJSONFromSTDOUT(stdout) const maybeJSONURL = getJSONURLFromSTDOUT(stdout) + // Remove message sent back to danger-js + const withoutURLs: string = data + .toString() + .replace(maybeJSON, "") + .replace(maybeJSONURL, "") + + console.log(withoutURLs) + + // Pass it back to the user if (!results && maybeJSONURL) { d("Got JSON URL from STDOUT, results are at: \n" + maybeJSONURL) results = JSON.parse(readFileSync(maybeJSONURL.replace("danger-results:/", ""), "utf8")) } else if (!results && maybeJSON) { d("Got JSON results from STDOUT, results: \n" + maybeJSON) results = JSON.parse(maybeJSON) - } else { - // Pass it back to the user - console.log(data.toString()) } }) @@ -98,14 +106,17 @@ const getJSONURLFromSTDOUT = (stdout: string): string | undefined => { /** Pulls the JSON directly out, this has proven to be less reliable */ const getJSONFromSTDOUT = (stdout: string): string | undefined => { - const trimmed = stdout.trim() - return trimmed.startsWith("{") && - trimmed.endsWith("}") && - trimmed.includes("markdowns") && - trimmed.includes("fails") && - trimmed.includes("warnings") - ? trimmed - : undefined + const lines = stdout.split("\n") + return lines.find(line => { + const trimmed = line.trim() + return ( + trimmed.startsWith("{") && + trimmed.endsWith("}") && + trimmed.includes("markdowns") && + trimmed.includes("fails") && + trimmed.includes("warnings") + ) + }) } export default runDangerSubprocess diff --git a/source/danger-incoming-process-schema.json b/source/danger-incoming-process-schema.json index d08fe946e..149074105 100644 --- a/source/danger-incoming-process-schema.json +++ b/source/danger-incoming-process-schema.json @@ -486,21 +486,27 @@ "description": "Describes the possible arguments that\ncould be used when calling the CLI", "properties": { "base": { + "description": "The base reference used by danger PR (e.g. not master)", "type": "string" }, "dangerfile": { + "description": "A custom path for the dangerfile (can also be a remote reference)", "type": "string" }, "externalCiProvider": { + "description": "Used by danger-js o allow having a custom CI", "type": "string" }, "id": { + "description": "So you can have many danger runs in one code review", "type": "string" }, "textOnly": { + "description": "Use SDTOUT instead of posting to the code review side", "type": "string" }, "verbose": { + "description": "For debugging", "type": "string" } }, diff --git a/source/danger.d.ts b/source/danger.d.ts index 94c3ea28e..a4b6f3170 100644 --- a/source/danger.d.ts +++ b/source/danger.d.ts @@ -1108,13 +1108,21 @@ interface Violation { * could be used when calling the CLI */ interface CliArgs { + /** The base reference used by danger PR (e.g. not master) */ base: string + /** For debugging */ verbose: string + /** Used by danger-js o allow having a custom CI */ externalCiProvider: string + /** Use SDTOUT instead of posting to the code review side */ textOnly: string + /** A custom path for the dangerfile (can also be a remote reference) */ dangerfile: string + /** So you can have many danger runs in one code review */ id: string } + +// NOTE: if add something new here, you need to change dslGenerator.ts /** A function with a callback function, which Danger wraps in a Promise */ type CallbackableFn = (callback: (done: any) => void) => void diff --git a/source/dsl/cli-args.ts b/source/dsl/cli-args.ts index c3889fa44..ff471104f 100644 --- a/source/dsl/cli-args.ts +++ b/source/dsl/cli-args.ts @@ -3,10 +3,18 @@ * could be used when calling the CLI */ export interface CliArgs { + /** The base reference used by danger PR (e.g. not master) */ base: string + /** For debugging */ verbose: string + /** Used by danger-js o allow having a custom CI */ externalCiProvider: string + /** Use SDTOUT instead of posting to the code review side */ textOnly: string + /** A custom path for the dangerfile (can also be a remote reference) */ dangerfile: string + /** So you can have many danger runs in one code review */ id: string } + +// NOTE: if add something new here, you need to change dslGenerator.ts diff --git a/source/platforms/github/GitHubAPI.ts b/source/platforms/github/GitHubAPI.ts index 93ee20de6..155e0bf54 100644 --- a/source/platforms/github/GitHubAPI.ts +++ b/source/platforms/github/GitHubAPI.ts @@ -353,18 +353,28 @@ export class GitHubAPI { if (passed === "pending") { state = "pending" } - const res = await this.post( - `repos/${repo}/statuses/${ref}`, - {}, - { - state: state, - context: process.env["PERIL_BOT_USER_ID"] ? "Peril" : "Danger", - target_url: url || "http://danger.systems/js", - description: message, + // Suppress errors, because in OSS + // this failure could be due to access rights. + // + // So only error when it's a real message. + try { + const res = await this.post( + `repos/${repo}/statuses/${ref}`, + {}, + { + state: state, + context: process.env["PERIL_BOT_USER_ID"] ? "Peril" : "Danger", + target_url: url || "http://danger.systems/js", + description: message, + }, + true + ) + return res.ok + } catch (error) { + if (prJSON.base.repo.private) { + console.log("Could not post a commit status.") } - ) - - return res.ok + } } postCheckRun = async (check: CheckOptions, token: string) => { diff --git a/source/platforms/github/_tests/_github_api.test.ts b/source/platforms/github/_tests/_github_api.test.ts index 79908f087..cd3901cc1 100644 --- a/source/platforms/github/_tests/_github_api.test.ts +++ b/source/platforms/github/_tests/_github_api.test.ts @@ -162,7 +162,7 @@ describe("API testing", () => { method: "POST", body: '{"state":"pending","context":"Danger","target_url":"http://danger.systems/js","description":"message"}', }, - undefined + true ) }) @@ -181,7 +181,7 @@ describe("API testing", () => { method: "POST", body: '{"state":"failure","context":"Danger","target_url":"http://danger.systems/js","description":"message"}', }, - undefined + true ) }) @@ -200,7 +200,7 @@ describe("API testing", () => { method: "POST", body: '{"state":"success","context":"Danger","target_url":"http://example.org","description":"message"}', }, - undefined + true ) }) @@ -219,7 +219,7 @@ describe("API testing", () => { method: "POST", body: '{"state":"success","context":"Danger","target_url":"http://danger.systems/js","description":"message"}', }, - undefined + true ) }) diff --git a/source/runner/_tests/_executor.test.ts b/source/runner/_tests/_executor.test.ts index 61cfbf5f9..f0c24a12b 100644 --- a/source/runner/_tests/_executor.test.ts +++ b/source/runner/_tests/_executor.test.ts @@ -29,7 +29,7 @@ const defaultConfig = { const fakeCI = new FakeCI({}) const defaultDsl = (platform: any): Promise => { - return jsonDSLGenerator(platform, fakeCI).then(jsonDSL => { + return jsonDSLGenerator(platform, fakeCI, {} as any).then(jsonDSL => { jsonDSL.github = { pr: { number: 1, diff --git a/source/runner/dslGenerator.ts b/source/runner/dslGenerator.ts index 83482a8b9..493eabe6b 100644 --- a/source/runner/dslGenerator.ts +++ b/source/runner/dslGenerator.ts @@ -3,8 +3,13 @@ import { DangerDSLJSONType } from "../dsl/DangerDSL" import { CliArgs } from "../dsl/cli-args" import { CISource } from "../ci_source/ci_source" import { emptyGitJSON } from "../platforms/github/GitHubGit" +import { CommanderStatic } from "commander" -export const jsonDSLGenerator = async (platform: Platform, source: CISource): Promise => { +export const jsonDSLGenerator = async ( + platform: Platform, + source: CISource, + program: CommanderStatic +): Promise => { const useSimpleDSL = platform.getPlatformReviewSimpleRepresentation && source.useEventDSL const git = useSimpleDSL ? emptyGitJSON() : await platform.getPlatformGitRepresentation() @@ -15,6 +20,14 @@ export const jsonDSLGenerator = async (platform: Platform, source: CISource): Pr const platformDSL = await getDSLFunc!() + const cliArgs: CliArgs = { + base: program.base, + dangerfile: program.dangerfile, + externalCiProvider: program.externalCiProvider, + id: program.id, + textOnly: program.textOnly, + verbose: program.verbose, + } return { git, [platform.name === "BitBucketServer" ? "bitbucket_server" : "github"]: platformDSL, @@ -24,7 +37,7 @@ export const jsonDSLGenerator = async (platform: Platform, source: CISource): Pr additionalHeaders: {}, baseURL: process.env["DANGER_GITHUB_API_BASE_URL"] || undefined, }, - cliArgs: {} as CliArgs, + cliArgs, }, } } diff --git a/source/runner/jsonToContext.ts b/source/runner/jsonToContext.ts index a30db1c4f..ad3a5ceb6 100644 --- a/source/runner/jsonToContext.ts +++ b/source/runner/jsonToContext.ts @@ -3,6 +3,7 @@ import { jsonToDSL } from "./jsonToDSL" import { contextForDanger, DangerContext } from "./Dangerfile" import { DangerDSLJSON } from "./dangerDSLJSON" import { CISource } from "../ci_source/ci_source" +import { CommanderStatic } from "commander" /** * Reads in the JSON string converts to a dsl object and gets the change context @@ -11,8 +12,12 @@ import { CISource } from "../ci_source/ci_source" * @param program {any} commander * @returns {Promise} context for danger */ -export async function jsonToContext(JSONString: string, program: any, source: CISource): Promise { - const dslJSON = { danger: new DangerDSLJSON(JSONString, program as CliArgs) } +export async function jsonToContext( + JSONString: string, + program: CommanderStatic, + source: CISource +): Promise { + const dslJSON = { danger: new DangerDSLJSON(JSONString, (program as any) as CliArgs) } const dsl = await jsonToDSL(dslJSON.danger, source) return contextForDanger(dsl) } diff --git a/source/runner/runners/_tests/vm2.test.ts b/source/runner/runners/_tests/vm2.test.ts index 45936ce74..8e790b0a3 100644 --- a/source/runner/runners/_tests/vm2.test.ts +++ b/source/runner/runners/_tests/vm2.test.ts @@ -48,7 +48,7 @@ runners.forEach(run => { const source = new FakeCI({}) exec = new Executor(source, platform, run.fn, config) - const dsl = await jsonDSLGenerator(platform, new FakeCI({})) + const dsl = await jsonDSLGenerator(platform, new FakeCI({}), {} as any) dsl.github = { pr: { number: 1,