Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow returning a value and having Danger execute the code #574

Merged
merged 2 commits into from
Apr 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ matrix:
- node scripts/run-fixtures.js

# Does the real danger run
- node_js: node
- node_js: '9'
script:
- yarn jest --outputFile test-results.json --json --runInBand
- yarn run link
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
## Master

* Prints a link to the comment in the build log [@orta][]
* A Dangerfile can return a default export, and then Danger will handle the execution of that code [@orta][]

## 3.5.0 - 3.5.1

Expand Down
2 changes: 1 addition & 1 deletion scripts/run-fixtures.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Note: Keep this ES6 only, want to be able to run this directly via node
// to ensur that something like ts-node doesn't mess up paths etc
// to ensure that something like ts-node doesn't mess up paths etc

// 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 --inspect distribution/commands/danger-runner.js --text-only --dangerfile /Users/orta/dev/projects/danger/danger-js/source/runner/_tests/fixtures/__DangerfileAsync.js

Expand Down
1 change: 1 addition & 0 deletions source/runner/_tests/fixtures/__DangerfileDefaultExport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => warn("Synchronous Warning")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default async () =>
new Promise(res => {
setTimeout(() => {
warn("Asynchronous Warning")
res()
}, 10)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"fails": [],
"warnings": [
{
"message": "Synchronous Warning"
}
],
"messages": [],
"markdowns": [],
"scheduled": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"fails": [],
"warnings": [
{
"message": "Asynchronous Warning"
}
],
"messages": [],
"markdowns": [],
"scheduled": []
}
32 changes: 32 additions & 0 deletions source/runner/runners/_tests/vm2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,38 @@ runners.forEach(run => {
expect(results.fails[0].message).toContain("Danger failed to run")
expect(results.markdowns[0].message).toContain("Error: failure")
})

it("handles running default export code", async () => {
const context = await setupDangerfileContext()
const runtime = await exec.runner.createDangerfileRuntimeEnvironment(context)
const results = await exec.runner.runDangerfileEnvironment(
resolve(fixtures, "__DangerfileDefaultExport.js"),
undefined,
runtime
)
expect(results).toEqual({
fails: [],
messages: [],
markdowns: [],
warnings: [{ message: "Synchronous Warning" }],
})
})

it("handles running default export async code", async () => {
const context = await setupDangerfileContext()
const runtime = await exec.runner.createDangerfileRuntimeEnvironment(context)
const results = await exec.runner.runDangerfileEnvironment(
resolve(fixtures, "__DangerfileDefaultExportAsync.js"),
undefined,
runtime
)
expect(results).toEqual({
fails: [],
messages: [],
markdowns: [],
warnings: [{ message: "Asynchronous Warning" }],
})
})
})
})
})
18 changes: 13 additions & 5 deletions source/runner/runners/inline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,20 @@ export async function createDangerfileRuntimeEnvironment(dangerfileContext: Dang
* The values inside a Danger context are applied as globals to the Dangerfiles runtime.
*
* @param {string} filename the file path for the dangerfile
* @param {any} environment the results of createDangerfileRuntimeEnvironment
* @param {string} originalContents optional, the JS pre-compiled
* @param {DangerContext} environment the results of createDangerfileRuntimeEnvironment
* @param {any | undefined} injectedObjectToExport an optional object for passing into default exports
* @returns {DangerResults} the results of the run
*/
export async function runDangerfileEnvironment(
filename: string,
originalContents: string | undefined,
environment: DangerContext
environment: DangerContext,
injectedObjectToExport: any | undefined = undefined
): Promise<DangerResults> {
// We need to change the local runtime to support running JavaScript
// and TypeScript through babel first. This is a simple implmentation
// and if we need more nuance, then we can look at other implementations
// and TypeScript through babel first. This is a simple implementation
// and if we need more nuance, then we can look at other options
const customModuleHandler = (module: any, filename: string) => {
if (!filename.includes("node_modules")) {
d("Handling custom module: ", filename)
Expand Down Expand Up @@ -70,7 +73,12 @@ export async function runDangerfileEnvironment(
}

d("Started parsing Dangerfile: ", filename)
_require(compiled, filename, {})
const optionalExport = _require(compiled, filename, {})

if (typeof optionalExport.default === "function") {
d("Running default export from Dangerfile", filename)
await optionalExport.default(injectedObjectToExport)
}
d("Finished running dangerfile: ", filename)
// Don't stop all current async code from breaking,
// however new code (without Peril support) can run
Expand Down
8 changes: 5 additions & 3 deletions source/runner/runners/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@ export interface DangerRunner {
* The values inside a Danger context are applied as globals to the Dangerfiles runtime.
*
* @param {string} filename the file path for the dangerfile
* @param {string | undefined} originalContents the contents of the Dangerfile, or undefined to use fs.readFileSync
* @param {any} environment the resuts of createDangerfileRuntimeEnvironment
* @param {string} originalContents optional, the JS pre-compiled
* @param {DangerContext} environment the results of createDangerfileRuntimeEnvironment
* @param {any | undefined} injectedObjectToExport an optional object for passing into default exports
* @returns {DangerResults} the results of the run
*/
runDangerfileEnvironment: (
filename: string,
originalContents: string | undefined,
environment: any
environment: any,
injectedObjectToExport?: any
) => Promise<DangerResults>

/**
Expand Down
8 changes: 6 additions & 2 deletions source/runner/runners/vm2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export async function createDangerfileRuntimeEnvironment(dangerfileContext: Dang
export async function runDangerfileEnvironment(
filename: string,
originalContents: string | undefined,
environment: NodeVMOptions
environment: NodeVMOptions,
injectedObjectToExport: any | undefined = undefined
) {
const vm = new NodeVM(environment)

Expand All @@ -48,7 +49,10 @@ export async function runDangerfileEnvironment(
let content = cleanDangerfile(originalContents)

try {
vm.run(content, filename)
const optionalExport = vm.run(content, filename)
if (typeof optionalExport.default === "function") {
await optionalExport.default(injectedObjectToExport)
}

const results = environment.sandbox!.results!
await Promise.all(
Expand Down