Skip to content

Commit

Permalink
Runs in prod, doesn't work in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
orta committed Oct 14, 2017
2 parents 21ce74d + e6e319e commit 9f0ac21
Show file tree
Hide file tree
Showing 13 changed files with 328 additions and 404 deletions.
51 changes: 51 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# How to contribute

## Setup

```sh
git clone https://github.com/danger/danger-js.git
cd danger-js

# if you don't have yarn installed
npm install -g yarn

yarn install
```

You can then verify your install by running the tests, and the linters:

```sh
yarn test
yarn lint
```

The fixers for both tslint and prettier will be applied when you commit, and on a push your code will be verified
that it compiles.

### What is the TODO?

Check the issues, I try and keep my short term perspective there. Long term is in the [VISION.md](VISION.md).

### Releasing a new version of Danger

Following [this commit](https://github.com/danger/danger-js/commit/a26ac3b3bd4f002acd37f6a363c8e74c9d5039ab) as a model:

- Checkout the `master` branch. Ensure your working tree is clean, and make sure you have the latest changes by running `git pull`.
- Update `package.json` with the new version - for the sake of this example, the new version is **0.21.0**.
- Modify `changelog.md`, adding a new `### 0.21.0` heading under the `### Master` heading at the top of the file.
- Commit both changes with the commit message **Version bump**.
- Tag this commit - `git tag 0.21.0`.
- Push the commit and tag to master - `git push origin master --follow-tags`. Travis CI will build the tagged commit and publish that tagged version to NPM.

:ship:

## License, Contributor's Guidelines and Code of Conduct

We try to keep as much discussion as possible in GitHub issues, but also have a pretty inactive Slack --- if you'd like an invite, ping [@Orta](https://twitter.com/orta/) a DM on Twitter with your email. It's mostly interesting if you want to stay on top of Danger without all the emails from GitHub.

> This project is open source under the MIT license, which means you have full access to the source code and can modify it to fit your own needs.
>
> This project subscribes to the [Moya Contributors Guidelines](https://github.com/Moya/contributors) which TLDR: means we give out push access easily and often.
>
> Contributors subscribe to the [Contributor Code of Conduct](http://contributor-covenant.org/version/1/3/0/) based on the [Contributor Covenant](http://contributor-covenant.org) version 1.3.0.
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2016 Danger
Copyright (c) 2017 Orta Therox

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Some quick links to get you started:

## This thing is broken, I should help improve it!

Awesommmmee.
Awesommmmee. Everything you need is down below. You can also refer to [CONTRIBUTING](CONTRIBUTING.md) file where you'll find the same information listed below.

```sh
git clone https://github.com/danger/danger-js.git
Expand Down
17 changes: 16 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
// Please add your own contribution below inside the Master section, ideally with a consumer's perspective in mind.
// Please add your own contribution below inside the Master section
// These docs are aimed at users rather than danger developers, so please limit technical
// terminology to in here.

### Master


### 2.0.0-alpha.18 - 19

- 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.17


- Improve CircleCI PR detection

### 2.0.0-alpha.16

Some UX fixes:

- Don't show warnings about not setting a commit status (unless in verbose) - orta
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "danger",
"version": "2.0.0-alpha.15",
"version": "2.0.0-alpha.19",
"description": "Unit tests for Team Culture",
"main": "distribution/danger.js",
"typings": "distribution/danger.d.ts",
Expand Down
10 changes: 5 additions & 5 deletions source/ci_source/_tests/_get_ci_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
36 changes: 18 additions & 18 deletions source/ci_source/get_ci_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions source/ci_source/providers/Circle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class Circle implements CISource {
}

get isPR(): boolean {
if (ensureEnvKeysExist(this.env, ["CI_PULL_REQUEST"])) {
if (ensureEnvKeysExist(this.env, ["CI_PULL_REQUEST"]) || ensureEnvKeysExist(this.env, ["CIRCLE_PULL_REQUEST"])) {
return true
}

Expand All @@ -50,7 +50,7 @@ export class Circle implements CISource {
}

private _prParseURL(): { owner?: string; reponame?: string; id?: string } {
const prUrl = this.env.CI_PULL_REQUEST || ""
const prUrl = this.env.CI_PULL_REQUEST || this.env.CIRCLE_PULL_REQUEST || ""
const splitSlug = prUrl.split("/")
if (splitSlug.length === 7) {
const owner = splitSlug[3]
Expand Down
10 changes: 9 additions & 1 deletion source/ci_source/providers/_tests/_circle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const correctEnv = {
CIRCLE_PROJECT_REPONAME: "someproject",
CIRCLE_BUILD_NUM: "1501",
CIRCLE_PR_NUMBER: "800",
CIRCLE_PULL_REQUEST: "https://github.com/artsy/eigen/pull/800",
CI_PULL_REQUEST: "https://github.com/artsy/eigen/pull/800",
}

Expand Down Expand Up @@ -48,6 +49,7 @@ describe(".isPR", () => {
CIRCLE_PROJECT_REPONAME: "someproject",
CIRCLE_BUILD_NUM: "1501",
CIRCLE_PR_NUMBER: "800",
CIRCLE_PULL_REQUEST: "https://github.com/artsy/eigen/pull/800",
CI_PULL_REQUEST: "https://github.com/artsy/eigen/pull/800",
}
env[key] = null
Expand All @@ -68,7 +70,7 @@ describe(".isPR", () => {
})
})

describe(".pullReuestID", () => {
describe(".pullRequestID", () => {
it("pulls it out of the env", () => {
const circle = new Circle({ CIRCLE_PR_NUMBER: "800" })
expect(circle.pullRequestID).toEqual("800")
Expand All @@ -78,8 +80,14 @@ describe(".pullReuestID", () => {
let env = {
CI_PULL_REQUEST: "https://github.com/artsy/eigen/pull/23",
}
let env2 = {
CIRCLE_PULL_REQUEST: "https://github.com/artsy/eigen/pull/23",
}
const circle = new Circle(env)
expect(circle.pullRequestID).toEqual("23")

const circle2 = new Circle(env2)
expect(circle2.pullRequestID).toEqual("23")
})
})

Expand Down
2 changes: 1 addition & 1 deletion source/commands/utils/getRuntimeCISource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?")
Expand Down
3 changes: 2 additions & 1 deletion source/commands/utils/runDangerSubprocess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const runDangerSubprocess = (subprocessName: string, dslJSONString: string, exec
child.on("close", async code => {
console.log(`child process exited with code ${code}`)
// Submit an error back to the PR
if (process.exitCode) {
if (code) {
process.exitCode = code
const results = resultsWithFailure(`${subprocessName}\` failed.`, "### Log\n\n" + markdownCode(allLogs))
await exec.handleResults(results)
}
Expand Down
Loading

0 comments on commit 9f0ac21

Please sign in to comment.