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

Upgrade @octokit/rest from v16.43.1 to v18.12.0 - Fixes #1202 #1204

Merged
merged 5 commits into from Feb 1, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -15,6 +15,7 @@

<!-- Your comment below this -->

- *Breaking:* Upgrade @octokit/rest from v16.43.1 to v18.12.0 - [#1204](https://github.com/danger/danger-js/pull/1204) [@fbartho]

<!-- Your comment above this -->

Expand Down
7 changes: 4 additions & 3 deletions package.json
Expand Up @@ -94,6 +94,7 @@
"@babel/plugin-transform-typescript": "7.1.0",
"@babel/preset-env": "7.1.0",
"@babel/traverse": "7.1.0",
"@octokit/openapi-types": "^11.2.0",
"@types/async-retry": "^1.4.1",
"@types/debug": "0.0.30",
"@types/get-stdin": "^5.0.1",
Expand Down Expand Up @@ -132,12 +133,12 @@
"tslint": "^5.11.0",
"tslint-config-prettier": "^1.15.0",
"typedoc": "0.9.0",
"typescript": "^3.9.7",
"typescript": "^4.5.5",
"typescript-json-schema": "^0.32.0"
},
"dependencies": {
"@babel/polyfill": "^7.2.5",
"@octokit/rest": "^16.43.1",
"@octokit/rest": "^18.12.0",
"async-retry": "1.2.3",
"chalk": "^2.3.0",
"commander": "^2.18.0",
Expand All @@ -157,7 +158,7 @@
"lodash.keys": "^4.0.8",
"lodash.mapvalues": "^4.6.0",
"lodash.memoize": "^4.1.2",
"memfs-or-file-map-to-github-branch": "^1.1.0",
"memfs-or-file-map-to-github-branch": "^1.2.1",
"micromatch": "^4.0.4",
"node-cleanup": "^2.1.2",
"node-fetch": "^2.6.7",
Expand Down
6 changes: 3 additions & 3 deletions scripts/run-fixtures.js
Expand Up @@ -5,7 +5,7 @@

// Toggle this on to update the JSON files for each run
// or use `yarn test:update-fixtures`
const writeResults = false || process.argv.includes('--update')
const writeResults = false || process.argv.includes("--update")

const fs = require("fs")
const child_process = require("child_process")
Expand All @@ -32,7 +32,7 @@ const fixtures = fs

let runCount = 0

console.log("Running Fixures for Danger JS. This uses the built version of danger.\n")
console.log("Running Fixtures for Danger JS. This uses the built version of danger.\n")

// Runs the danger runner over a fixture, then compares it to the
// fixtured JSON data
Expand Down Expand Up @@ -107,7 +107,7 @@ const next = () => {
}
}

