From 14b621caf5bcdcbf7476b3286afbc30380d2cc3f Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sat, 14 Dec 2019 17:39:39 +0100 Subject: [PATCH] test/style: remove or hide unused code in git.js, add tests --- .eslintrc.js | 2 +- Gruntfile.js | 2 +- package-lock.json | 31 +++++- package.json | 3 +- tasks/{publish.js => publish-to-aws.js} | 6 +- tasks/task-tests/.eslintrc.js | 9 ++ tasks/task-tests/README.md | 1 + tasks/task-tests/git.test.js | 121 ++++++++++++++++++++++++ tasks/task-tests/mocha.opts | 0 tasks/util/git.js | 77 ++++++++------- 10 files changed, 202 insertions(+), 50 deletions(-) rename tasks/{publish.js => publish-to-aws.js} (93%) create mode 100644 tasks/task-tests/.eslintrc.js create mode 100644 tasks/task-tests/README.md create mode 100644 tasks/task-tests/git.test.js create mode 100644 tasks/task-tests/mocha.opts diff --git a/.eslintrc.js b/.eslintrc.js index 0abcd580..65c24093 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -17,7 +17,7 @@ module.exports = { // Best Practices // //----------------// 'default-case': 'warn', - 'dot-notation': ['error', { allowKeywords: false }], + 'dot-notation': 'warn', 'guard-for-in': 'warn', 'no-alert': 'error', 'no-caller': 'error', diff --git a/Gruntfile.js b/Gruntfile.js index 0326cfc8..65e0b8f3 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -279,7 +279,7 @@ module.exports = function(grunt) { 'bgShell:integrationTests', 'sauce', 'metrics', - 'publish:latest' + 'publish-to-aws' ]); grunt.registerTask('on-file-change', [ 'build', diff --git a/package-lock.json b/package-lock.json index 9c79a3d7..5d119122 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2594,6 +2594,19 @@ "strip-json-comments": "^2.0.1", "tslint": "^5.14.0", "typescript": "^3.4.3" + }, + "dependencies": { + "fs-extra": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-6.0.1.tgz", + "integrity": "sha512-GnyIkKhhzXZUWFCaJzvyDLEEgDkPfb4/TPvJCJVuS8MWZgoSsErf++QpiAlDnKFcqhRlm+tIOcencCjyJE6ZCA==", + "dev": true, + "requires": { + "graceful-fs": "^4.1.2", + "jsonfile": "^4.0.0", + "universalify": "^0.1.0" + } + } } }, "dustjs-linkedin": { @@ -3699,14 +3712,22 @@ "dev": true }, "fs-extra": { - "version": "6.0.1", - "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-6.0.1.tgz", - "integrity": "sha1-irwSj3lG4xATXdyTuYvdtBDno0s=", + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-8.1.0.tgz", + "integrity": "sha512-yhlQgA6mnOJUKOsRUFsgJdQCvkKhcz8tlZG5HBQfReYZy46OwLcY+Zia0mtdHsOo9y/hP+CxMN0TU9QxoOtG4g==", "dev": true, "requires": { - "graceful-fs": "^4.1.2", + "graceful-fs": "^4.2.0", "jsonfile": "^4.0.0", "universalify": "^0.1.0" + }, + "dependencies": { + "graceful-fs": { + "version": "4.2.3", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.3.tgz", + "integrity": "sha512-a30VEBm4PEdx1dRB7MFK7BejejvCvBronbLjht+sHuGYj8PHs7M/5Z+rt5lw551vZ7yfTCj4Vuyy3mSJytDWRQ==", + "dev": true + } } }, "fs-readdir-recursive": { @@ -10566,7 +10587,7 @@ "universalify": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", - "integrity": "sha1-tkb2m+OULavOzJ1mOcgNwQXvqmY=", + "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==", "dev": true }, "unpipe": { diff --git a/package.json b/package.json index 6c8ca555..19f096a5 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "eslint-config-prettier": "^6.7.0", "eslint-plugin-compat": "^3.3.0", "eslint-plugin-es5": "^1.4.1", + "fs-extra": "^8.1.0", "grunt": "^1.0.4", "grunt-babel": "^5.0.0", "grunt-bg-shell": "^2.3.3", @@ -86,7 +87,7 @@ "lint": "eslint --max-warnings 0 . ", "dtslint": "dtslint types", "test": "grunt", - "extensive-tests-and-publish-to-aws": "grunt --stack extensive-tests-and-publish-to-aws", + "extensive-tests-and-publish-to-aws": "npx mocha tasks/task-tests/ && grunt --stack extensive-tests-and-publish-to-aws", "integration-test": "grunt integration-tests", "--- combined tasks ---": "", "check-before-pull-request": "concurrently --kill-others-on-fail npm:lint npm:dtslint npm:check-format npm:test" diff --git a/tasks/publish.js b/tasks/publish-to-aws.js similarity index 93% rename from tasks/publish.js rename to tasks/publish-to-aws.js index 54c99f5c..b551e1c9 100644 --- a/tasks/publish.js +++ b/tasks/publish-to-aws.js @@ -6,12 +6,12 @@ const semver = require('semver'); module.exports = function(grunt) { const registerAsyncTask = createRegisterAsyncTaskFn(grunt); - registerAsyncTask('publish:latest', async () => { + registerAsyncTask('publish-to-aws', async () => { grunt.log.writeln('remotes: ' + (await git.remotes())); grunt.log.writeln('branches: ' + (await git.branches())); const commitInfo = await git.commitInfo(); - grunt.log.writeln('tag: ' + commitInfo.tagName); + grunt.log.writeln('tag: ', commitInfo.tagName); const suffixes = []; @@ -22,7 +22,7 @@ module.exports = function(grunt) { } // Publish tags by their tag-name - if (commitInfo.tagName && semver.valid(commitInfo.tagName)) { + if (commitInfo.tagName != null && semver.valid(commitInfo.tagName)) { suffixes.push('-' + commitInfo.tagName); } diff --git a/tasks/task-tests/.eslintrc.js b/tasks/task-tests/.eslintrc.js new file mode 100644 index 00000000..604aeb2c --- /dev/null +++ b/tasks/task-tests/.eslintrc.js @@ -0,0 +1,9 @@ +module.exports = { + extends: '../../.eslintrc.js', + env: { + mocha: true + }, + parserOptions: { + ecmaVersion: 2018 + } +}; diff --git a/tasks/task-tests/README.md b/tasks/task-tests/README.md new file mode 100644 index 00000000..f4a88806 --- /dev/null +++ b/tasks/task-tests/README.md @@ -0,0 +1 @@ +Use `mocha tasks/task-tests` to run these tests diff --git a/tasks/task-tests/git.test.js b/tasks/task-tests/git.test.js new file mode 100644 index 00000000..89546a8b --- /dev/null +++ b/tasks/task-tests/git.test.js @@ -0,0 +1,121 @@ +const os = require('os'); +const path = require('path'); +const fs = require('fs-extra'); +const chai = require('chai'); +chai.use(require('dirty-chai')); + +const git = require('../util/git'); + +const expect = chai.expect; + +const tmpBaseDir = path.join(os.tmpdir(), 'handlebars-task-tests'); +const tmpDir = path.join(tmpBaseDir, Date.now().toString(36)); +const remoteDir = path.join(tmpDir, 'remote-repo'); +const cloneDir = path.join(tmpDir, 'clone-repo'); +const oldCwd = process.cwd(); + +describe('utils/git', function() { + beforeEach(async function() { + await fs.remove(tmpDir); + await createRepositoryThatActsAsRemote(); + process.chdir(tmpDir); + await git.git('clone', 'remote-repo', 'clone-repo'); + process.chdir(cloneDir); + }); + + async function createRepositoryThatActsAsRemote() { + await fs.mkdirp(remoteDir); + process.chdir(remoteDir); + + await git.git('init'); + await fs.writeFile('testfile.txt', 'Testfile'); + await git.add('testfile.txt'); + await git.commit('commit message'); + } + + afterEach(function() { + process.chdir(oldCwd); + }); + + describe('the "remotes"-function', function() { + it('should list all remotes', async function() { + await git.git('remote', 'set-url', 'origin', 'https://test.org/test'); + await git.git('remote', 'add', 'second-remote', 'https://test.org/test2'); + + const result = await git.remotes(); + + expect(result.trim().split('\n')).to.deep.equal([ + 'origin\thttps://test.org/test (fetch)', + 'origin\thttps://test.org/test (push)', + 'second-remote\thttps://test.org/test2 (fetch)', + 'second-remote\thttps://test.org/test2 (push)' + ]); + }); + }); + + describe('the "branches"-function', function() { + it('should list all branches', async function() { + await git.git('branch', 'test'); + await git.git('branch', 'test2'); + + const result = await git.branches(); + expect(result.trim().split('\n')).to.deep.equal([ + '* master', + ' test', + ' test2', + ' remotes/origin/HEAD -> origin/master', + ' remotes/origin/master' + ]); + }); + }); + + describe('the "commitInfo"-function', function() { + it('should list head and master sha', async function() { + const result = await git.commitInfo(); + expect(result.masterSha).to.equal(result.headSha); + expect(result.masterSha).to.match(/^[0-9a-f]+$/); + expect(result.headSha).to.match(/^[0-9a-f]+$/); + }); + + it('should have "isMaster=true" if the master branch is checked out', async function() { + const result = await git.commitInfo(); + expect(result.isMaster).to.be.true(); + }); + + it('should have "isMaster=true" if the current commit is the last commit of the master branch', async function() { + await git.git('checkout', '-b', 'new-branch'); + + const result = await git.commitInfo(); + expect(result.isMaster).to.be.true(); + }); + + it('should have "isMaster=false" if the current commit is NOT the last commit of the master branch', async function() { + await git.git('checkout', '-b', 'new-branch'); + fs.writeFile('new-file.txt', 'new-file'); + await git.add('new-file.txt'); + await git.commit('added new file'); + + const result = await git.commitInfo(); + expect(result.isMaster).to.be.false(); + }); + + it('should show the current tag', async function() { + await git.git('tag', 'test-tag'); + const result = await git.commitInfo(); + expect(result.tagName).to.be.equal('test-tag'); + }); + + it('should show a version tag rather than standard tags', async function() { + await git.git('tag', 'test-tag'); + await git.git('tag', 'v1.2'); + await git.git('tag', 'test-tag2'); + const result = await git.commitInfo(); + expect(result.tagName).to.be.equal('v1.2'); + }); + + it('should show no tag if there is no tag', async function() { + const result = await git.commitInfo(); + expect(result.tagName).to.be.null(); + }); + }); +}); diff --git a/tasks/task-tests/mocha.opts b/tasks/task-tests/mocha.opts new file mode 100644 index 00000000..e69de29b diff --git a/tasks/util/git.js b/tasks/util/git.js index a86fcbb6..46c9cbe7 100644 --- a/tasks/util/git.js +++ b/tasks/util/git.js @@ -2,64 +2,63 @@ const childProcess = require('child_process'); module.exports = { async remotes() { - return git('remotes', '-v'); + return git('remote', '-v'); }, async branches() { return git('branch', '-a'); }, - async clean() { - const stdout = git('diff-index', '--name-only', 'HEAD', '--'); - return stdout === ''; - }, async commitInfo() { - const headSha = await this.headSha(); - const masterSha = await this.masterSha(); + const headSha = await getHeadSha(); + const masterSha = await getMasterSha(); return { headSha, masterSha, - tagName: await this.tagName(), + tagName: await getTagName(), isMaster: headSha === masterSha }; }, - async headSha() { - const stdout = await git(' rev-parse', '--short', 'HEAD'); - return stdout.trim(); - }, - async masterSha() { - try { - const stdout = await git('rev-parse', '--short', 'origin/master'); - return stdout.trim(); - } catch (error) { - if (/Needed a single revision/.test(error.message)) { - // Master was not checked out but in this case, so we know we are not master. We can ignore this - return ''; - } - throw error; - } - }, - async add(path) { return git('add', '-f', path); }, async commit(message) { return git('commit', '--message', message); }, - async tag(name) { - return git('tag', '-a', `--message=${name}`, name); - }, - async tagName() { - const stdout = await git('tag', '-l', '--points-at', 'HEAD'); - - const tags = stdout.trim().split(/\n|\r\n/); - const versionTags = tags.filter(tag => /^v/.test(tag)); - - if (versionTags[0] != null) { - return versionTags; - } - return tags[0]; - } + git // visible for testing }; +async function getHeadSha() { + const stdout = await git('rev-parse', '--short', 'HEAD'); + return stdout.trim(); +} + +async function getMasterSha() { + try { + const stdout = await git('rev-parse', '--short', 'origin/master'); + return stdout.trim(); + } catch (error) { + if (/Needed a single revision/.test(error.message)) { + // Master was not checked out but in this case, so we know we are not master. We can ignore this + return ''; + } + throw error; + } +} + +async function getTagName() { + const stdout = await git('tag', '-l', '--points-at', 'HEAD'); + const trimmedStdout = stdout.trim(); + if (trimmedStdout === '') { + return null; // there is no tag + } + + const tags = trimmedStdout.split(/\n|\r\n/); + const versionTags = tags.filter(tag => /^v/.test(tag)); + if (versionTags[0] != null) { + return versionTags[0]; + } + return tags[0]; +} + async function git(...args) { return new Promise((resolve, reject) => childProcess.execFile('git', args, (err, stdout) => {