Merge pull request #383 from danger/ashfurrow-sync-methods

Moves away from Sync functions
danger · Oct 1, 2017 · 912b953 · 912b953
2 parents f0c224c + 5423a7d
commit 912b953
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 213 deletions.
diff --git a/changelog.md b/changelog.md
@@ -5,6 +5,7 @@
 // I'm adding it without a version number since I don't know what version it'll be if/when this is merged <_<
 
 - Improve CircleCI PR detection
+- Moves internal methods away from Sync to avoid problems when running in Peril - ashfurrow
 - Passes through non-zero exit codes from `danger process` runs - ashfurrow
 
 ### 2.0.0-alpha.16

diff --git a/source/ci_source/_tests/_get_ci_source.test.ts b/source/ci_source/_tests/_get_ci_source.test.ts
@@ -14,14 +14,14 @@ describe(".getCISourceForEnv", () => {
   })
 })
 
-describe(".getCISourceForExternal", () => {
-  test("should resolve module relatively", () => {
-    const ci = getCISourceForExternal({}, "./source/ci_source/_tests/fixtures/dummy_ci.js")
+describe(".getCISourceForExternal", async () => {
+  test("should resolve module relatively", async () => {
+    const ci = await getCISourceForExternal({}, "./source/ci_source/_tests/fixtures/dummy_ci.js")
     expect(ci).toBeInstanceOf(DummyCI)
   })
 
-  test("should return undefined if module resolution fails", () => {
-    const ci = getCISourceForExternal({}, "./dummy_ci.js")
+  test("should return undefined if module resolution fails", async () => {
+    const ci = await getCISourceForExternal({}, "./dummy_ci.js")
     expect(ci).toBeUndefined()
   })
 })
diff --git a/source/ci_source/get_ci_source.ts b/source/ci_source/get_ci_source.ts
@@ -20,35 +20,35 @@ export function getCISourceForEnv(env: Env): CISource | undefined {
  * @export
  * @param {Env} env The environment.
  * @param {string} modulePath relative path to CI provider
- * @returns {?CISource} a CI source if module loaded successfully, undefined otherwise
+ * @returns {Promise<?CISource>} a CI source if module loaded successfully, undefined otherwise
  */
-export function getCISourceForExternal(env: Env, modulePath: string): CISource | undefined {
+export async function getCISourceForExternal(env: Env, modulePath: string): Promise<CISource | undefined> {
   const path = resolve(process.cwd(), modulePath)
-
-  try {
-    const exist = fs.statSync(path).isFile()
-
-    if (exist) {
-      const externalModule = require(path) //tslint:disable-line:no-require-imports
-      const moduleConstructor = externalModule.default || externalModule
-      return new moduleConstructor(env)
-    }
-  } catch (e) {
-    console.error(`could not load CI provider at ${modulePath} due to ${e}`)
-  }
-  return undefined
+  return new Promise<CISource | undefined>(resolve => {
+    fs.stat(path, (error, stat) => {
+      if (error) {
+        console.error(`could not load CI provider at ${modulePath} due to ${error}`)
+      }
+      if (stat && stat.isFile()) {
+        const externalModule = require(path) //tslint:disable-line:no-require-imports
+        const moduleConstructor = externalModule.default || externalModule
+        resolve(new moduleConstructor(env))
+      }
+      resolve()
+    })
+  })
 }
 
 /**
  * Gets a CI Source.
  * @export
  * @param {Env} env The environment.
  * @param {string} modulePath relative path to CI provider
- * @returns {?CISource} a CI source if module loaded successfully, undefined otherwise
+ * @returns {Promise<?CISource>} a CI source if module loaded successfully, undefined otherwise
  */
-export function getCISource(env: Env, modulePath: string | undefined): CISource | undefined {
+export async function getCISource(env: Env, modulePath: string | undefined): Promise<CISource | undefined> {
   if (modulePath) {
-    const external = getCISourceForExternal(env, modulePath)
+    const external = await getCISourceForExternal(env, modulePath)
     if (external) {
       return external
     }

diff --git a/source/commands/utils/getRuntimeCISource.ts b/source/commands/utils/getRuntimeCISource.ts
@@ -5,7 +5,7 @@ import { SharedCLI } from "./sharedDangerfileArgs"
 import { CISource } from "../../ci_source/ci_source"
 
 const getRuntimeCISource = async (app: SharedCLI): Promise<CISource | undefined> => {
-  const source = getCISource(process.env, app.externalCiProvider || undefined)
+  const source = await getCISource(process.env, app.externalCiProvider || undefined)
 
   if (!source) {
     console.log("Could not find a CI source for this run. Does Danger support this CI service?")

diff --git a/source/runner/DangerfileRunner.ts b/source/runner/DangerfileRunner.ts
@@ -94,9 +94,25 @@ export async function runDangerfileEnvironment(
 ): Promise<DangerResults> {
   const vm = new NodeVM(environment)
 
+  const fetchContents = async () => {
+    return new Promise<string>((resolve, reject) => {
+      if (originalContents) {
+        resolve(originalContents)
+      } else {
+        fs.readFile(filename, "utf8", (error, data) => {
+          if (error) {
+            reject(error)
+          } else {
+            resolve(data)
+          }
+        })
+      }
+    })
+  }
+
   // Require our dangerfile
-  originalContents = originalContents || fs.readFileSync(filename, "utf8")
-  let content = cleanDangerfile(originalContents)
+  const fetchedContents = await fetchContents()
+  let content = cleanDangerfile(fetchedContents)
 
   // TODO: Relative imports get TS/Babel