From 7f6265f61418ef66dca38ea730d4b7e4afdc6bcd Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 15 Nov 2019 10:28:39 +0100 Subject: [PATCH 1/6] feat: Finish implementing project creation on backend (#21) This change finishes the implementation of creating a project on the backend. --- .vscode/launch.json | 15 + .vscode/settings.json | 3 +- .vscode/tasks.json | 8 + package-lock.json | 67 +- package.json | 8 +- src/app.module.ts | 4 +- src/auth/user/user.controller.ts | 14 +- .../backend-project.service.spec.ts | 142 +++-- .../backend-project.service.ts | 31 +- src/config/config.service.ts | 7 + src/git/git.service.spec.ts | 583 ++++++++++++------ src/git/git.service.ts | 67 +- src/github/github.service.spec.ts | 142 ++++- src/github/github.service.ts | 86 ++- src/interfaces/git-hub-project.interface.ts | 5 + src/projects/projects.controller.spec.ts | 45 +- src/projects/projects.controller.ts | 48 +- src/utils/test-utils.ts | 22 + test.env | 1 + test/jest-e2e.json | 4 +- test/projects.e2e-spec.ts | 108 ++++ 21 files changed, 1068 insertions(+), 342 deletions(-) create mode 100644 src/interfaces/git-hub-project.interface.ts create mode 100644 src/utils/test-utils.ts create mode 100644 test/projects.e2e-spec.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index a1fe096d9..e41b933fd 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -18,6 +18,21 @@ "name": "Launch Chrome (Frontend)", "url": "http://localhost:4200", "webRoot": "${workspaceFolder}" + }, + { + "type": "node", + "request": "launch", + "name": "Debug e2e tests", + "program": "${workspaceRoot}/node_modules/.bin/jest", + "args": ["--runInBand", "--config", "./test/jest-e2e.json", "--verbose"], + "console": "integratedTerminal", + "env": { + "NODE_ENV": "test", + "TEST_GITHUB_TOKEN": "Insert your personal access token from GitHub", + "TEST_GITHUB_USER": "Insert your GitHub account name for testing" + }, + "sourceMaps": true, + "runtimeArgs": ["-r", "tsconfig-paths/register", "-r", "ts-node/register"] } ] } diff --git a/.vscode/settings.json b/.vscode/settings.json index ebe736cac..493c2f419 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,5 +5,6 @@ "nestjs", "prestart" ], -"files.autoSave": "onFocusChange" +"files.autoSave": "onFocusChange", +"debug.node.autoAttach": "off" } diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 10a3793ee..844011d95 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -143,6 +143,14 @@ "problemMatcher": [ "$tslint5" ] + }, + { + "type": "npm", + "script": "test:e2e:watch", + "problemMatcher": [ + "$awesomets-lint" + ], + "group": "build" } ] } diff --git a/package-lock.json b/package-lock.json index 226537c8b..b3ceb2898 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1440,6 +1440,12 @@ } } }, + "base-64": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/base-64/-/base-64-0.1.0.tgz", + "integrity": "sha1-eAqZyE59YAJgNhURxId2E78k9rs=", + "dev": true + }, "bcrypt-pbkdf": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.2.tgz", @@ -6054,12 +6060,6 @@ "to-regex": "^3.0.2" } }, - "mime": { - "version": "1.4.1", - "resolved": "https://registry.npmjs.org/mime/-/mime-1.4.1.tgz", - "integrity": "sha512-KI1+qOZu5DcW6wayYHSzR/tXKCDC5Om4s1z2QJjDULzLcmf3DvzS7oluY4HCTrc+9FiKmWUgeNLg7W3uIQvxtQ==", - "dev": true - }, "mime-db": { "version": "1.38.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.38.0.tgz", @@ -7929,22 +7929,14 @@ "integrity": "sha1-PFMZQukIwml8DsNEhYwobHygpgo=", "dev": true }, - "superagent": { - "version": "3.8.3", - "resolved": "https://registry.npmjs.org/superagent/-/superagent-3.8.3.tgz", - "integrity": "sha512-GLQtLMCoEIK4eDv6OGtkOoSMt3D+oq0y3dsxMuYuDvaNUvuT8eFBuLmfR0iYYzHC1e8hpzC6ZsxbuP6DIalMFA==", + "supertest": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/supertest/-/supertest-4.0.2.tgz", + "integrity": "sha512-1BAbvrOZsGA3YTCWqbmh14L0YEq0EGICX/nBnfkfVJn7SrxQV1I3pMYjSzG9y/7ZU2V9dWqyqk2POwxlb09duQ==", "dev": true, "requires": { - "component-emitter": "^1.2.0", - "cookiejar": "^2.1.0", - "debug": "^3.1.0", - "extend": "^3.0.0", - "form-data": "^2.3.1", - "formidable": "^1.2.0", - "methods": "^1.1.1", - "mime": "^1.4.1", - "qs": "^6.5.1", - "readable-stream": "^2.3.5" + "methods": "^1.1.2", + "superagent": "^3.8.3" }, "dependencies": { "isarray": { @@ -7953,6 +7945,12 @@ "integrity": "sha1-u5NdSFgsuhaMBoNJV6VKPgcSTxE=", "dev": true }, + "mime": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/mime/-/mime-1.6.0.tgz", + "integrity": "sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg==", + "dev": true + }, "readable-stream": { "version": "2.3.6", "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", @@ -7976,17 +7974,34 @@ "requires": { "safe-buffer": "~5.1.0" } + }, + "superagent": { + "version": "3.8.3", + "resolved": "https://registry.npmjs.org/superagent/-/superagent-3.8.3.tgz", + "integrity": "sha512-GLQtLMCoEIK4eDv6OGtkOoSMt3D+oq0y3dsxMuYuDvaNUvuT8eFBuLmfR0iYYzHC1e8hpzC6ZsxbuP6DIalMFA==", + "dev": true, + "requires": { + "component-emitter": "^1.2.0", + "cookiejar": "^2.1.0", + "debug": "^3.1.0", + "extend": "^3.0.0", + "form-data": "^2.3.1", + "formidable": "^1.2.0", + "methods": "^1.1.1", + "mime": "^1.4.1", + "qs": "^6.5.1", + "readable-stream": "^2.3.5" + } } } }, - "supertest": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/supertest/-/supertest-4.0.2.tgz", - "integrity": "sha512-1BAbvrOZsGA3YTCWqbmh14L0YEq0EGICX/nBnfkfVJn7SrxQV1I3pMYjSzG9y/7ZU2V9dWqyqk2POwxlb09duQ==", + "supertest-session": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/supertest-session/-/supertest-session-4.0.0.tgz", + "integrity": "sha512-9d7KAL+K9hnnicov7USv/Nu1tSl40qSrOsB8zZHOEtfEzHaAoID6tbl1NeCVUg7SJreJMlNn+KJ88V7FW8gD6Q==", "dev": true, "requires": { - "methods": "^1.1.2", - "superagent": "^3.8.3" + "object-assign": "^4.0.1" } }, "supports-color": { diff --git a/package.json b/package.json index 1c2b126c9..1b206ac5e 100644 --- a/package.json +++ b/package.json @@ -16,10 +16,12 @@ "start:prod": "cross-env NODE_ENV=production node dist/main.js", "lint": "tslint -p tsconfig.lint.json -c tslint.json", "test": "cross-env NODE_ENV=test jest", - "test:watch": "cross-env NODE_ENV=test jest --watch", + "test:watch": "cross-env NODE_ENV=test jest --watch --verbose", "test:cov": "cross-env NODE_ENV=test jest --coverage", "test:debug": "cross-env NODE_ENV=test node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", - "test:e2e": "cross-env NODE_ENV=test jest --config ./test/jest-e2e.json" + "test:e2e": "cross-env NODE_ENV=test jest --verbose --config ./test/jest-e2e.json", + "test:e2e:watch": "cross-env NODE_ENV=test jest --verbose --config ./test/jest-e2e.json --watch --onlyChanged", + "test:e2e:debug": "cross-env NODE_ENV=test node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand --config ./test/jest-e2e.json" }, "dependencies": { "@hapi/joi": "^16.1.4", @@ -58,6 +60,7 @@ "@types/node": "^12.7.5", "@types/supertest": "^2.0.8", "axios-mock-adapter": "^1.17.0", + "base-64": "^0.1.0", "cross-env": "^6.0.0", "gulp": "^4.0.2", "gulp-run-command": "0.0.9", @@ -65,6 +68,7 @@ "nodemon": "^1.19.2", "prettier": "^1.18.2", "supertest": "^4.0.2", + "supertest-session": "^4.0.0", "ts-jest": "^24.1.0", "ts-mockito": "^2.5.0", "ts-node": "^8.4.1", diff --git a/src/app.module.ts b/src/app.module.ts index bab42485b..e77e533c0 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -72,12 +72,14 @@ export class AppModule implements NestModule { if (process.env.NODE_ENV !== 'test') { // It seems that MemoryStore has a bug (https://github.com/roccomuso/memorystore/issues/11) - // so that it doesn't properly shut down when unit tests end + // so that it doesn't properly shut down when unit tests end. On the other hand it is safe + // to be used in production. sessionOpts.store = new MemoryStore({ checkPeriod: 24 * 60 * 60 * 1000, // 24 hrs * 60 min * 60 sec * 1000 ms }); } else { cookie.secure = false; + sessionOpts.store = new expressSession.MemoryStore(); } ExpressSessionMiddleware.configure(sessionOpts); diff --git a/src/auth/user/user.controller.ts b/src/auth/user/user.controller.ts index 7d9c17b60..f03c3636b 100644 --- a/src/auth/user/user.controller.ts +++ b/src/auth/user/user.controller.ts @@ -8,6 +8,7 @@ import { ValidationPipe, Delete, Headers, + Param, } from '@nestjs/common'; import { Observable } from 'rxjs'; @@ -36,7 +37,7 @@ export class UserController { ): Promise { return callWithErrorHandling( () => this.githubService.getAccessToken(loginDto.code, loginDto.state), - (response) => { + response => { const ghToken = response.data as GitHubAccessToken; return new AccessTokenDto(ghToken.access_token); }, @@ -56,7 +57,7 @@ export class UserController { ): Promise { return callWithErrorHandling( () => this.githubService.getUserInformation(token), - (response) => { + response => { const user = response.data as GitHubUser; session.login = user.login; return new GitHubUserDto(user); @@ -65,4 +66,13 @@ export class UserController { ); } + // Method used in tests to set the username in the session + @Get('user/test/:username') + public async getUser( + @Session() session: any, + @Headers('authorization') token: string, + @Param() params: any, + ): Promise { + session.login = params.username; + } } diff --git a/src/backend-project/backend-project.service.spec.ts b/src/backend-project/backend-project.service.spec.ts index f52de4910..4cfb1b059 100644 --- a/src/backend-project/backend-project.service.spec.ts +++ b/src/backend-project/backend-project.service.spec.ts @@ -49,7 +49,7 @@ describe('BackendProjectService', () => { ]); await gitService.commit(repoDir, 'Initial commit'); - remoteRepo = await gitService.clone(repoDir, path.join(tmpDir, 'remoteTestRepo'), ['--bare']); + remoteRepo = await gitService.clone(repoDir, path.join(tmpDir, 'remoteTestRepo'), true); }); afterEach(() => { @@ -67,51 +67,113 @@ describe('BackendProjectService', () => { ); }); - it('does clone remote project if it does not exist locally', async () => { - expect.assertions(2); - - // Setup - const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); - - // Execute - const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'master'); - - // Verify - expect(cloneDir).toEqual(expectedCloneDir); - expect(fs.existsSync(path.join(expectedCloneDir, '.git'))).toBeTruthy(); + describe('cloneOrUpdateProject', () => { + it('does clone remote project if it does not exist locally', async () => { + // Setup + expect.assertions(3); + const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); + + // Execute + const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'master', 'jdoe'); + + // Verify + expect(cloneDir).toEqual(expectedCloneDir); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBe(true); + expect(await gitService.currentBranch(cloneDir)).toEqual('master'); + }); + + it('updates local project if it does exist locally', async () => { + // Setup + expect.assertions(4); + const setupRepo = await gitService.clone(remoteRepo, path.join(config.workDirectory, 'setuprepo')); + const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); + await gitService.clone(remoteRepo, expectedCloneDir); + const secondCommit = await gitService.commit(setupRepo, 'empty commit', { '--allow-empty': null }); + await gitService.push(setupRepo, 'origin', 'master'); + + // Execute + const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'master', 'jdoe'); + + // Verify + expect(cloneDir).toEqual(expectedCloneDir); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBe(true); + expect(await gitService.currentBranch(cloneDir)).toEqual('master'); + const log = await gitService.log(cloneDir); + const expected = new RegExp(`^${secondCommit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); + + it('works with different local branch name if it does not exist locally', async () => { + // Setup + expect.assertions(3); + const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); + + // Execute + const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'local-branch', 'jdoe', 'master'); + + // Verify + expect(cloneDir).toEqual(expectedCloneDir); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBe(true); + expect(await gitService.currentBranch(cloneDir)).toEqual('local-branch'); + }); + + it('works with different local branch name if it does exist locally', async () => { + // Setup + expect.assertions(4); + const setupRepo = await gitService.clone(remoteRepo, path.join(config.workDirectory, 'setuprepo')); + const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); + await gitService.clone(remoteRepo, expectedCloneDir); + const secondCommit = await gitService.commit(setupRepo, 'empty commit', { '--allow-empty': null }); + await gitService.push(setupRepo, 'origin', 'master'); + + // Execute + const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'local-branch', 'jdoe', 'master'); + + // Verify + expect(cloneDir).toEqual(expectedCloneDir); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBe(true); + expect(await gitService.currentBranch(cloneDir)).toEqual('local-branch'); + const log = await gitService.log(cloneDir); + const expected = new RegExp(`^${secondCommit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); }); - it('updates project if it does exist locally', async () => { - expect.assertions(3); - - // Setup - const setupRepo = await gitService.clone(remoteRepo, path.join(config.workDirectory, 'setuprepo')); - const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); - await gitService.clone(remoteRepo, expectedCloneDir); - const secondCommit = await gitService.commit(setupRepo, 'empty commit', { '--allow-empty': null }); - await gitService.push(setupRepo, 'origin', 'master'); - - // Execute - const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'master'); - - // Verify - expect(cloneDir).toEqual(expectedCloneDir); - expect(fs.existsSync(path.join(expectedCloneDir, '.git'))).toBeTruthy(); - const log = await gitService.log(cloneDir); - const expected = new RegExp(`^${secondCommit.commit}.+`); - expect(log.latest.hash).toMatch(expected); + describe('clone', () => { + it('updates project if it does exist locally', async () => { + expect.assertions(3); + + // Setup + const setupRepo = await gitService.clone(remoteRepo, path.join(config.workDirectory, 'setuprepo')); + const expectedCloneDir = path.join(config.workDirectory, 'jdoe-testproject'); + await gitService.clone(remoteRepo, expectedCloneDir); + const secondCommit = await gitService.commit(setupRepo, 'empty commit', { '--allow-empty': null }); + await gitService.push(setupRepo, 'origin', 'master'); + + // Execute + const cloneDir = await sut.cloneOrUpdateProject(remoteRepo, expectedCloneDir, 'master', 'jdoe'); + + // Verify + expect(cloneDir).toEqual(expectedCloneDir); + expect(fs.existsSync(path.join(expectedCloneDir, '.git'))).toBe(true); + const log = await gitService.log(cloneDir); + const expected = new RegExp(`^${secondCommit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); }); - it('Returns expected branch name', () => { - expect(sut.branchName).toEqual('master'); - }); + describe('Helper methods', () => { + it('Returns expected branch name', () => { + expect(sut.branchName).toEqual('master'); + }); - it('Returns expected name of keyboards repo', () => { - expect(sut.keyboardsRepoName).toEqual('keyboards'); - }); + it('Returns expected name of keyboards repo', () => { + expect(sut.keyboardsRepoName).toEqual('keyboards'); + }); - it('Returns expected path of the keyboards repo', () => { - expect(sut.localKeyboardsRepo).toEqual(path.join(workDir, 'keyboards')); + it('Returns expected path of the keyboards repo', () => { + expect(sut.localKeyboardsRepo).toEqual(path.join(workDir, 'keyboards')); + }); }); }); diff --git a/src/backend-project/backend-project.service.ts b/src/backend-project/backend-project.service.ts index 9545acc96..4b7217015 100644 --- a/src/backend-project/backend-project.service.ts +++ b/src/backend-project/backend-project.service.ts @@ -16,18 +16,39 @@ export class BackendProjectService { public async cloneOrUpdateProject( remoteRepo: string, localRepo: string, - branch: string, + localBranch: string, + remoteName: string, + remoteBranch: string = localBranch, ): Promise { if (fs.existsSync(localRepo)) { - return this.gitService.pull(localRepo, remoteRepo, branch).then(() => localRepo); + if (!await this.gitService.hasRemote(localRepo, remoteName)) { + await this.gitService.addRemote(localRepo, remoteName, remoteRepo); + await this.gitService.fetch(localRepo, remoteName, remoteBranch); + } + const branchExists = await this.gitService.isBranch(localRepo, localBranch); + if (!branchExists) { + await this.checkoutBranch(localRepo, localBranch, `${remoteName}/${remoteBranch}`); + // this.gitService.push(localRepo, ) + } + return this.gitService.pull(localRepo, remoteName, remoteBranch).then(() => localRepo); } else { - return this.gitService.clone(remoteRepo, localRepo).then(async () => { - await this.gitService.checkoutBranch(localRepo, branch); - return localRepo; + return this.gitService.clone(remoteRepo, localRepo, false, remoteName).then(async () => { + return this.checkoutBranch(localRepo, localBranch, `${remoteName}/${remoteBranch}`); }); } } + private async checkoutBranch( + localRepo: string, + localBranch: string, + remoteBranch?: string, + ): Promise { + if (remoteBranch) { + return this.gitService.checkoutBranch(localRepo, localBranch, remoteBranch).then(() => localRepo); + } + return this.gitService.checkoutBranch(localRepo, localBranch).then(() => localRepo); + } + public get branchName() { return 'master'; } diff --git a/src/config/config.service.ts b/src/config/config.service.ts index ab2139e49..c304060ef 100644 --- a/src/config/config.service.ts +++ b/src/config/config.service.ts @@ -45,6 +45,8 @@ export class ConfigService { const expiresDays = process.env.EXPIRES_DAYS != null ? process.env.EXPIRES_DAYS : 1; const cookieMaxAge = process.env.COOKIE_MAX_AGE != null ? process.env.COOKIE_MAX_AGE : 1; const workDirectory = process.env.WORKDIR != null ? process.env.WORKDIR : '/tmp'; + const upstreamGitHubUrl = process.env.UPSTREAM_GITHUB_URL != null + ? process.env.UPSTREAM_GITHUB_URL : 'https://github.com/keymanapp'; const envVarsSchema: Joi.ObjectSchema = Joi.object({ NODE_ENV: Joi.string() @@ -67,6 +69,7 @@ export class ConfigService { EXPIRES_DAYS: Joi.number().default(expiresDays), COOKIE_MAX_AGE: Joi.number().default(cookieMaxAge), WORKDIR: Joi.string().default(workDirectory), + UPSTREAM_GITHUB_URL: Joi.string().default(upstreamGitHubUrl), }); const { error, value: validatedEnvConfig } = envVarsSchema.validate(envConfig); @@ -111,4 +114,8 @@ export class ConfigService { public get workDirectory(): string { return this.envConfig.WORKDIR; } + + public get upstreamGitHubUrl(): string { + return this.envConfig.UPSTREAM_GITHUB_URL; + } } diff --git a/src/git/git.service.spec.ts b/src/git/git.service.spec.ts index 142f4fb14..f996cf17b 100644 --- a/src/git/git.service.spec.ts +++ b/src/git/git.service.spec.ts @@ -1,16 +1,18 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { CommitSummary } from 'simple-git/promise'; +import * as simplegit from 'simple-git/promise'; import { deleteFolderRecursive } from '../utils/delete-folder'; import { GitService } from './git.service'; import fs = require('fs'); import os = require('os'); import path = require('path'); -import { CommitSummary } from 'simple-git/promise'; describe('GitService', () => { let sut: GitService; let tmpDir: string; let module: TestingModule; + let git: simplegit.SimpleGit; beforeEach(async () => { module = await Test.createTestingModule({ @@ -20,6 +22,7 @@ describe('GitService', () => { const prefix = path.join(os.tmpdir(), 'gitrepotest-'); tmpDir = fs.mkdtempSync(prefix); sut = module.get(GitService); + git = simplegit(); jest.setTimeout(10000/*10s*/); }); @@ -55,251 +58,429 @@ describe('GitService', () => { expect(sut).toBeDefined(); }); - it('Can create git repo', async () => { - expect.assertions(2); - const expectedRepoDir = path.join(tmpDir, 'mytest'); - const dir = await sut.createRepo(expectedRepoDir); - expect(dir).toEqual(expectedRepoDir); - expect(fs.existsSync(path.join(expectedRepoDir, '.git'))).toBeTruthy(); - }); + describe('createRepo', () => { + it('Can create git repo', async () => { + expect.assertions(2); + const expectedRepoDir = path.join(tmpDir, 'mytest'); + const dir = await sut.createRepo(expectedRepoDir); + expect(dir).toEqual(expectedRepoDir); + expect(fs.existsSync(path.join(expectedRepoDir, '.git'))).toBeTruthy(); + }); - it('can add files and commit', async () => { - expect.assertions(2); + it('can add files and commit', async () => { + expect.assertions(2); - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - const commitSummary = await createInitialCommit(sut, repoDir); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + const commitSummary = await createInitialCommit(sut, repoDir); - expect(commitSummary.branch).toEqual('master'); - expect(commitSummary.commit).toBeTruthy(); + expect(commitSummary.branch).toEqual('master'); + expect(commitSummary.commit).toBeTruthy(); + }); }); - it('can determine latest commit', async () => { - expect.assertions(2); - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - const secondCommit = await addCommit(sut, repoDir, path.join(repoDir, 'somefile1.txt'), 'Additional text', 'second commit'); + describe('log', () => { + it('can determine latest commit', async () => { + // Setup + expect.assertions(2); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const secondCommit = await addCommit(sut, repoDir, path.join(repoDir, 'somefile1.txt'), 'Additional text', 'second commit'); + + // Execute + const log = await sut.log(repoDir); + + // Verify + const expected = new RegExp(`^${secondCommit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + expect(log.total).toEqual(1); + }); + }); - const log = await sut.log(repoDir); - const expected = new RegExp(`^${secondCommit.commit}.+`); - expect(log.latest.hash).toMatch(expected); - expect(log.total).toEqual(1); + describe('clone', () => { + it('fails clone if relative path', async () => { + expect.assertions(1); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + + // Execute/Verify + await expect(sut.clone(repoDir, 'clonedRepo')).rejects.toEqual(new Error('relative path')); + }); + + it('can clone the repo with correct path if localname contains full path', async () => { + expect.assertions(4); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + + // Execute + const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); + + // Verify + expect(cloneDir).toEqual(path.join(tmpDir, 'clonedRepo')); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBeTruthy(); + expect(fs.existsSync(path.join(cloneDir, 'somefile1.txt'))).toBeTruthy(); + expect(fs.existsSync(path.join(cloneDir, 'somefile2.txt'))).toBeTruthy(); + }); + + it('sets expected remote when cloning', async () => { + // Setup + expect.assertions(3); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + + // Execute + const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo'), false, 'foo'); + + // Verify + expect(cloneDir).toEqual(path.join(tmpDir, 'clonedRepo')); + expect(fs.existsSync(path.join(cloneDir, '.git'))).toBeTruthy(); + await git.cwd(cloneDir); + const remotes = await git.getRemotes(false); + expect(remotes[0].name).toEqual('foo'); + }); }); - it('fails clone if relative path', async () => { - expect.assertions(1); + describe('export and import', () => { + it('can export and import patch', async () => { + expect.assertions(2); - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); + // Setup 1 + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + const commit = await createInitialCommit(sut, repoDir); - // Execute/Verify - await expect(sut.clone(repoDir, 'clonedRepo')).rejects.toEqual(new Error('relative path')); - }); + // Execute 1 + const patch = await sut.export(repoDir, 'HEAD'); + + // Verify 1 + expect(patch).toEqual(path.join(repoDir, '0001-my-commit-message.patch')); - it('can clone the repo with correct path if localname contains full path', async () => { - expect.assertions(4); + // Setup 2 + const secondRepoDir = await sut.createRepo(path.join(tmpDir, 'secondRepo')); - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); + // Execute 2 + await sut.import(secondRepoDir, patch); - // Execute/Verify - const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); - expect(cloneDir).toEqual(path.join(tmpDir, 'clonedRepo')); - expect(fs.existsSync(path.join(cloneDir, '.git'))).toBeTruthy(); - expect(fs.existsSync(path.join(cloneDir, 'somefile1.txt'))).toBeTruthy(); - expect(fs.existsSync(path.join(cloneDir, 'somefile2.txt'))).toBeTruthy(); + // Verify 2 + const log = await sut.log(secondRepoDir); + expect(log.total).toEqual(1); + }); }); - it('can export and import patch', async () => { - expect.assertions(2); + describe('checkoutBranch', () => { + it('can create new branch', async () => { + expect.assertions(5); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + + // Execute + await sut.checkoutBranch(repoDir, 'test'); + await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Verify + const branchSummary = await sut.getBranches(repoDir); + expect(branchSummary.all).toContain('test'); + expect(branchSummary.current).toEqual('test'); + expect(branchSummary.branches.test.label).toEqual('commit on test branch'); + expect(branchSummary.branches.master.label).toEqual('my commit message'); + expect(branchSummary.branches.test.commit).not.toEqual(branchSummary.branches.master.commit); + }); + + it('can checkout existing branch with different local name', async () => { + expect.assertions(6); + + // Setup + const upstreamRepoDir = await sut.createRepo(path.join(tmpDir, 'upstreamRepo'), true); + const repoDir = await sut.clone(upstreamRepoDir, path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + await sut.push(repoDir, 'origin', 'master'); + + // Execute + await sut.checkoutBranch(repoDir, 'test', 'origin/master'); + + // Verify + await sut.checkoutBranch(repoDir, 'master'); + await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on master branch', + ); + await sut.push(repoDir, 'origin', 'master'); + await sut.checkoutBranch(repoDir, 'test'); + await sut.pull(repoDir); + + const branchSummary = await sut.getBranches(repoDir); + expect(branchSummary.all).toContain('test'); + expect(branchSummary.all).toContain('master'); + expect(branchSummary.current).toEqual('test'); + expect(branchSummary.branches.test.label).toEqual('commit on master branch'); + expect(branchSummary.branches.test.label).toEqual('commit on master branch'); + expect(branchSummary.branches.test.commit).toEqual(branchSummary.branches.master.commit); + }); + + it('trying to create an existing branch does not fail', async () => { + expect.assertions(5); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + await sut.checkoutBranch(repoDir, 'test'); + await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Execute + await sut.checkoutBranch(repoDir, 'test'); + + // Verify + const branchSummary = await sut.getBranches(repoDir); + expect(branchSummary.all).toContain('test'); + expect(branchSummary.current).toEqual('test'); + expect(branchSummary.branches.test.label).toEqual('commit on test branch'); + expect(branchSummary.branches.master.label).toEqual('my commit message'); + expect(branchSummary.branches.test.commit).not.toEqual( + branchSummary.branches.master.commit, + ); + }); + + it('can get current branch', async () => { + expect.assertions(2); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + await sut.checkoutBranch(repoDir, 'test'); + + // Execute/Verify + expect(await sut.currentBranch(repoDir)).toEqual('test'); + await sut.checkoutBranch(repoDir, 'master'); + expect(await sut.currentBranch(repoDir)).toEqual('master'); + }); + }); - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - const commit = await createInitialCommit(sut, repoDir); + describe('isBranch', () => { + it('can check that branch exists', async () => { + expect.assertions(1); - // Execute - const patch = await sut.export(repoDir, 'HEAD'); - expect(patch).toEqual(path.join(repoDir, '0001-my-commit-message.patch')); + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + await sut.checkoutBranch(repoDir, 'test'); - const secondRepoDir = await sut.createRepo(path.join(tmpDir, 'secondRepo')); - await sut.import(secondRepoDir, patch); + // Execute + const isBranch = await sut.isBranch(repoDir, 'test'); - // Verify - const log = await sut.log(secondRepoDir); - expect(log.total).toEqual(1); + // Verify + expect(isBranch).toBeTruthy(); + }); }); - it('can create new branch', async () => { - expect.assertions(5); - - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - - // Execute - await sut.checkoutBranch(repoDir, 'test'); - await addCommit( - sut, - repoDir, - path.join(repoDir, 'somefile1.txt'), - 'Additional text', - 'commit on test branch', - ); - - // Verify - const branchSummary = await sut.getBranches(repoDir); - expect(branchSummary.all).toContain('test'); - expect(branchSummary.current).toEqual('test'); - expect(branchSummary.branches.test.label).toEqual('commit on test branch'); - expect(branchSummary.branches.master.label).toEqual('my commit message'); - expect(branchSummary.branches.test.commit).not.toEqual(branchSummary.branches.master.commit); + describe('push', () => { + it('can push', async () => { + expect.assertions(1); + + // Setup + const bareRepoDir = await sut.createRepo(path.join(tmpDir, 'bareRepo'), true); + const clonedDir = await sut.clone(bareRepoDir, path.join(tmpDir, 'clonedRepo')); + await createInitialCommit(sut, clonedDir); + const commit = await addCommit( + sut, + clonedDir, + path.join(clonedDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Execute + await sut.push(clonedDir, 'origin', 'master'); + + // Verify + const log = await sut.log(bareRepoDir); + const expected = new RegExp(`^${commit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); }); - it('trying to create an existing branch does not fail', async () => { - expect.assertions(5); - - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - await sut.checkoutBranch(repoDir, 'test'); - await addCommit( - sut, - repoDir, - path.join(repoDir, 'somefile1.txt'), - 'Additional text', - 'commit on test branch', - ); - - // Execute - await sut.checkoutBranch(repoDir, 'test'); - - // Verify - const branchSummary = await sut.getBranches(repoDir); - expect(branchSummary.all).toContain('test'); - expect(branchSummary.current).toEqual('test'); - expect(branchSummary.branches.test.label).toEqual('commit on test branch'); - expect(branchSummary.branches.master.label).toEqual('my commit message'); - expect(branchSummary.branches.test.commit).not.toEqual( - branchSummary.branches.master.commit, - ); + describe('pull', () => { + it('can pull', async () => { + expect.assertions(1); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const clonedDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); + const commit = await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Execute (pull from mytest to clonedRepo) + await sut.pull(clonedDir, 'origin', 'master'); + + // Verify + const log = await sut.log(clonedDir); + const expected = new RegExp(`^${commit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); + + it('can pull from default', async () => { + expect.assertions(1); + + // Setup + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const clonedDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); + const commit = await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Execute (pull from mytest to clonedRepo) + await sut.pull(clonedDir); + + // Verify + const log = await sut.log(clonedDir); + const expected = new RegExp(`^${commit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); }); - it('can check that branch exists', async () => { - expect.assertions(1); + describe('fetch', () => { + it('can fetch from remote', async () => { + // Setup + expect.assertions(1); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const clonedDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo'), false, 'upstream'); + const commit = await addCommit( + sut, + repoDir, + path.join(repoDir, 'somefile1.txt'), + 'Additional text', + 'commit on test branch', + ); + + // Execute + await sut.fetch(clonedDir, 'upstream', 'master'); + + // Verify + await sut.checkoutBranch(clonedDir, 'foo', 'upstream/master'); + const log = await sut.log(clonedDir); + const expected = new RegExp(`^${commit.commit}.+`); + expect(log.latest.hash).toMatch(expected); + }); + }); - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - await sut.checkoutBranch(repoDir, 'test'); + describe('isGitRepo', () => { + it('isGitRepo returns false for normal directory', async () => { + expect.assertions(1); - // Execute - const isBranch = await sut.isBranch(repoDir, 'test'); + // Setup + const repoPath = path.join(tmpDir, 'normalDir'); + fs.mkdirSync(repoPath); - // Verify - expect(isBranch).toBeTruthy(); - }); + // Execute + const isRepo = await sut.isGitRepo(repoPath); - it('can get current branch', async () => { - expect.assertions(2); + // Verify + expect(isRepo).toEqual(false); + }); - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - await sut.checkoutBranch(repoDir, 'test'); + it('isGitRepo returns false for non-existing directory', async () => { + expect.assertions(1); - // Execute/Verify - expect(await sut.currentBranch(repoDir)).toEqual('test'); - await sut.checkoutBranch(repoDir, 'master'); - expect(await sut.currentBranch(repoDir)).toEqual('master'); - }); + // Setup + const repoPath = path.join(tmpDir, 'non-existing'); - it('can push', async () => { - expect.assertions(1); - - // Setup - const bareRepoDir = await sut.createRepo(path.join(tmpDir, 'bareRepo'), true); - const clonedDir = await sut.clone(bareRepoDir, path.join(tmpDir, 'clonedRepo')); - await createInitialCommit(sut, clonedDir); - const commit = await addCommit( - sut, - clonedDir, - path.join(clonedDir, 'somefile1.txt'), - 'Additional text', - 'commit on test branch', - ); - - // Execute - await sut.push(clonedDir, 'origin', 'master'); - - // Verify - const log = await sut.log(bareRepoDir); - const expected = new RegExp(`^${commit.commit}.+`); - expect(log.latest.hash).toMatch(expected); - }); + // Execute + const isRepo = await sut.isGitRepo(repoPath); - it('can pull', async () => { - expect.assertions(1); - - // Setup - const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); - await createInitialCommit(sut, repoDir); - const clonedDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); - const commit = await addCommit( - sut, - repoDir, - path.join(repoDir, 'somefile1.txt'), - 'Additional text', - 'commit on test branch', - ); - - // Execute (pull from mytest to clonedRepo) - await sut.pull(clonedDir, 'origin', 'master'); - - // Verify - const log = await sut.log(clonedDir); - const expected = new RegExp(`^${commit.commit}.+`); - expect(log.latest.hash).toMatch(expected); - }); + // Verify + expect(isRepo).toEqual(false); + }); - it('isGitRepo returns false for normal directory', async () => { - expect.assertions(1); + it('isGitRepo returns true for git directory', async () => { + expect.assertions(1); - // Setup - const repoPath = path.join(tmpDir, 'normalDir'); - fs.mkdirSync(repoPath); + // Setup + const repoPath = path.join(tmpDir, 'normalDir'); + await sut.createRepo(repoPath); - // Execute - const isRepo = await sut.isGitRepo(repoPath); + // Execute + const isRepo = await sut.isGitRepo(repoPath); - // Verify - expect(isRepo).toEqual(false); + // Verify + expect(isRepo).toEqual(true); + }); }); - it('isGitRepo returns false for non-existing directory', async () => { - expect.assertions(1); + describe('hasRemote', () => { - // Setup - const repoPath = path.join(tmpDir, 'non-existing'); + it('returns false if remote is not set', async () => { + // Setup + expect.assertions(1); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo'), false, 'foo'); - // Execute - const isRepo = await sut.isGitRepo(repoPath); + // Execute + const hasBar = await sut.hasRemote(cloneDir, 'bar'); - // Verify - expect(isRepo).toEqual(false); - }); + // Verify + expect(hasBar).toBe(false); + }); - it('isGitRepo returns true for git directory', async () => { - expect.assertions(1); + it('returns true if remote is set', async () => { + // Setup + expect.assertions(1); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo'), false, 'foo'); - // Setup - const repoPath = path.join(tmpDir, 'normalDir'); - await sut.createRepo(repoPath); + // Execute + const hasFoo = await sut.hasRemote(cloneDir, 'foo'); - // Execute - const isRepo = await sut.isGitRepo(repoPath); + // Verify + expect(hasFoo).toBe(true); + }); + }); - // Verify - expect(isRepo).toEqual(true); + describe('addRemote', () => { + it('can add a new remote', async () => { + // Setup + expect.assertions(2); + const repoDir = await sut.createRepo(path.join(tmpDir, 'mytest')); + await createInitialCommit(sut, repoDir); + const cloneDir = await sut.clone(repoDir, path.join(tmpDir, 'clonedRepo')); + + // Execute + await sut.addRemote(cloneDir, 'jdoe', 'https://url.to/remote/repo.git'); + + // Verify + expect(await sut.hasRemote(cloneDir, 'origin')).toBe(true); + expect(await sut.hasRemote(cloneDir, 'jdoe')).toBe(true); + }); }); }); diff --git a/src/git/git.service.ts b/src/git/git.service.ts index 08f58f7a0..a9c562cc7 100644 --- a/src/git/git.service.ts +++ b/src/git/git.service.ts @@ -36,7 +36,7 @@ export class GitService { repoPath: string, bare: boolean = false, ): Promise { - return new Promise((resolve) => { + return new Promise(resolve => { fs.mkdir(repoPath, async () => { await this.git.cwd(repoPath); await this.git.init(bare); @@ -61,19 +61,27 @@ export class GitService { } public async clone( - repoPath: string, + remoteUrl: string, localPath: string, - options: string[] = null, + bare: boolean = false, + remoteName?: string, ): Promise { if (!path.isAbsolute(localPath)) { throw new Error('relative path'); } - return this.git.clone(repoPath, localPath, options).then(async () => { - await this.git.cwd(localPath); - await this.addDefaultConfig(); - return localPath; - }); + const options = []; + if (bare) { + options.push('--bare'); + } + if (remoteName) { + options.push(`--origin=${remoteName}`); + } + + await this.git.clone(remoteUrl, localPath, options); + await this.git.cwd(localPath); + await this.addDefaultConfig(); + return localPath; } public async export(repoDir: string, commit: string): Promise { @@ -95,12 +103,20 @@ export class GitService { return this.git.log({ '-1': null }); } - public async checkoutBranch(repoDir: string, name: string): Promise { + public async checkoutBranch( + repoDir: string, + name: string, + trackingBranch: string = null, + ): Promise { await this.git.cwd(repoDir); if (await this.isBranch(repoDir, name)) { return this.git.checkout(name); } + if (trackingBranch) { + return this.git.checkoutBranch(name, trackingBranch); + } + return this.git.checkoutLocalBranch(name); } @@ -133,23 +149,26 @@ export class GitService { return this.git.push(remote, branch); } - public async fetch(repoDir: string, remote: string): Promise { + public async fetch(repoDir: string, remote: string, remoteBranch: string): Promise { await this.git.cwd(repoDir); - return this.git.fetch(remote); + return this.git.fetch(remote, remoteBranch); } public async pull( repoDir: string, - remote: string, - branch: string, + remote?: string, + branch?: string, ): Promise { await this.git.cwd(repoDir); - return this.git.pull(remote, branch); + if (remote) { + return this.git.pull(remote, branch); + } + return this.git.pull(); } public async isGitRepo(repoDir: string): Promise { - return new Promise((resolve) => { - fs.access(repoDir, fs.constants.R_OK, async (err) => { + return new Promise(resolve => { + fs.access(repoDir, fs.constants.R_OK, async err => { if (err) { resolve(false); } else { @@ -159,4 +178,20 @@ export class GitService { }); }); } + + public async hasRemote(repoDir: string, remoteName: string): Promise { + await this.git.cwd(repoDir); + const remotes = await this.git.getRemotes(false); + for (const remote of remotes) { + if (remote.name === remoteName) { + return true; + } + } + return false; + } + + public async addRemote(repoDir: string, remoteName: string, remoteRepo: string): Promise { + await this.git.cwd(repoDir); + return this.git.addRemote(remoteName, remoteRepo); + } } diff --git a/src/github/github.service.spec.ts b/src/github/github.service.spec.ts index d1f86b76b..b192e6a72 100644 --- a/src/github/github.service.spec.ts +++ b/src/github/github.service.spec.ts @@ -1,11 +1,12 @@ import { HttpService } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { AxiosResponse } from 'axios'; -import { of } from 'rxjs'; +import { of, throwError, Scheduler, VirtualTimeScheduler, pipe } from 'rxjs'; import { ConfigModule } from '../config/config.module'; import { GithubService } from './github.service'; import { TokenService } from '../token/token.service'; +import { tap } from 'rxjs/operators'; describe('GitHub Service', () => { const projectFromGitHub = { @@ -24,11 +25,27 @@ describe('GitHub Service', () => { size: 11195, default_branch: 'master', }; + const resultSuccess: AxiosResponse = { + data: 'Some text', + status: 200, + statusText: '', + headers: {}, + config: {}, + }; + const resultError: AxiosResponse = { + data: 'Repo does not exist', + status: 404, + statusText: '', + headers: {}, + config: {}, + }; + let sut: GithubService; let spyHttpService: HttpService; beforeEach(async () => { jest.setTimeout(10000 /*10s*/); + jest.useFakeTimers(); const testingModule: TestingModule = await Test.createTestingModule({ imports: [ConfigModule], providers: [ @@ -46,6 +63,10 @@ describe('GitHub Service', () => { post: jest.fn(() => true), }), }, + { + provide: Scheduler, + useValue: new VirtualTimeScheduler(), + }, ], }).compile(); @@ -207,6 +228,10 @@ describe('GitHub Service', () => { }); describe('fork repo', () => { + beforeEach(() => { + jest.useRealTimers(); + }); + it('creates a fork', async () => { // Setup expect.assertions(2); @@ -219,17 +244,124 @@ describe('GitHub Service', () => { config: {}, }; jest.spyOn(spyHttpService, 'post').mockImplementationOnce(() => of(result)); + jest + .spyOn(spyHttpService, 'get') + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => of(resultSuccess)); // Execute - const gitHubProject = await sut.forkRepo('12345', 'upstreamUser', 'myRepo').toPromise(); + const gitHubProject = await sut.forkRepo('12345', 'upstreamUser', 'foo', 'jdoe') + .toPromise(); // Verify - expect(gitHubProject).toEqual(projectFromGitHub); + expect(gitHubProject.full_name).toEqual('jdoe/foo'); expect(spyHttpService.post).toHaveBeenCalledWith( - 'https://api.github.com/repos/upstreamUser/myRepo/forks', - { headers: { Authorization: '12345' } }, + 'https://api.github.com/repos/upstreamUser/foo/forks', + null, + { headers: { authorization: '12345' } }, ); }); + + it('does not fail if fork already exists', async () => { + // Setup + expect.assertions(2); + + const result: AxiosResponse = { + data: projectFromGitHub, + status: 200, + statusText: '', + headers: {}, + config: {}, + }; + jest + .spyOn(spyHttpService, 'post') + .mockImplementationOnce(() => of(result)); + jest + .spyOn(spyHttpService, 'get') + .mockImplementationOnce(() => of(resultSuccess)); + + // Execute + const gitHubProject = await sut + .forkRepo('12345', 'upstreamUser', 'foo', 'jdoe') + .toPromise(); + + // Verify + expect(gitHubProject.full_name).toEqual('jdoe/foo'); + expect(spyHttpService.post).not.toHaveBeenCalled(); + }); + }); + + describe('check existence of repo', () => { + beforeEach(() => { + jest.useRealTimers(); + }); + + it('repo does not exist', async () => { + // Setup + expect.assertions(1); + jest.spyOn(spyHttpService, 'get').mockImplementationOnce(() => throwError(resultError) ); + + // Execute + const exists = await sut.repoExists('owner', 'repo').toPromise(); + + // Verify + expect(exists).toBe(false); + }); + + it('repo exists', async () => { + // Setup + expect.assertions(1); + jest.spyOn(spyHttpService, 'get').mockImplementationOnce(() => of(resultSuccess)); + + // Execute + const exists = await sut.repoExists('owner', 'repo').toPromise(); + + // Verify + expect(exists).toBe(true); + }); + }); + + describe('wait for existence of repo', () => { + beforeEach(() => { + jest.useRealTimers(); + }); + + it('waits until repo exists', async () => { + // Setup + expect.assertions(1); + jest + .spyOn(spyHttpService, 'get') + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => of(resultSuccess)); + + // Execute + await sut.waitForRepoToExist('owner', 'repo', 4).toPromise(); + + // Verify + expect(spyHttpService.get).toHaveBeenCalledTimes(4); + }); + + it('times out if repo does not exist', async () => { + // Setup + expect.assertions(1); + jest + .spyOn(spyHttpService, 'get') + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => throwError(resultError)) + .mockImplementationOnce(() => of(resultSuccess)); + + // Execute/Verify + try { + await sut.waitForRepoToExist('owner', 'repo', 3).toPromise(); + } catch (error) { + expect(spyHttpService.get).toHaveBeenCalledTimes(3); + } + }); }); }); diff --git a/src/github/github.service.ts b/src/github/github.service.ts index 783ffd9ee..6354a7396 100644 --- a/src/github/github.service.ts +++ b/src/github/github.service.ts @@ -1,17 +1,17 @@ import { HttpService, Injectable } from '@nestjs/common'; import { AxiosResponse, AxiosRequestConfig } from 'axios'; -import { Observable, of, empty } from 'rxjs'; -import { map, expand, concatMap } from 'rxjs/operators'; +import { Observable, of, empty, interval } from 'rxjs'; +import { map, expand, concatMap, catchError, mapTo, switchMap, takeWhile, takeLast } from 'rxjs/operators'; import { TokenService } from '../token/token.service'; import { ConfigService } from '../config/config.service'; import { GitHubAccessToken } from '../interfaces/github-access-token.interface'; import { GitHubUser } from '../interfaces/git-hub-user.interface'; +import { GitHubProject } from '../interfaces/git-hub-project.interface'; const redirectUri = '/index.html'; const scope = 'repo read:user user:email'; -interface GitHubProject { name: string; full_name?: string; owner?: { login: string; }; } @Injectable() export class GithubService { @@ -30,9 +30,7 @@ export class GithubService { const state = this.tokenService.createRandomString(10); const url = { url: - `https://github.com/login/oauth/authorize?client_id=${ - this.config.clientId - }&` + + `https://github.com/login/oauth/authorize?client_id=${this.config.clientId}&` + `redirect_uri=${this.getRedirectUri()}&scope=${scope}&state=${state}`, }; return of(url); @@ -73,9 +71,7 @@ export class GithubService { }; return this.httpService.get( 'https://github.com/login/oauth/access_token' + - `?client_id=${this.config.clientId}&client_secret=${ - this.config.clientSecret - }` + + `?client_id=${this.config.clientId}&client_secret=${this.config.clientSecret}` + `&code=${code}&state=${state}`, opt, ); @@ -107,12 +103,17 @@ export class GithubService { const url = `https://api.github.com/user/repos?type=public&sort=full_name&page=${page}&per_page=${pageSize}`; return this.getReposPage(url, token).pipe( - expand(({ nextPageUrl }) => nextPageUrl ? this.getReposPage(nextPageUrl, token) : empty()), + expand(({ nextPageUrl }) => + nextPageUrl ? this.getReposPage(nextPageUrl, token) : empty(), + ), concatMap(({ content }) => content), ); } - private getReposPage(url: string, token: string): Observable<{ content: GitHubProject[], nextPageUrl: string | null }> { + private getReposPage( + url: string, + token: string, + ): Observable<{ content: GitHubProject[]; nextPageUrl: string | null }> { return this.httpService .get(url, { headers: { Authorization: token } }) .pipe( @@ -135,17 +136,72 @@ export class GithubService { return url; } + public waitForRepoToExist( + owner: string, + repo: string, + timeoutSeconds: number = 300, + ): Observable { + return interval(1000).pipe( + switchMap((x: number) => { + if (x >= timeoutSeconds) { + throw new Error(`timeout after ${timeoutSeconds} seconds without seeing repo`); + } + + return this.repoExists(owner, repo); + }), + takeWhile(exists => !exists), + takeLast(1), + map(() => { /* empty */ }), + ); + } + + public repoExists(owner: string, repo: string): Observable { + return this.httpService.get(`https://github.com/${owner}/${repo}`).pipe( + mapTo(true), + catchError(() => of(false)), + ); + } + public forkRepo( token: string, - owner: string, + upstreamOwner: string, repo: string, + user: string, ): Observable { if (token == null || token.length === 0) { return null; } - return this.httpService.post(`https://api.github.com/repos/${owner}/${repo}/forks`, { - headers: { Authorization: token }, - }).pipe(map(result => result.data)); + return this.repoExists(user, repo).pipe( + switchMap(exists => { + if (exists) { + return of({ name: repo, full_name: `${user}/${repo}` }); + } + + let project: GitHubProject = null; + + return this.httpService + .post( + `https://api.github.com/repos/${upstreamOwner}/${repo}/forks`, + null, + { + headers: { authorization: token }, + }, + ) + .pipe( + switchMap(result => { + project = result.data; + return this.waitForRepoToExist(user, repo).pipe( + map(() => project), + ); + }), + catchError(() => of({ name: null })), + ); + }), + ); + } + + public get organizationName() { + return 'keymanapp'; } } diff --git a/src/interfaces/git-hub-project.interface.ts b/src/interfaces/git-hub-project.interface.ts new file mode 100644 index 000000000..81be5c51d --- /dev/null +++ b/src/interfaces/git-hub-project.interface.ts @@ -0,0 +1,5 @@ +export interface GitHubProject { + name: string; + full_name?: string; + owner?: { login: string; }; +} diff --git a/src/projects/projects.controller.spec.ts b/src/projects/projects.controller.spec.ts index db5277353..707994524 100644 --- a/src/projects/projects.controller.spec.ts +++ b/src/projects/projects.controller.spec.ts @@ -1,7 +1,8 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ProjectsController } from './projects.controller'; import { HttpModule } from '@nestjs/common/http'; -import { of, empty, combineLatest } from 'rxjs'; +import { of, empty, from } from 'rxjs'; +import { map } from 'rxjs/operators'; import fs = require('fs'); import os = require('os'); @@ -32,6 +33,8 @@ describe('Projects Controller', () => { provide: GithubService, useFactory: () => ({ getRepos: jest.fn(() => true), + forkRepo: jest.fn(() => of({ name: 'foo' })), + organizationName: jest.fn(() => 'keymanapp'), }), }, GitService, @@ -115,7 +118,7 @@ describe('Projects Controller', () => { await gitService.clone( repoDir, path.join(gitHubDir, 'foo', 'remoteTestRepo.git'), - ['--bare'], + true, ); }); @@ -142,7 +145,7 @@ describe('Projects Controller', () => { await gitService.clone( keyboardsDummyRepo, path.join(gitHubDir, user, 'keyboards.git'), - ['--bare'], + true, ); } @@ -152,7 +155,7 @@ describe('Projects Controller', () => { await gitService.clone( keyboardsDummyRepo, path.join(gitHubDir, 'keymanapp', 'keyboards.git'), - ['--bare'], + true, ); } @@ -169,21 +172,33 @@ describe('Projects Controller', () => { // Verify // Should clone single-keyboards repo const expectedRepo = path.join(workDir, 'foo-remoteTestRepo'); - expect(fs.existsSync(path.join(expectedRepo, '.git'))).toBeTruthy(); + expect(fs.existsSync(path.join(expectedRepo, '.git'))).toBe(true); expect(project).toEqual({ repoUrl: expectedRepo, name: 'remoteTestRepo' }); // Should clone keyboards repo const expectedKeyboardsRepo = path.join(workDir, 'keyboards'); - expect(fs.existsSync(path.join(expectedKeyboardsRepo, '.git'))).toBeTruthy(); + expect(fs.existsSync(path.join(expectedKeyboardsRepo, '.git'))).toBe(true); expect(await gitService.currentBranch(expectedKeyboardsRepo)).toEqual('foo-remoteTestRepo'); }); - // not working yet - xit('clones single-keyboard repo and forks keyboards repo', async () => { - expect.assertions(4); + it('clones single-keyboard repo and forks keyboards repo', async () => { + expect.assertions(5); // Setup const session = { login: 'foo' }; + await createGlobalKeyboardsRepo(); + + jest + .spyOn(githubService, 'forkRepo') + .mockImplementation(() => { + return from(gitService.clone( + path.join(gitHubDir, 'dummy', 'keyboards'), + path.join(gitHubDir, 'foo', 'keyboards.git'), + true, + )).pipe( + map(() => ({ name: 'foo' })), + ); + }); // Execute const project = await sut @@ -193,21 +208,19 @@ describe('Projects Controller', () => { // Verify // Should clone single-keyboards repo const expectedRepo = path.join(workDir, 'foo-remoteTestRepo'); - expect(fs.existsSync(path.join(expectedRepo, '.git'))).toBeTruthy(); + expect(fs.existsSync(path.join(expectedRepo, '.git'))).toBe(true); expect(project).toEqual({ repoUrl: expectedRepo, name: 'remoteTestRepo', }); + // should create fork of keyboards repo + expect(fs.existsSync(path.join(gitHubDir, 'foo', 'keyboards.git'))).toBe(true); // Should clone keyboards repo const expectedKeyboardsRepo = path.join(workDir, 'keyboards'); - expect( - fs.existsSync(path.join(expectedKeyboardsRepo, '.git')), - ).toBeTruthy(); - expect(await gitService.currentBranch(expectedKeyboardsRepo)).toEqual( - 'foo-remoteTestRepo', - ); + expect(fs.existsSync(path.join(expectedKeyboardsRepo, '.git'))).toBe(true); + expect(await gitService.currentBranch(expectedKeyboardsRepo)).toEqual('foo-remoteTestRepo'); }); }); }); diff --git a/src/projects/projects.controller.ts b/src/projects/projects.controller.ts index 154e606e6..3e5da3fc6 100644 --- a/src/projects/projects.controller.ts +++ b/src/projects/projects.controller.ts @@ -1,6 +1,6 @@ import { Controller, Get, Headers, Session, Post, Param } from '@nestjs/common'; import { Observable, from, combineLatest } from 'rxjs'; -import { map, toArray, filter } from 'rxjs/operators'; +import { map, toArray, filter, switchMap } from 'rxjs/operators'; import { GithubService } from '../github/github.service'; import { BackendProjectService } from '../backend-project/backend-project.service'; @@ -40,19 +40,45 @@ export class ProjectsController { const localRepo = this.backendService.getProjectRepo(session.login, params.repo); const remoteRepo = `${this.gitHubUrl}/${session.login}/${params.repo}.git`; const createSingleProject = from( - this.backendService.cloneOrUpdateProject(remoteRepo, localRepo, this.backendService.branchName), - ).pipe(map(project => ({ name: params.repo, repoUrl: project }))); - - const remoteKeyboardsRepo = `${this.gitHubUrl}/${session.login}/${this.backendService.keyboardsRepoName}.git`; - const localKeyboardsRepo = this.backendService.localKeyboardsRepo; - const createKeyboardsProject = from( this.backendService.cloneOrUpdateProject( - remoteKeyboardsRepo, - localKeyboardsRepo, - `${session.login}-${params.repo}`, + remoteRepo, + localRepo, + this.backendService.branchName, + session.login, ), ).pipe(map(project => ({ name: params.repo, repoUrl: project }))); - return combineLatest(createSingleProject, createKeyboardsProject, (result1) => result1); + const createKeyboardsRepo = from( + this.forkCloneAndUpdateProject(token, session.login, this.backendService.keyboardsRepoName, params.repo, 'master'), + ).pipe(map(project => ({ name: params.repo, repoUrl: project }))); + + return combineLatest( + createSingleProject, + createKeyboardsRepo, + result1 => result1, + ); + } + + private forkCloneAndUpdateProject( + token: string, + gitHubUser: string, + repoName: string, + branchName: string, + remoteBranch: string, + ): Observable { + return this.githubService + .forkRepo(token, this.githubService.organizationName, repoName, gitHubUser) + .pipe(switchMap(() => { + const remoteKeyboardsRepo = `${this.gitHubUrl}/${gitHubUser}/${this.backendService.keyboardsRepoName}.git`; + const localKeyboardsRepo = this.backendService.localKeyboardsRepo; + return from(this.backendService.cloneOrUpdateProject( + remoteKeyboardsRepo, + localKeyboardsRepo, + `${gitHubUser}-${branchName}`, + gitHubUser, + remoteBranch, + )); + }), + ); } } diff --git a/src/utils/test-utils.ts b/src/utils/test-utils.ts new file mode 100644 index 000000000..b03a17c9a --- /dev/null +++ b/src/utils/test-utils.ts @@ -0,0 +1,22 @@ +function evaluateCondition(condition) { + return typeof condition === 'function' ? condition() : !!condition; +} + +export function describeIf(name: string, condition: any, tests: any) { + if (evaluateCondition(condition)) { + describe(name, tests); + } else { + describe(name, () => { + it(`skipped '${name}' because condition doesn't apply`, () => { /* empty */ }); + }); + describe.skip('', tests); + } +} + +export function itIf(name: string, condition: any, test: any) { + if (evaluateCondition(condition)) { + return it(name, test); + } else { + return it.skip(name, test); + } +} diff --git a/test.env b/test.env index 178335534..22aaf43b6 100644 --- a/test.env +++ b/test.env @@ -6,3 +6,4 @@ SESSION_SECRET=session-secret EXPIRES_DAYS=1 COOKIE_MAX_AGE=1 WORKDIR=/tmp +UPSTREAM_GITHUB_URL=https://github.com/keymanapptest diff --git a/test/jest-e2e.json b/test/jest-e2e.json index e9d912f3e..0f42fdd13 100644 --- a/test/jest-e2e.json +++ b/test/jest-e2e.json @@ -5,5 +5,7 @@ "testRegex": ".e2e-spec.ts$", "transform": { "^.+\\.(t|j)s$": "ts-jest" - } + }, + "coverageDirectory": "/../coverage", + "verbose": true } diff --git a/test/projects.e2e-spec.ts b/test/projects.e2e-spec.ts new file mode 100644 index 000000000..b898b9bcb --- /dev/null +++ b/test/projects.e2e-spec.ts @@ -0,0 +1,108 @@ +import { HttpService, INestApplication } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import * as base64 from 'base-64'; +import { empty } from 'rxjs'; +import { catchError } from 'rxjs/operators'; +import session = require('supertest-session'); +import { AppModule } from '../src/app.module'; +import { describeIf } from '../src/utils/test-utils'; +import { GithubService } from '../src/github/github.service'; + +function canRunTheseTests(): boolean { + return ( + process.env.TEST_GITHUB_TOKEN != null && + process.env.TEST_GITHUB_USER != null + ); +} + +describeIf('ProjectsController (e2e)', canRunTheseTests(), () => { + let app: INestApplication; + let httpService: HttpService; + let token: string; + let user: string; + let testSession: any; + let authenticatedSession: any; + let githubService: GithubService; + + async function deleteKeyboardsRepo(): Promise { + await httpService.delete(`https://api.github.com/repos/${user}/keyboards`, { + headers: { Authorization: token }, + }).pipe(catchError(() => empty())).toPromise(); + await httpService.delete(`https://api.github.com/repos/${user}/keyboards-1`, { + headers: { Authorization: token }, + }).pipe(catchError(() => empty())).toPromise(); + } + + beforeAll(async () => { + jest.setTimeout(20000); + jest.useRealTimers(); + const accessToken = process.env.TEST_GITHUB_TOKEN; + user = process.env.TEST_GITHUB_USER; + + const tokenString = `${user}:${accessToken}`; + token = `Basic ${base64.encode(tokenString)}`; + + const moduleFixture: TestingModule = await Test.createTestingModule({ + imports: [AppModule], + }).compile(); + + httpService = moduleFixture.get(HttpService); + githubService = moduleFixture.get(GithubService); + jest.spyOn(githubService, 'organizationName', 'get') + .mockImplementation(() => 'keymanapptest'); + + app = moduleFixture.createNestApplication(); + app.setGlobalPrefix('api'); + await app.init(); + + testSession = session(app.getHttpServer()); + }); + + afterAll(async () => { + await app.close(); + }); + + beforeEach(async (done) => { + await deleteKeyboardsRepo(); + testSession + .get(`/api/auth/user/test/${user}`) + .set('Authorization', token) + .expect(200) + .end((err: any, res: any) => { + if (err) { return done(err); } + authenticatedSession = testSession; + return done(); + }); + }); + + afterEach(async () => { + await deleteKeyboardsRepo(); + }); + + it('clones single-keyboard project and forks and clones keyboards repo', async (done) => { + // Execute/Verify + authenticatedSession + .post('/api/projects/khmer_angkor') + .set('Authorization', token) + .expect(201) + .end((err) => { + expect(err).toBeNull(); + done(); + }); + }); + + it('clones single-keyboard project and clones keyboards repo if keyboards repo exists', async done => { + // Setup + await githubService.forkRepo(token, 'keymanapptest', 'keyboards', user).toPromise(); + + // Execute/Verify + authenticatedSession + .post('/api/projects/khmer_angkor') + .set('Authorization', token) + .expect(201) + .end((err) => { + expect(err).toBeNull(); + done(); + }); + }); +}); From 5575e42a418daf845fb62557fd4e49b6a00bda1c Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 2 Dec 2019 17:20:52 +0100 Subject: [PATCH 2/6] feat: Extract keyboard id from keyboard_info or repo name This implements parts of #10. --- .../backend-project.service.spec.ts | 57 +++++++++++++++++++ .../backend-project.service.ts | 14 ++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/backend-project/backend-project.service.spec.ts b/src/backend-project/backend-project.service.spec.ts index 4cfb1b059..0e5c9477f 100644 --- a/src/backend-project/backend-project.service.spec.ts +++ b/src/backend-project/backend-project.service.spec.ts @@ -176,4 +176,61 @@ describe('BackendProjectService', () => { }); }); + describe('getKeyboardId', () => { + let testDir: string; + + beforeEach(() => { + const prefix = path.join(os.tmpdir(), 'backendprojecttests-'); + testDir = fs.mkdtempSync(prefix); + }); + + afterEach(() => { + deleteFolderRecursive(tmpDir); + }); + + it('extracts id from keyboard_info file if it has an id', () => { + // Setup + expect.assertions(1); + fs.appendFileSync(path.join(testDir, 'enga.keyboard_info'), `{ + "id": "test", + "license": "mit", + "languages": [ "enq-Latn" ], + "description": "The Enga keyboard supports the Enga language of Papua New Guinea" + }`); + + // Execute + const id = sut.getKeyboardId('enga', testDir); + + // Verify + expect(id).toEqual('test'); + }); + + it('gets id from repo name if keyboard_info file does not have id', () => { + // Setup + expect.assertions(1); + fs.appendFileSync(path.join(testDir, 'enga.keyboard_info'), `{ + "license": "mit", + "languages": [ "enq-Latn" ], + "description": "The Enga keyboard supports the Enga language of Papua New Guinea" + }`); + + // Execute + const id = sut.getKeyboardId('enga', testDir); + + // Verify + expect(id).toEqual('enga'); + }); + + it('gets id from repo name if keyboard_info file does not exist', () => { + // Setup + expect.assertions(1); + + // Execute + const id = sut.getKeyboardId('enga', testDir); + + // Verify + expect(id).toEqual('enga'); + }); + }); + }); diff --git a/src/backend-project/backend-project.service.ts b/src/backend-project/backend-project.service.ts index 4b7217015..8be7094c6 100644 --- a/src/backend-project/backend-project.service.ts +++ b/src/backend-project/backend-project.service.ts @@ -28,7 +28,6 @@ export class BackendProjectService { const branchExists = await this.gitService.isBranch(localRepo, localBranch); if (!branchExists) { await this.checkoutBranch(localRepo, localBranch, `${remoteName}/${remoteBranch}`); - // this.gitService.push(localRepo, ) } return this.gitService.pull(localRepo, remoteName, remoteBranch).then(() => localRepo); } else { @@ -60,4 +59,17 @@ export class BackendProjectService { public get localKeyboardsRepo() { return path.join(this.config.workDirectory, this.keyboardsRepoName); } + + public getKeyboardId(repoName: string, localRepo: string): string { + const keyboardInfoFile = path.join(localRepo, `${repoName}.keyboard_info`); + if (fs.existsSync(keyboardInfoFile)) { + const fileContent = fs.readFileSync(keyboardInfoFile); + const data = JSON.parse(fileContent.toString()); + if (!!data.id) { + return data.id; + } + } + + return repoName; + } } From d43e99b56f3aa7feb58d97ed2f64cb7dc8b66802 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 2 Dec 2019 17:46:45 +0100 Subject: [PATCH 3/6] feat: Use extracted keyboard id as return value for createRepo --- src/projects/projects.controller.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/projects/projects.controller.ts b/src/projects/projects.controller.ts index 3e5da3fc6..e3a4c96a9 100644 --- a/src/projects/projects.controller.ts +++ b/src/projects/projects.controller.ts @@ -46,7 +46,10 @@ export class ProjectsController { this.backendService.branchName, session.login, ), - ).pipe(map(project => ({ name: params.repo, repoUrl: project }))); + ).pipe(map(project => ({ + name: this.backendService.getKeyboardId(params.repo, localRepo), + repoUrl: project, + }))); const createKeyboardsRepo = from( this.forkCloneAndUpdateProject(token, session.login, this.backendService.keyboardsRepoName, params.repo, 'master'), From 5c294b6c4baa7be639aa90b97451eb2003f671e2 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 2 Dec 2019 17:58:56 +0100 Subject: [PATCH 4/6] docs: Update readme for e2e tests --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index 13ed89d29..05913d7ce 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,15 @@ In the `*.env` file replace the values for `CLIENT_ID` and `CLIENT_SECRET` with _Client Secret_ that GitHub displays for the app. You should also replace the value for `SESSION_SECRET` with a random value. +If you want to run all e2e tests, you'll have to create a test user on GitHub. Fork +[kdotester1/khmer_angkor](https://github.com/kdotester1/khmer_angkor) to your test +account and set two environment variables before running the e2e tests: + +```bash +export TEST_GITHUB_USER= +export TEST_GITHUB_TOKEN= +``` + ## Development ```bash From a1705e78fd08ced7286c5b25531d570a00d5e671 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 6 Dec 2019 15:43:27 +0100 Subject: [PATCH 5/6] Follow-up of code review - rename repos used in e2e tests - add some comments - some cleanup of unused code --- README.md | 2 +- .../backend-project.service.spec.ts | 4 --- .../backend-project.service.ts | 6 +--- src/config/config.service.ts | 15 ++++++---- src/github/github.service.ts | 4 --- src/projects/projects.controller.ts | 8 ++++-- src/projects/projects.module.ts | 3 +- src/utils/test-utils.ts | 5 ++++ test.env | 1 - test/projects.e2e-spec.ts | 28 ++++++++++++++----- 10 files changed, 45 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 05913d7ce..d0be37b41 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ _Client Secret_ that GitHub displays for the app. You should also replace the va `SESSION_SECRET` with a random value. If you want to run all e2e tests, you'll have to create a test user on GitHub. Fork -[kdotester1/khmer_angkor](https://github.com/kdotester1/khmer_angkor) to your test +[keymanapp/test_kdo_khmer_angkor](https://github.com/keymanapp/test_kdo_khmer_angkor) to your test account and set two environment variables before running the e2e tests: ```bash diff --git a/src/backend-project/backend-project.service.spec.ts b/src/backend-project/backend-project.service.spec.ts index 0e5c9477f..937be859d 100644 --- a/src/backend-project/backend-project.service.spec.ts +++ b/src/backend-project/backend-project.service.spec.ts @@ -167,10 +167,6 @@ describe('BackendProjectService', () => { expect(sut.branchName).toEqual('master'); }); - it('Returns expected name of keyboards repo', () => { - expect(sut.keyboardsRepoName).toEqual('keyboards'); - }); - it('Returns expected path of the keyboards repo', () => { expect(sut.localKeyboardsRepo).toEqual(path.join(workDir, 'keyboards')); }); diff --git a/src/backend-project/backend-project.service.ts b/src/backend-project/backend-project.service.ts index 8be7094c6..4d20db592 100644 --- a/src/backend-project/backend-project.service.ts +++ b/src/backend-project/backend-project.service.ts @@ -52,12 +52,8 @@ export class BackendProjectService { return 'master'; } - public get keyboardsRepoName() { - return 'keyboards'; - } - public get localKeyboardsRepo() { - return path.join(this.config.workDirectory, this.keyboardsRepoName); + return path.join(this.config.workDirectory, this.config.keyboardsRepoName); } public getKeyboardId(repoName: string, localRepo: string): string { diff --git a/src/config/config.service.ts b/src/config/config.service.ts index c304060ef..a861876ab 100644 --- a/src/config/config.service.ts +++ b/src/config/config.service.ts @@ -45,8 +45,6 @@ export class ConfigService { const expiresDays = process.env.EXPIRES_DAYS != null ? process.env.EXPIRES_DAYS : 1; const cookieMaxAge = process.env.COOKIE_MAX_AGE != null ? process.env.COOKIE_MAX_AGE : 1; const workDirectory = process.env.WORKDIR != null ? process.env.WORKDIR : '/tmp'; - const upstreamGitHubUrl = process.env.UPSTREAM_GITHUB_URL != null - ? process.env.UPSTREAM_GITHUB_URL : 'https://github.com/keymanapp'; const envVarsSchema: Joi.ObjectSchema = Joi.object({ NODE_ENV: Joi.string() @@ -69,7 +67,6 @@ export class ConfigService { EXPIRES_DAYS: Joi.number().default(expiresDays), COOKIE_MAX_AGE: Joi.number().default(cookieMaxAge), WORKDIR: Joi.string().default(workDirectory), - UPSTREAM_GITHUB_URL: Joi.string().default(upstreamGitHubUrl), }); const { error, value: validatedEnvConfig } = envVarsSchema.validate(envConfig); @@ -115,7 +112,15 @@ export class ConfigService { return this.envConfig.WORKDIR; } - public get upstreamGitHubUrl(): string { - return this.envConfig.UPSTREAM_GITHUB_URL; + // Name of the GitHub organization that hosts the keyboards repo. This property exists + // so that we can change the name for e2e tests. + public get organizationName() { + return 'keymanapp'; + } + + // Name of the `keyboards` repo. This property exists so that we can change the name + // for e2e tests. + public get keyboardsRepoName(): string { + return 'keyboards'; } } diff --git a/src/github/github.service.ts b/src/github/github.service.ts index 6354a7396..f752bc7b9 100644 --- a/src/github/github.service.ts +++ b/src/github/github.service.ts @@ -200,8 +200,4 @@ export class GithubService { }), ); } - - public get organizationName() { - return 'keymanapp'; - } } diff --git a/src/projects/projects.controller.ts b/src/projects/projects.controller.ts index e3a4c96a9..8f6bd9073 100644 --- a/src/projects/projects.controller.ts +++ b/src/projects/projects.controller.ts @@ -3,6 +3,7 @@ import { Observable, from, combineLatest } from 'rxjs'; import { map, toArray, filter, switchMap } from 'rxjs/operators'; import { GithubService } from '../github/github.service'; import { BackendProjectService } from '../backend-project/backend-project.service'; +import { ConfigService } from '../config/config.service'; interface Project { name: string; repoUrl?: string; } @@ -11,6 +12,7 @@ export class ProjectsController { constructor( private readonly githubService: GithubService, private readonly backendService: BackendProjectService, + private readonly configService: ConfigService, ) {} public get gitHubUrl(): string { @@ -52,7 +54,7 @@ export class ProjectsController { }))); const createKeyboardsRepo = from( - this.forkCloneAndUpdateProject(token, session.login, this.backendService.keyboardsRepoName, params.repo, 'master'), + this.forkCloneAndUpdateProject(token, session.login, this.configService.keyboardsRepoName, params.repo, 'master'), ).pipe(map(project => ({ name: params.repo, repoUrl: project }))); return combineLatest( @@ -70,9 +72,9 @@ export class ProjectsController { remoteBranch: string, ): Observable { return this.githubService - .forkRepo(token, this.githubService.organizationName, repoName, gitHubUser) + .forkRepo(token, this.configService.organizationName, repoName, gitHubUser) .pipe(switchMap(() => { - const remoteKeyboardsRepo = `${this.gitHubUrl}/${gitHubUser}/${this.backendService.keyboardsRepoName}.git`; + const remoteKeyboardsRepo = `${this.gitHubUrl}/${gitHubUser}/${this.configService.keyboardsRepoName}.git`; const localKeyboardsRepo = this.backendService.localKeyboardsRepo; return from(this.backendService.cloneOrUpdateProject( remoteKeyboardsRepo, diff --git a/src/projects/projects.module.ts b/src/projects/projects.module.ts index 29b6db880..2e5922c70 100644 --- a/src/projects/projects.module.ts +++ b/src/projects/projects.module.ts @@ -1,10 +1,11 @@ import { Module } from '@nestjs/common'; import { BackendProjectModule } from '../backend-project/backend-project.module'; +import { ConfigModule } from '../config/config.module'; import { GithubModule } from '../github/github.module'; import { ProjectsController } from './projects.controller'; @Module({ - imports: [GithubModule, BackendProjectModule], + imports: [GithubModule, BackendProjectModule, ConfigModule], controllers: [ProjectsController], }) export class ProjectsModule {} diff --git a/src/utils/test-utils.ts b/src/utils/test-utils.ts index b03a17c9a..b952d3d40 100644 --- a/src/utils/test-utils.ts +++ b/src/utils/test-utils.ts @@ -2,6 +2,10 @@ function evaluateCondition(condition) { return typeof condition === 'function' ? condition() : !!condition; } +// Skip the tests if condition is not met. If the tests are skipped it +// adds a new (green-baring test) to make it explicit why the tests +// are skipped (unfortunately Jest doesn't allow to specify a message +// why a test is skipped). export function describeIf(name: string, condition: any, tests: any) { if (evaluateCondition(condition)) { describe(name, tests); @@ -13,6 +17,7 @@ export function describeIf(name: string, condition: any, tests: any) { } } +// Skip this test if the condition is not met export function itIf(name: string, condition: any, test: any) { if (evaluateCondition(condition)) { return it(name, test); diff --git a/test.env b/test.env index 22aaf43b6..178335534 100644 --- a/test.env +++ b/test.env @@ -6,4 +6,3 @@ SESSION_SECRET=session-secret EXPIRES_DAYS=1 COOKIE_MAX_AGE=1 WORKDIR=/tmp -UPSTREAM_GITHUB_URL=https://github.com/keymanapptest diff --git a/test/projects.e2e-spec.ts b/test/projects.e2e-spec.ts index b898b9bcb..b069be4b0 100644 --- a/test/projects.e2e-spec.ts +++ b/test/projects.e2e-spec.ts @@ -7,6 +7,7 @@ import session = require('supertest-session'); import { AppModule } from '../src/app.module'; import { describeIf } from '../src/utils/test-utils'; import { GithubService } from '../src/github/github.service'; +import { ConfigService } from '../src/config/config.service'; function canRunTheseTests(): boolean { return ( @@ -23,12 +24,15 @@ describeIf('ProjectsController (e2e)', canRunTheseTests(), () => { let testSession: any; let authenticatedSession: any; let githubService: GithubService; + let configService: ConfigService; async function deleteKeyboardsRepo(): Promise { - await httpService.delete(`https://api.github.com/repos/${user}/keyboards`, { + const keyboardsRepo = `https://api.github.com/repos/${user}/${configService.keyboardsRepoName}`; + await httpService.delete(keyboardsRepo, { headers: { Authorization: token }, }).pipe(catchError(() => empty())).toPromise(); - await httpService.delete(`https://api.github.com/repos/${user}/keyboards-1`, { + // when we try to fork a repo that already exists GitHub creates a repo-1. + await httpService.delete(`${keyboardsRepo}-1`, { headers: { Authorization: token }, }).pipe(catchError(() => empty())).toPromise(); } @@ -48,8 +52,11 @@ describeIf('ProjectsController (e2e)', canRunTheseTests(), () => { httpService = moduleFixture.get(HttpService); githubService = moduleFixture.get(GithubService); - jest.spyOn(githubService, 'organizationName', 'get') + configService = moduleFixture.get(ConfigService); + jest.spyOn(configService, 'organizationName', 'get') .mockImplementation(() => 'keymanapptest'); + jest.spyOn(configService, 'keyboardsRepoName', 'get') + .mockImplementation(() => 'test_kdo_keyboards'); app = moduleFixture.createNestApplication(); app.setGlobalPrefix('api'); @@ -82,7 +89,7 @@ describeIf('ProjectsController (e2e)', canRunTheseTests(), () => { it('clones single-keyboard project and forks and clones keyboards repo', async (done) => { // Execute/Verify authenticatedSession - .post('/api/projects/khmer_angkor') + .post('/api/projects/test_kdo_khmer_angkor') .set('Authorization', token) .expect(201) .end((err) => { @@ -93,14 +100,21 @@ describeIf('ProjectsController (e2e)', canRunTheseTests(), () => { it('clones single-keyboard project and clones keyboards repo if keyboards repo exists', async done => { // Setup - await githubService.forkRepo(token, 'keymanapptest', 'keyboards', user).toPromise(); + await githubService + .forkRepo( + token, + configService.organizationName, + configService.keyboardsRepoName, + user, + ) + .toPromise(); // Execute/Verify authenticatedSession - .post('/api/projects/khmer_angkor') + .post('/api/projects/test_kdo_khmer_angkor') .set('Authorization', token) .expect(201) - .end((err) => { + .end(err => { expect(err).toBeNull(); done(); }); From cb79b4f47147af3d4fad2403c56037c13365947a Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Tue, 10 Dec 2019 11:09:56 +0100 Subject: [PATCH 6/6] docs: Add comments on some methods --- src/github/github.service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/github/github.service.ts b/src/github/github.service.ts index f752bc7b9..2c8d5023f 100644 --- a/src/github/github.service.ts +++ b/src/github/github.service.ts @@ -110,6 +110,7 @@ export class GithubService { ); } + // Get one page full of repos private getReposPage( url: string, token: string, @@ -124,6 +125,7 @@ export class GithubService { ); } + // Extract the URL of the next page from the headers private getUrlOfNextPage(response: AxiosResponse): string | null { let url: string | null = null; const link = response.headers.link; @@ -136,6 +138,8 @@ export class GithubService { return url; } + // When we fork a project on GitHub the API call immediately returns before the + // repo actually exists. This method waits until the project appears (or timeout). public waitForRepoToExist( owner: string, repo: string,