@nerdwallet/shepherd - npm Package Compare versions

Comparing version 1.13.1 to 1.14.0

CHANGELOG.md

@@ -0,1 +1,8 @@
+# [1.14.0](https://github.com/NerdWalletOSS/shepherd/compare/v1.13.1...v1.14.0) (2021-02-11)
+
+
+### Features
+
+* add github service ([#377](https://github.com/NerdWalletOSS/shepherd/issues/377)) ([7bbd54a](https://github.com/NerdWalletOSS/shepherd/commit/7bbd54a40666ede0b45f8e247fad1a0a0f65cf2a))
+
 ## [1.13.1](https://github.com/NerdWalletOSS/shepherd/compare/v1.13.0...v1.13.1) (2021-01-18)
@@ -2,0 +9,0 @@

lib/adapters/github.d.ts

@@ -1,13 +0,8 @@
-import { Octokit } from '@octokit/rest';
 import { IMigrationContext } from '../migration-context';
 import { IEnvironmentVariables, IRepo } from './base';
 import GitAdapter from './git';
+import GithubService from '../services/github';
 declare class GithubAdapter extends GitAdapter {
-    private octokit;
-    /**
-     * Constructs a new GitHub adapter. The second parameter allows for
-     * dependency injection during testing. If an Octokit instance is not
-     * provided, one will be created and authenticated automatically.
-     */
-    constructor(migrationContext: IMigrationContext, octokit?: Octokit);
+    private githubService;
+    constructor(migrationContext: IMigrationContext, githubService: GithubService);
     getCandidateRepos(): Promise<IRepo[]>;
@@ -14,0 +9,0 @@ mapRepoAfterCheckout(repo: Readonly<IRepo>): Promise<IRepo>;

lib/adapters/github.js

@@ -16,9 +16,6 @@ "use strict";
 /* eslint-disable class-methods-use-this */
-const rest_1 = require("@octokit/rest");
 const chalk_1 = __importDefault(require("chalk"));
 const lodash_1 = __importDefault(require("lodash"));
-const netrc_1 = __importDefault(require("netrc"));
 const path_1 = __importDefault(require("path"));
 const git_1 = __importDefault(require("./git"));
-const VALID_SEARCH_TYPES = ['code', 'repositories'];
 var SafetyStatus;
@@ -31,24 +28,6 @@ (function (SafetyStatus) {
 class GithubAdapter extends git_1.default {
-    /**
-     * Constructs a new GitHub adapter. The second parameter allows for
-     * dependency injection during testing. If an Octokit instance is not
-     * provided, one will be created and authenticated automatically.
-     */
-    constructor(migrationContext, octokit) {
+    constructor(migrationContext, githubService) {
         super(migrationContext);
         this.migrationContext = migrationContext;
-        if (octokit) {
-            this.octokit = octokit;
-        }
-        else {
-            const netrcAuth = netrc_1.default();
-            const token = process.env.GITHUB_TOKEN || lodash_1.default.get(netrcAuth['api.github.com'], 'password', undefined);
-            if (!token) {
-                throw new Error(`No Github credentials found; set either GITHUB_TOKEN or 
-        set a token on the 'password' field in ~/.netrc for api.github.com`);
-            }
-            this.octokit = new rest_1.Octokit({
-                auth: token
-            });
-        }
+        this.githubService = githubService;
     }
@@ -58,4 +37,4 @@ getCandidateRepos() {
             const { org, search_type, search_query } = this.migrationContext.migration.spec.adapter;
-            let repoNames = [];
-            // list all of an orgs repos
+            let repoNames;
+            // list all of an orgs active repos
             if (org) {
@@ -65,25 +44,9 @@ if (search_query) {
                 }
-                const repos = yield this.octokit.paginate(this.octokit.repos.listForOrg, { org });
-                const unarchivedRepos = repos.filter((r) => !r.archived);
-                repoNames = unarchivedRepos.map((r) => r.full_name).sort();
+                repoNames = yield this.githubService.getActiveReposForOrg({ org });
             }
             else {
-                if (search_type && !VALID_SEARCH_TYPES.includes(search_type)) {
-                    throw new Error(`"search_type" must be one of the following: 
-          ${VALID_SEARCH_TYPES.map(e => `'${e}'`).join(' | ')}`);
-                }
-                let searchMethod;
-                let fullNamePath;
-                switch (search_type) {
-                    case 'repositories':
-                        searchMethod = this.octokit.search.repos;
-                        fullNamePath = 'full_name';
-                        break;
-                    case 'code':
-                    default:
-                        searchMethod = this.octokit.search.code; // github code search query. results are less reliable
-                        fullNamePath = 'repository.full_name';
-                }
-                const searchResults = yield this.octokit.paginate(searchMethod, { q: search_query }, (r) => r.data);
-                repoNames = searchResults.map((r) => lodash_1.default.get(r, fullNamePath)).sort();
+                repoNames = yield this.githubService.getActiveReposForSearchTypeAndQuery({
+                    search_type,
+                    search_query
+                });
             }
@@ -96,7 +59,7 @@ return lodash_1.default.uniq(repoNames).map((r) => this.parseRepo(r));
             const { owner, name } = repo;
-            const { data } = yield this.octokit.repos.get({
+            const defaultBranch = yield this.githubService.getDefaultBranchForRepo({
                 owner,
                 repo: name,
             });
-            return Object.assign(Object.assign({}, repo), { defaultBranch: data.default_branch });
+            return Object.assign(Object.assign({}, repo), { defaultBranch });
         });
@@ -168,3 +131,3 @@ }
             // Let's check if a PR already exists
-            const { data: pullRequests } = yield this.octokit.pulls.list({
+            const pullRequests = yield this.githubService.listPullRequests({
                 owner,
@@ -178,3 +141,3 @@ repo: name,
                     // A pull request exists and is open, let's update it
-                    yield this.octokit.pulls.update({
+                    yield this.githubService.updatePullRequest({
                         owner,
@@ -195,3 +158,3 @@ repo: name,
                 // No PR yet - we have to create it
-                yield this.octokit.pulls.create({
+                yield this.githubService.createPullRequest({
                     owner,
@@ -212,3 +175,3 @@ repo: name,
             // First, check for a pull request
-            const { data: pullRequests } = yield this.octokit.pulls.list({
+            const pullRequests = yield this.githubService.listPullRequests({
                 owner,
@@ -221,3 +184,3 @@ repo: name,
                 // GitHub's API is weird - you need a second query to get information about mergeability
-                const { data: pullRequest } = yield this.octokit.pulls.get({
+                const { data: pullRequest } = yield this.githubService.getPullRequest({
                     owner,
@@ -239,3 +202,3 @@ repo: name,
                     // by things like required reviews
-                    const combinedStatus = yield this.octokit.repos.getCombinedStatusForRef({
+                    const combinedStatus = yield this.githubService.getCombinedRefStatus({
                         owner,
@@ -272,3 +235,3 @@ repo: name,
                     // This will throw an exception if the branch does not exist
-                    yield this.octokit.repos.getBranch({
+                    yield this.githubService.getBranch({
                         owner,
@@ -324,3 +287,3 @@ repo: name,
                 // a branch yet
-                const pullRequests = yield this.octokit.paginate(this.octokit.pulls.list, {
+                const pullRequests = yield this.githubService.listPullRequests({
                     owner,
@@ -327,0 +290,0 @@ repo: name,

lib/adapters/github.test.js

@@ -15,3 +15,5 @@ "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-const github_1 = __importDefault(require("./github"));
+const github_1 = __importDefault(require("../services/github"));
+const github_2 = __importDefault(require("./github"));
+jest.mock('../services/github');
 const mockMigrationContext = () => ({
@@ -28,11 +30,15 @@ migration: {
         it('recognizes two repos as equal', () => {
+            const mocktokit = {};
             const repo1 = { owner: 'NerdWallet', name: 'shepherd' };
             const repo2 = { owner: 'NerdWallet', name: 'shepherd' };
-            const adapter = new github_1.default(mockMigrationContext(), {});
+            const service = new github_1.default(mocktokit);
+            const adapter = new github_2.default(mockMigrationContext(), service);
             expect(adapter.reposEqual(repo1, repo2)).toBe(true);
         });
         it('recognizes two repos as equal if one is missing a default branch', () => {
+            const mocktokit = {};
             const repo1 = { owner: 'NerdWallet', name: 'shepherd', defaultBranch: 'master' };
             const repo2 = { owner: 'NerdWallet', name: 'shepherd' };
-            const adapter = new github_1.default(mockMigrationContext(), {});
+            const service = new github_1.default(mocktokit);
+            const adapter = new github_2.default(mockMigrationContext(), service);
             expect(adapter.reposEqual(repo1, repo2)).toBe(true);
@@ -42,43 +48,32 @@ });
     describe('getCandidateRepos', () => {
-        it('validates search_type option if provided', () => __awaiter(void 0, void 0, void 0, function* () {
-            const mocktokit = {
-                repos: {
-                    get: jest.fn().mockReturnValue({
-                        data: {
-                            default_branch: 'develop',
-                        },
-                    }),
-                },
-                search: {}
+        it('validates search_query is not specified with org search', () => {
+            const mocktokit = {};
+            const migrationCtx = mockMigrationContext();
+            migrationCtx.migration.spec.adapter = {
+                org: 'testOrg',
+                type: 'github',
+                search_query: 'topics:test'
             };
+            const service = new github_1.default(mocktokit);
+            const adapter = new github_2.default(migrationCtx, service);
+            return expect(adapter.getCandidateRepos())
+                .rejects
+                .toThrow('Cannot use both \"org\" and \"search_query\" in GitHub adapter. Pick one.');
+        });
+        it('performs org search if specified and returns expected result', () => __awaiter(void 0, void 0, void 0, function* () {
+            const mocktokit = {};
             const migrationCtx = mockMigrationContext();
             migrationCtx.migration.spec.adapter = {
                 type: 'github',
-                search_type: 'invalid_search_type'
+                org: 'testOrg'
             };
-            const adapter = new github_1.default(migrationCtx, mocktokit);
-            try {
-                yield adapter.getCandidateRepos();
-            }
-            catch (e) {
-                expect(e.message).toContain(`"search_type" must be one of the following:`);
-            }
+            const service = new github_1.default(mocktokit);
+            service.getActiveReposForOrg.mockResolvedValue(['testOrg/test-repo']);
+            const adapter = new github_2.default(migrationCtx, service);
+            const result = yield adapter.getCandidateRepos();
+            expect(service.getActiveReposForOrg).toBeCalledWith({ org: 'testOrg' });
+            expect(result).toStrictEqual([{ owner: 'testOrg', name: 'test-repo' }]);
         }));
         it(`performs repository search and returns expected result if 'respositories' is specified for search_type`, () => __awaiter(void 0, void 0, void 0, function* () {
-            const mocktokit = {
-                paginate: jest
-                    .fn()
-                    .mockResolvedValue([{
-                        full_name: 'repoownername/test-repo'
-                    }]),
-                search: {
-                    repos: jest.fn().mockReturnValue({
-                        data: {
-                            items: [{
-                                    full_name: 'repoownername/test-repo'
-                                }]
-                        }
-                    })
-                }
-            };
+            const mocktokit = {};
             const migrationCtx = mockMigrationContext();
@@ -90,27 +85,14 @@ migrationCtx.migration.spec.adapter = {
             };
-            const adapter = new github_1.default(migrationCtx, mocktokit);
+            const service = new github_1.default(mocktokit);
+            service.getActiveReposForSearchTypeAndQuery.mockResolvedValue(['repoownername/test-repo']);
+            const adapter = new github_2.default(migrationCtx, service);
             const result = yield adapter.getCandidateRepos();
-            expect(mocktokit.paginate.mock.calls.length).toBe(1);
-            expect(mocktokit.paginate.mock.calls[0][1]).toStrictEqual({ q: 'topics:test' });
+            expect(service.getActiveReposForSearchTypeAndQuery).toBeCalledWith({
+                search_type: 'repositories',
+                search_query: 'topics:test'
+            });
             expect(result).toStrictEqual([{ owner: 'repoownername', name: 'test-repo' }]);
         }));
         it(`performs code search and returns expected result if search_type is 'code' or is not provided`, () => __awaiter(void 0, void 0, void 0, function* () {
-            const mocktokit = {
-                paginate: jest
-                    .fn()
-                    .mockResolvedValue([{
-                        repository: {
-                            full_name: 'repoownername/test-repo'
-                        }
-                    }]),
-                search: {
-                    code: jest.fn().mockReturnValue({
-                        data: {
-                            items: [{
-                                    full_name: 'repoownername/test-repo'
-                                }]
-                        }
-                    })
-                }
-            };
+            const mocktokit = {};
             const migrationCtx = mockMigrationContext();
@@ -127,4 +109,6 @@ migrationCtx.migration.spec.adapter = {
             };
-            const adapterWithSearchType = new github_1.default(migrationCtx, mocktokit);
-            const adapterWithoutSearchType = new github_1.default(migrationCtxWithoutSearchType, mocktokit);
+            const service = new github_1.default(mocktokit);
+            service.getActiveReposForSearchTypeAndQuery.mockResolvedValue(['repoownername/test-repo']);
+            const adapterWithSearchType = new github_2.default(migrationCtx, service);
+            const adapterWithoutSearchType = new github_2.default(migrationCtxWithoutSearchType, service);
             const getCandidateRepos = [
@@ -135,5 +119,7 @@ adapterWithSearchType.getCandidateRepos(),
             const results = yield Promise.all(getCandidateRepos);
-            expect(mocktokit.paginate.mock.calls.length).toBe(2);
-            expect(mocktokit.paginate.mock.calls[0][1]).toStrictEqual({ q: 'path:/ filename:package.json in:path' });
-            expect(mocktokit.paginate.mock.calls[1][1]).toStrictEqual({ q: 'path:/ filename:package.json in:path' });
+            expect(service.getActiveReposForSearchTypeAndQuery).toBeCalledTimes(2);
+            expect(service.getActiveReposForSearchTypeAndQuery).toBeCalledWith({
+                search_type: 'code',
+                search_query: 'path:/ filename:package.json in:path'
+            });
             expect(results[0]).toStrictEqual([{ owner: 'repoownername', name: 'test-repo' }]);
@@ -143,14 +129,20 @@ expect(results[1]).toStrictEqual([{ owner: 'repoownername', name: 'test-repo' }]);
     });
+    describe('parseRepo', () => {
+        it('throws if owner or name not found in repo string', () => {
+            const mocktokit = {};
+            const migrationCtx = mockMigrationContext();
+            const service = new github_1.default(mocktokit);
+            const adapter = new github_2.default(migrationCtx, service);
+            const calledWithoutName = adapter.parseRepo.bind(null, 'ownerbutnoname/');
+            const calledWithoutOwner = adapter.parseRepo.bind(null, '/namebutnoowner');
+            const calledWithNeither = adapter.parseRepo.bind(null, '');
+            expect(calledWithoutName).toThrow('Could not parse repo "ownerbutnoname/"');
+            expect(calledWithoutOwner).toThrow('Could not parse repo "/namebutnoowner"');
+            expect(calledWithNeither).toThrow('Could not parse repo ""');
+        });
+    });
     describe('mapRepoAfterCheckout', () => {
         it('saves the default branch', () => __awaiter(void 0, void 0, void 0, function* () {
-            const mocktokit = {
-                repos: {
-                    get: jest.fn().mockReturnValue({
-                        data: {
-                            default_branch: 'develop',
-                        },
-                    }),
-                },
-            };
-            const adapter = new github_1.default(mockMigrationContext(), mocktokit);
+            const mocktokit = {};
+            const service = new github_1.default(mocktokit);
             const repo = {
@@ -160,3 +152,7 @@ owner: 'NerdWallet',
             };
+            service.getDefaultBranchForRepo.mockResolvedValue('develop');
+            const adapter = new github_2.default(mockMigrationContext(), service);
             const mappedRepo = yield adapter.mapRepoAfterCheckout(repo);
+            expect(service.getDefaultBranchForRepo).toBeCalledTimes(1);
+            expect(service.getDefaultBranchForRepo).toBeCalledWith({ owner: repo.owner, repo: repo.name });
             expect(mappedRepo).toEqual(Object.assign(Object.assign({}, repo), { defaultBranch: 'develop' }));
@@ -166,16 +162,2 @@ }));
     describe('prRepo', () => {
-        const mockPrOctokit = (existingPr) => ({
-            pulls: {
-                list: jest.fn().mockReturnValue(existingPr),
-                create: jest.fn(),
-                update: jest.fn(),
-            },
-            repos: {
-                get: jest.fn().mockReturnValue({
-                    data: {
-                        default_branch: 'master',
-                    },
-                }),
-            },
-        });
         const REPO = {
@@ -187,11 +169,15 @@ owner: 'NerdWallet',
         it('creates a new PR if one does not exist', () => __awaiter(void 0, void 0, void 0, function* () {
-            const octokit = mockPrOctokit({ data: [] });
-            const adapter = new github_1.default(mockMigrationContext(), octokit);
+            const octokit = {};
+            const service = new github_1.default(octokit);
+            service.listPullRequests.mockResolvedValue([]);
+            const adapter = new github_2.default(mockMigrationContext(), service);
             yield adapter.createPullRequest(REPO, 'Test PR message');
-            const listMock = octokit.pulls.list;
-            const createMock = octokit.pulls.create;
-            expect(listMock).toBeCalled();
-            expect(createMock).toBeCalledWith({
+            expect(service.listPullRequests).toBeCalledWith({
                 owner: 'NerdWallet',
                 repo: 'shepherd',
+                head: 'NerdWallet:test-migration',
+            });
+            expect(service.createPullRequest).toBeCalledWith({
+                owner: 'NerdWallet',
+                repo: 'shepherd',
                 head: 'test-migration',
@@ -204,30 +190,28 @@ base: 'master',
         it('updates a PR if one exists and is open', () => __awaiter(void 0, void 0, void 0, function* () {
-            const octokit = mockPrOctokit({
-                data: [{
-                        number: 1234,
-                        state: 'open',
-                    }],
-            });
-            const adapter = new github_1.default(mockMigrationContext(), octokit);
+            const octokit = {};
+            const service = new github_1.default(octokit);
+            service.listPullRequests.mockResolvedValue([{
+                    number: 1234,
+                    state: 'open',
+                }]);
+            const adapter = new github_2.default(mockMigrationContext(), service);
             yield adapter.createPullRequest(REPO, 'Test PR message, part 2');
-            const updateMock = octokit.pulls.update;
-            expect(updateMock).toBeCalledWith({
+            expect(service.updatePullRequest).toBeCalledWith({
                 owner: 'NerdWallet',
                 repo: 'shepherd',
-                pull_number: 1234,
                 title: 'Test migration',
                 body: 'Test PR message, part 2',
+                pull_number: 1234
             });
         }));
         it('does not update a closed PR', () => __awaiter(void 0, void 0, void 0, function* () {
-            const octokit = mockPrOctokit({
-                data: [{
-                        number: 1234,
-                        state: 'closed',
-                    }],
-            });
-            const adapter = new github_1.default(mockMigrationContext(), octokit);
+            const octokit = {};
+            const service = new github_1.default(octokit);
+            service.listPullRequests.mockResolvedValue([{
+                    number: 1234,
+                    state: 'closed',
+                }]);
+            const adapter = new github_2.default(mockMigrationContext(), service);
             yield expect(adapter.createPullRequest(REPO, 'Test PR message, part 2')).rejects.toThrow();
-            const updateMock = octokit.pulls.update;
-            expect(updateMock).not.toBeCalled();
+            expect(service.updatePullRequest).not.toBeCalled();
         }));
@@ -234,0 +218,0 @@ });

lib/adapters/index.js

@@ -8,6 +8,9 @@ "use strict";
 const github_1 = __importDefault(require("./github"));
+const github_2 = __importDefault(require("../services/github"));
 function adapterForName(name, context) {
     switch (name) {
-        case 'github':
-            return new github_1.default(context);
+        case 'github': {
+            const githubService = new github_2.default();
+            return new github_1.default(context, githubService);
+        }
         default:
@@ -14,0 +17,0 @@ throw new Error(`Unknown adapter: ${name}`);

package.json

 {
   "name": "@nerdwallet/shepherd",
-  "version": "1.13.1",
+  "version": "1.14.0",
   "description": "A utility for applying code changes across many repositories",
@@ -49,3 +49,3 @@ "keywords": [
   "dependencies": {
-    "@octokit/rest": "^18.0.12",
+    "@octokit/rest": "^18.1.0",
     "@types/js-yaml": "^3.12.6",
@@ -55,4 +55,4 @@ "chalk": "^4.1.0",
     "commander": "^6.2.1",
-    "fs-extra": "^9.0.1",
-    "joi": "^17.3.0",
+    "fs-extra": "^9.1.0",
+    "joi": "^17.4.0",
     "js-yaml": "^3.14.1",
@@ -62,9 +62,9 @@ "lodash": "^4.17.19",
     "netrc": "^0.1.4",
-    "ora": "^5.0.0",
+    "ora": "^5.3.0",
     "preferences": "^2.0.2",
-    "simple-git": "^2.31.0"
+    "simple-git": "^2.34.2"
   },
   "devDependencies": {
-    "@octokit/plugin-rest-endpoint-methods": "^4.4.3",
-    "@octokit/types": "^6.2.1",
+    "@octokit/plugin-rest-endpoint-methods": "^4.10.2",
+    "@octokit/types": "^6.8.3",
     "@semantic-release/changelog": "^5.0.1",
@@ -76,12 +76,12 @@ "@semantic-release/git": "^9.0.0",
     "@types/log-symbols": "^2.0.0",
-    "@types/node": "^14.14.20",
-    "@typescript-eslint/eslint-plugin": "^4.12.0",
-    "@typescript-eslint/parser": "^4.12.0",
-    "eslint": "^7.5.0",
+    "@types/node": "^14.14.25",
+    "@typescript-eslint/eslint-plugin": "^4.15.0",
+    "@typescript-eslint/parser": "^4.15.0",
+    "eslint": "^7.19.0",
     "eslint-plugin-import": "^2.22.0",
-    "eslint-plugin-prefer-arrow": "^1.2.1",
+    "eslint-plugin-prefer-arrow": "^1.2.3",
     "jest": "^26.6.3",
-    "semantic-release": "^17.1.1",
-    "ts-jest": "^26.1.3",
-    "typescript": "^3.9.7"
+    "semantic-release": "^17.3.8",
+    "ts-jest": "^26.5.1",
+    "typescript": "^3.9.9"
   },
@@ -88,0 +88,0 @@ "publishConfig": {

No alert changes

Improved metrics

Total package byte prevSize

217276

increased by15.84%

Number of package files

94

increased by6.82%

Lines of code

2524

increased by17.89%

Dependency changes

• Updated@octokit/rest@^18.1.0

• Updatedfs-extra@^9.1.0

• Updatedjoi@^17.4.0

• Updatedora@^5.3.0

• Updatedsimple-git@^2.34.2