process.on("unhandledRejection", function (reason, _p) {
process.on("unhandledRejection", function(reason, _p) {
console.log(chalk.red("Error: "), reason)
process.exitCode = 1
})
Expand Down
8 changes: 4 additions & 4 deletions source/platforms/bitbucket_cloud/BitBucketCloudAPI.ts
Expand Up @@ -310,7 +310,7 @@ export class BitBucketCloudAPI {
// API implementation
private api = async (url: string, headers: any = {}, body: any = {}, method: string, suppressErrors?: boolean) => {
if (this.credentials.type === "PASSWORD") {
headers["Authorization"] = `Basic ${new Buffer(
headers["Authorization"] = `Basic ${Buffer.from(
this.credentials.username + ":" + this.credentials.password
).toString("base64")}`
} else {
Expand All @@ -324,9 +324,9 @@ export class BitBucketCloudAPI {
this.oauthURL,
{
...headers,
Authorization: `Basic ${new Buffer(this.credentials.oauthKey + ":" + this.credentials.oauthSecret).toString(
"base64"
)}`,
Authorization: `Basic ${Buffer.from(
this.credentials.oauthKey + ":" + this.credentials.oauthSecret
).toString("base64")}`,
"Content-Type": "application/x-www-form-urlencoded",
},
params,
Expand Down
Expand Up @@ -22,7 +22,7 @@ describe("API testing - BitBucket Cloud", () => {
let textResult: string
const expectedJSONHeaders = {
"Content-Type": "application/json",
Authorization: `Basic ${new Buffer("username:password").toString("base64")}`,
Authorization: `Basic ${Buffer.from("username:password").toString("base64")}`,
}

function APIFactory(username: string, password: string, uuid: string) {
Expand Down Expand Up @@ -354,7 +354,7 @@ describe("API testing - BitBucket Cloud", () => {

const expectedAuthHeaders = {
"Content-Type": "application/x-www-form-urlencoded",
Authorization: `Basic ${new Buffer("superOAUTHKey:superSecretOAUTH").toString("base64")}`,
Authorization: `Basic ${Buffer.from("superOAUTHKey:superSecretOAUTH").toString("base64")}`,
}
const expectedOAUTHRequestHeaders = {
"Content-Type": "application/json",
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/bitbucket_server/BitBucketServerAPI.ts
Expand Up @@ -339,7 +339,7 @@ export class BitBucketServerAPI implements BitBucketServerAPIDSL {
if (this.repoCredentials.token) {
headers["Authorization"] = `Bearer ${this.repoCredentials.token}`
} else if (this.repoCredentials.password) {
headers["Authorization"] = `Basic ${new Buffer(
headers["Authorization"] = `Basic ${Buffer.from(
this.repoCredentials.username + ":" + this.repoCredentials.password
).toString("base64")}`
}
Expand Down
Expand Up @@ -9,7 +9,7 @@ describe("API testing - BitBucket Server", () => {
const host = "http://localhost:7990"
const expectedJSONHeaders = {
"Content-Type": "application/json",
Authorization: `Basic ${new Buffer("username:password").toString("base64")}`,
Authorization: `Basic ${Buffer.from("username:password").toString("base64")}`,
}

function APIFactory({ password, token }: { password?: string; token?: string }) {
Expand Down
6 changes: 3 additions & 3 deletions source/platforms/github/GitHubAPI.ts
Expand Up @@ -46,7 +46,7 @@ export class GitHubAPI {
const token = accessTokenForApp || this.token!

const host = process.env["DANGER_GITHUB_API_BASE_URL"] || process.env["GITHUB_URL"] || undefined
const options: GitHubNodeAPI.Options & { debug: boolean } = {
const options: ConstructorParameters<typeof GitHubNodeAPI>[0] & { debug: boolean } = {
debug: !!process.env.LOG_FETCH_REQUESTS,
baseUrl: host,
auth: `token ${token}`,
Expand Down Expand Up @@ -76,7 +76,7 @@ export class GitHubAPI {
}

const data = await this.getFileContents(path, repoSlug, ref)
const buffer = new Buffer(data.content, "base64")
const buffer = Buffer.from(data.content, "base64")
return buffer.toString()
}

Expand Down Expand Up @@ -421,7 +421,7 @@ export class GitHubAPI {
return res.ok
} catch (error) {
this.d(`Posting a status to: ${statusURL} failed, this is the response:`)
this.d(error.message)
this.d((error && (error as Error).message) || error)
}
}

Expand Down
41 changes: 32 additions & 9 deletions source/platforms/github/GitHubUtils.ts
@@ -1,8 +1,20 @@
import { basename } from "path"
import { components as OctokitOpenApiTypes } from "@octokit/openapi-types"
import { filepathContentsMapToUpdateGitHubBranch, BranchCreationConfig } from "memfs-or-file-map-to-github-branch"

import { sentence, href } from "../../runner/DangerUtils"
import { GitHubPRDSL, GitHubUtilsDSL } from "./../../dsl/GitHubDSL"
import { debug } from "../../debug"
import { filepathContentsMapToUpdateGitHubBranch, BranchCreationConfig } from "memfs-or-file-map-to-github-branch"

export type GetContentResponseData =
| OctokitOpenApiTypes["schemas"]["content-file"]
| OctokitOpenApiTypes["schemas"]["content-symlink"]
| OctokitOpenApiTypes["schemas"]["content-submodule"]
export function isFileContents(
response: GetContentResponseData
): response is OctokitOpenApiTypes["schemas"]["content-file"] {
return response.type === "file"
}

const d = debug("GitHub::Utils")

Expand Down Expand Up @@ -66,14 +78,18 @@ export const fileContentsGenerator = (
owner: repoSlug.split("/")[0],
}
try {
// response of getContents() can be one of 4 things. We are interested in file responses only
// https://developer.github.com/v3/repos/contents/#get-contents
const response = await api.repos.getContents(opts)
// response of getContent() can be one of 4 things. We are interested in file responses only
// https://docs.github.com/en/rest/reference/repos#get-repository-content
const response = await api.repos.getContent(opts)
if (!response || !response.data) {
return ""
}
if (Array.isArray(response.data)) {
// If we get an array, we have a directory
return ""
}
if (response && response.data && response.data.content) {
const buffer = new Buffer(response.data.content, response.data.encoding)
if (isFileContents(response.data) && response.data.content) {
const buffer = Buffer.from(response.data.content, response.data.encoding)
return buffer.toString()
} else {
return ""
Expand All @@ -94,7 +110,7 @@ export const createUpdatedIssueWithIDGenerator = (api: GitHub) => async (
// by label
const uniqueHeader = `Danger-Issue-ID-${id.replace(/ /g, "_")}`
const q = `user:${settings.owner} repo:${settings.repo} ${uniqueHeader}`
const { data: searchResults } = await api.search.issues({ q })
const { data: searchResults } = await api.search.issuesAndPullRequests({ q })
d(`Got ${searchResults.total_count} for ${uniqueHeader}`)

const body = `${content}\n\n${uniqueHeader}`
Expand All @@ -104,7 +120,14 @@ export const createUpdatedIssueWithIDGenerator = (api: GitHub) => async (
if (searchResults.total_count > 0 && searchResults.items[0]) {
const issueToUpdate = searchResults.items[0]
d(`Found: ${issueToUpdate}`)
const { data: issue } = await api.issues.update({ body, owner, repo, title, number: issueToUpdate.number, state })
const { data: issue } = await api.issues.update({
body,
owner,
repo,
title,
issue_number: issueToUpdate.number,
state,
})
return issue.html_url
} else {
const { data: issue } = await api.issues.create({ body, owner, repo, title })
Expand Down Expand Up @@ -161,7 +184,7 @@ export const createOrUpdatePR = (pr: GitHubPRDSL | undefined, api: GitHub) => as
if (existingPR) {
d("Updating existing PR")
return await api.pulls.update({
number: existingPR.number,
pull_number: existingPR.number,
base: config.baseBranch,
owner,
repo,
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/github/_tests/_github_git.test.ts
Expand Up @@ -64,7 +64,7 @@ describe("the dangerfile gitDSL", () => {

nodeGitHubAPI = new NodeGitHub()
const mockContents = async ({ ref }: any) => (await requestWithFixturedJSON(`static_file.${ref}.json`))()
nodeGitHubAPI.repos.getContents = mockContents as any
nodeGitHubAPI.repos.getContent = mockContents as any

gitJSONDSL = await github.getPlatformGitRepresentation()
const githubJSONDSL = await github.getPlatformReviewDSLRepresentation()
Expand Down
12 changes: 6 additions & 6 deletions source/platforms/github/_tests/_github_utils.test.ts
Expand Up @@ -4,11 +4,11 @@ import { readFileSync } from "fs"
import { resolve } from "path"

const fixtures = resolve(__dirname, "..", "..", "_tests", "fixtures")
const fixuredData = (path: string) => JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString())
const pr = fixuredData("github_pr.json")
const fixturedData = (path: string) => JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString())
const pr = fixturedData("github_pr.json")
const apiFake = {
repos: {
getContents: jest.fn(),
getContent: jest.fn(),
},
} as any

Expand Down Expand Up @@ -38,11 +38,11 @@ describe("fileLinks", () => {
})
})

describe("getContents", () => {
it("should call the API's getContents", () => {
describe("getContent", () => {
it("should call the API's getContent", () => {
const sut = utils(pr, apiFake)
sut.fileContents("/a/b/c.ts")
expect(apiFake.repos.getContents).toHaveBeenCalledWith({
expect(apiFake.repos.getContent).toHaveBeenCalledWith({
owner: "orta",
path: "/a/b/c.ts",
ref: "genevc",
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/github/_tests/fixturedGitHubDSL.ts
Expand Up @@ -54,7 +54,7 @@ export const fixturedGitHubDSL = async (): Promise<DangerDSLType> => {

nodeGitHubAPI = new NodeGitHub()
const mockContents = async ({ ref }: any) => (await requestWithFixturedJSON(`static_file.${ref}.json`))()
nodeGitHubAPI.repos.getContents = mockContents as any
nodeGitHubAPI.repos.getContent = mockContents as any

gitJSONDSL = await github.getPlatformGitRepresentation()
const githubJSONDSL = await github.getPlatformReviewDSLRepresentation()
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/github/comms/checks/resultsToCheck.ts
Expand Up @@ -70,7 +70,7 @@ export const resultsToCheck = async (
try {
// response of getContents() can be one of 4 things. We are interested in file responses only
// https://developer.github.com/v3/repos/contents/#get-contents
const { data } = await api.repos.getContents({
const { data } = await api.repos.getContent({
path,
ref: pr.head.sha,
repo: pr.head.repo.name,
Expand Down
2 changes: 1 addition & 1 deletion source/platforms/gitlab/GitLabAPI.ts
Expand Up @@ -237,7 +237,7 @@ class GitLabAPI {
} catch (e) {
this.d("getFileContents", e)
// GitHubAPI.fileContents returns "" when the file does not exist, keep it consistent across providers
if (e.response.status === 404) {
if ((e as any).response.status === 404) {
return ""
}
throw e
Expand Down
2 changes: 1 addition & 1 deletion source/runner/Executor.ts
Expand Up @@ -100,7 +100,7 @@ export class Executor {
try {
results = await this.runner.runDangerfileEnvironment([file], [undefined], runtime)
} catch (error) {
results = this.resultsForError(error)
results = this.resultsForError(error as Error)
}

await this.handleResults(results, runtime.danger.git)
Expand Down
2 changes: 1 addition & 1 deletion source/runner/jsonToDSL.ts
Expand Up @@ -85,7 +85,7 @@ const apiForDSL = (dsl: DangerDSLJSONType): Octokit | BitBucketServerAPI | GitLa
return new GitLabAPI(gitlab.metadata, getGitLabAPICredentialsFromEnv(process.env))
}

const options: Octokit.Options & { debug: boolean } = {
const options: ConstructorParameters<typeof Octokit>[0] & { debug: boolean } = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

debug: !!process.env.LOG_FETCH_REQUESTS,
baseUrl: dsl.settings.github.baseURL,
}
Expand Down
2 changes: 1 addition & 1 deletion source/runner/runners/inline.ts
Expand Up @@ -116,7 +116,7 @@ export const runDangerfileEnvironment = async (
d("Got a parse error: ", error)

// Call the internal functions to fail the build
const errorResults = resultsForCaughtError(filename, content, error)
const errorResults = resultsForCaughtError(filename, content, error as Error)
environment.markdown(errorResults.markdowns[0].message)
environment.fail(errorResults.fails[0].message)
}
Expand Down
1 change: 1 addition & 0 deletions source/runner/runners/utils/_tests/_transpiler.test.ts
@@ -1,5 +1,6 @@
jest.mock("fs", () => ({
readFileSync: jest.fn(),
realpathSync: {},
existsSync: jest.fn(),
}))
jest.mock("path", () => {
Expand Down