Skip to content

Commit

Permalink
Update the DSL to expose a file contents API
Browse files Browse the repository at this point in the history
  • Loading branch information
orta committed Jul 28, 2017
1 parent 7d8528a commit ff6e6e8
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 6 deletions.
6 changes: 6 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

### Master

- Exposes an internal API for reading a file from a GitHub repo as `danger.github.utils.fileContents` - orta

Ideally this is what you should be using in plugins to read files, it's what Danger uses throughout the
codebase internally. This means that your plugin/dangerfile doesn't need to rely on running on the CI
when instead it could run via the GitHub API.

- Update prettier - orta

### 1.1.0
Expand Down
15 changes: 15 additions & 0 deletions docs/usage/extending-danger.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,27 @@ The default template comes with an example of a simple Danger plugin in `src/ind

This is the file where you want to paste in your existing Dangerfile code. Note, that you should not add a `import { danger, et, cetera } from "danger"` in your plugin. The dirty truth is that Danger completely ignores that at runtime, and just puts all of it's exports inside the global context. You'll get compiler errors, or confusing shadow variable issues. So assume that it's there, and working fine.

## Recommendations

* If you need to access the contents of a file in the repo, use `danger.github.utils.fileContents` instead of `fs.readFile`, this will mean that [Peril](https://github.com/danger/peril) (hosted Danger) will work with your plugin.

## Testing

Now that you're in the secret global club, you can see in `src/index.test.{j,t}s` that you can easily mock the Danger API. Calls like `warning`, `fail`, `message` and `markdown` can easily be tested via `jest.fn` mocks, and the Danger API can be stubbed by setting a JS object which is shaped how you'd like it to be.

Should be pretty easy to make Act Arrange and Assert style tests with that infrastructure in place.

If you want to test locally against a known PR what I have done in the past is:

* Ship a 0.0.1 release to NPM
* Run `yarn link` inside my plugin's working directory
* Then in an app that uses DangerJS, I add the dependency with `yarn add [plugin] -d`
* Then I convert that dependency to use a symlink to my working directory with `yarn link [plugin]`
* Then I make sure that my library's transpiled code is always up-to-date with `yarn tsc -- --watch`
* _Now_ I can use `danger pr` to run any existing pull request: `yarn danger -- pr -v https://github.com/artsy/emission/pull/597`

Note that using `danger pr` more than 2-3 times would require that you send authenticated requests, so make sure that your shell has the environment `DANGER_GITHUB_API_TOKEN` set to a valid token.

## Shipping

With your code tested and ready, you can publish - update your app's `package.json` and import your new node module into your Dangerfile. Done and dusted.
Expand Down
10 changes: 10 additions & 0 deletions source/danger.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,16 @@ declare module "danger" {
* @returns {string} A HTML string of <a>'s built as a sentence.
*/
fileLinks(paths: string[], useBasename?: boolean, repoSlug?: string, branch?: string): string

/**
* Downloads a file's contents via the GitHub API. You'll want to use
* this instead of `fs.readFile` when aiming to support working with Peril.
*
* @param {string} path The path fo the file that exists
* @param {string} repoSlug An optional reference to the repo's slug: e.g. danger/danger-js
* @param {string} ref An optional reference to a branch/sha
*/
fileContents(path: string, repoSlug?: string, ref?: string): Promise<string>
}

/**
Expand Down
10 changes: 10 additions & 0 deletions source/dsl/GitHubDSL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ export interface GitHubUtilsDSL {
* @returns {string} A HTML string of <a>'s built as a sentence.
*/
fileLinks(paths: string[], useBasename?: boolean, repoSlug?: string, branch?: string): string

/**
* Downloads a file's contents via the GitHub API. You'll want to use
* this instead of `fs.readFile` when aiming to support working with Peril.
*
* @param {string} path The path fo the file that exists
* @param {string} repoSlug An optional reference to the repo's slug: e.g. danger/danger-js
* @param {string} ref An optional reference to a branch/sha
*/
fileContents(path: string, repoSlug?: string, ref?: string): Promise<string>
}

/**
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/GitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class GitHub {
reviews,
requested_reviewers,
thisPR,
utils: GitHubUtils(pr),
utils: GitHubUtils(pr, this.api),
}
}

Expand Down
5 changes: 3 additions & 2 deletions source/platforms/github/GitHubUtils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { basename } from "path"
import { sentence, href } from "../../runner/DangerUtils"
import { GitHubPRDSL, GitHubUtilsDSL } from "./../../dsl/GitHubDSL"

import { GitHubAPI } from "../github/GitHubAPI"
// We need to curry in access to the GitHub PR metadata

const utils = (pr: GitHubPRDSL): GitHubUtilsDSL => {
const utils = (pr: GitHubPRDSL, api: GitHubAPI): GitHubUtilsDSL => {
/**
* Converts a set of filepaths into a sentence'd set of hrefs for the
* current PR. Can be configured to just show the name (instead of full filepath), to
Expand All @@ -27,6 +27,7 @@ const utils = (pr: GitHubPRDSL): GitHubUtilsDSL => {

return {
fileLinks,
fileContents: api.fileContents,
}
}

Expand Down
17 changes: 14 additions & 3 deletions source/platforms/github/_tests/_github_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import { resolve } from "path"
const fixtures = resolve(__dirname, "..", "..", "_tests", "fixtures")
const fixuredData = path => JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString())
const pr = fixuredData("github_pr.json")
const apiFake = {
fileContents: jest.fn(),
} as any

describe("fileLinks", () => {
it("Should convert a few paths into links", () => {
const sut = utils(pr)
const sut = utils(pr, apiFake)
const links = sut.fileLinks(["a/b/c", "d/e/f"])
const url = "https://github.com/artsy/emission/blob/genevc/a/b/c"
expect(links).toEqual(
Expand All @@ -18,17 +21,25 @@ describe("fileLinks", () => {
})

it("Should convert a few paths into links showing full links", () => {
const sut = utils(pr)
const sut = utils(pr, apiFake)
const links = sut.fileLinks(["a/b/c", "d/e/f"], false)
const url = "https://github.com/artsy/emission/blob/genevc"
expect(links).toEqual(`<a href="${url}/a/b/c">a/b/c</a> and <a href="${url}/d/e/f">d/e/f</a>`)
})

it("Should convert a few paths into links showing full link on a custom fork/branch", () => {
const sut = utils(pr)
const sut = utils(pr, apiFake)
const links = sut.fileLinks(["a/b/c", "d/e/f"], false, "orta/emission", "new")
const url = "https://github.com/orta/emission"

expect(links).toEqual(`<a href="${url}/blob/new/a/b/c">a/b/c</a> and <a href="${url}/blob/new/d/e/f">d/e/f</a>`)
})
})

describe("getContents", () => {
it("should call the API's getContents", () => {
const sut = utils(pr, apiFake)
sut.fileContents("/a/b/c.ts")
expect(apiFake.fileContents).toHaveBeenCalledWith("/a/b/c.ts")
})
})

0 comments on commit ff6e6e8

Please sign in to comment.