Attempt to streamline dev lifecycle (git hooks) (game-ci#357)

* fix: misalignments in dev lifecycle

* fix: dist no longer added to staged

* fix: misalignments in dev lifecycle

* chore: multi-platform hooks and tests

* chore: multi-platform hooks and tests

* chore: add intention for colors

* chore: update lint-staged to fix color

* chore: update dist files

* feat: move to lefthook (remove husky and lint-staged)

* feat: move to lefthook (remove husky and lint-staged)

* fix: test aftereach

* fix: test aftereach

* fix: early restore call

* feat: jest fails if something gets logged to console

* chore: add todos of misplaced code

* chore: update dist files

* chore: move jest file

jest.config.js

@@ -1,11 +1,30 @@
 module.exports = {
+  // Automatically clear mock calls and instances between every test
   clearMocks: true,
+
+  // An array of file extensions your modules use
   moduleFileExtensions: ['js', 'ts'],
+
+  // The test environment that will be used for testing
   testEnvironment: 'node',
+
+  // The glob patterns Jest uses to detect test files
   testMatch: ['**/*.test.ts'],
+
+  // This option allows use of a custom test runner
   testRunner: 'jest-circus/runner',
+
+  // A map with regular expressions for transformers to paths
   transform: {
     '^.+\\.ts$': 'ts-jest',
   },
+
+  // Indicates whether each individual test should be reported during the run
   verbose: true,
+
+  // An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader
+  modulePathIgnorePatterns: ['<rootDir>/lib/', '<rootDir>/dist/'],
+
+  // A list of paths to modules that run some code to configure or set up the testing framework before each test
+  setupFilesAfterEnv: ['<rootDir>/src/jest.setup.ts'],
 };

lefthook.yml

@@ -0,0 +1,38 @@
+# EXAMPLE USAGE
+# Refer for explanation to following link:
+# https://github.com/evilmartians/lefthook/blob/master/docs/full_guide.md
+#
+
+color: true
+extends: {}
+
+pre-push:
+  parallel: true
+  commands:
+    packages-audit:
+      tags: security
+      run: yarn audit
+
+pre-commit:
+  parallel: true
+  commands:
+    format documents:
+      glob: '*.{md,mdx}'
+      run: yarn prettier --write {staged_files}
+    format configs:
+      glob: '*.{json,yml,yaml}'
+      run: yarn prettier --write {staged_files}
+    format code:
+      glob: '*.{js,jsx,ts,tsx}'
+      exclude: 'dist/'
+      run: yarn prettier --write {staged_files} && yarn eslint {staged_files} && git add {staged_files}
+    run tests:
+      glob: '*.{js,jsx,ts,tsx}'
+      exclude: 'dist/'
+      run: yarn jest --passWithNoTests --findRelatedTests {staged_files}
+    build distributables:
+      skip: ['merge', 'rebase']
+      run: yarn build && git add dist
+    make shell script executable:
+      glob: '*.sh'
+      run: git update-index --chmod=+x

package.json

@@ -7,11 +7,10 @@
   "author": "Webber <[email protected]>",
   "license": "MIT",
   "scripts": {
-    "prebuild": "yarn",
-    "build": "tsc && ncc build lib --source-map --license licenses.txt",
+    "prepare": "lefthook install",
+    "build": "yarn && tsc && ncc build lib --source-map --license licenses.txt",
     "lint": "prettier --check \"src/**/*.{js,ts}\" && eslint src/**/*.ts",
     "format": "prettier --write \"src/**/*.{js,ts}\"",
-    "prepare": "husky install",
     "cli": "yarn ts-node src/index.ts -m cli",
     "cli-aws": "cross-env cloudRunnerCluster=aws yarn run test-cli",
     "cli-k8s": "cross-env cloudRunnerCluster=k8s yarn run test-cli",
@@ -41,6 +40,7 @@
     "yaml": "^1.10.2"
   },
   "devDependencies": {
+    "@arkweid/lefthook": "^0.7.7",
     "@types/jest": "^27.4.1",
     "@types/node": "^17.0.21",
     "@types/semver": "^7.3.9",
@@ -53,27 +53,13 @@
     "eslint-plugin-jest": "24.1.3",
     "eslint-plugin-prettier": "^3.3.1",
     "eslint-plugin-unicorn": "28.0.2",
-    "husky": "^7.0.4",
     "jest": "^27.5.1",
     "jest-circus": "^27.5.1",
+    "jest-fail-on-console": "^2.3.0",
     "js-yaml": "^4.1.0",
-    "lint-staged": "^12.3.4",
     "prettier": "^2.5.1",
     "ts-jest": "^27.1.3",
     "ts-node": "10.4.0",
     "typescript": "4.1.3"
-  },
-  "lint-staged": {
-    "*.{js,jsx,ts,tsx}": [
-      "prettier --write",
-      "eslint",
-      "jest --findRelatedTests"
-    ],
-    "*.{json,md,yaml,yml}": [
-      "prettier --write"
-    ],
-    "*.sh": [
-      "git update-index --chmod=+x"
-    ]
   }
 }

src/jest.setup.ts

@@ -0,0 +1,9 @@
+import failOnConsole from 'jest-fail-on-console';
+
+// Fail when console logs something inside a test - use spyOn instead
+failOnConsole({
+  shouldFailOnWarn: true,
+  shouldFailOnError: true,
+  shouldFailOnLog: true,
+  shouldFailOnAssert: true,
+});

src/model/build-parameters.test.ts

@@ -5,16 +5,15 @@ import BuildParameters from './build-parameters';
 import Input from './input';
 import Platform from './platform';
 
+// Todo - Don't use process.env directly, that's what the input model class is for.
 const testLicense =
   '<?xml version="1.0" encoding="UTF-8"?><root>\n    <License id="Terms">\n        <MachineBindings>\n            <Binding Key="1" Value="576562626572264761624c65526f7578"/>\n            <Binding Key="2" Value="576562626572264761624c65526f7578"/>\n        </MachineBindings>\n        <MachineID Value="D7nTUnjNAmtsUMcnoyrqkgIbYdM="/>\n        <SerialHash Value="2033b8ac3e6faa3742ca9f0bfae44d18f2a96b80"/>\n        <Features>\n            <Feature Value="33"/>\n            <Feature Value="1"/>\n            <Feature Value="12"/>\n            <Feature Value="2"/>\n            <Feature Value="24"/>\n            <Feature Value="3"/>\n            <Feature Value="36"/>\n            <Feature Value="17"/>\n            <Feature Value="19"/>\n            <Feature Value="62"/>\n        </Features>\n        <DeveloperData Value="AQAAAEY0LUJHUlgtWEQ0RS1aQ1dWLUM1SlctR0RIQg=="/>\n        <SerialMasked Value="F4-BGRX-XD4E-ZCWV-C5JW-XXXX"/>\n        <StartDate Value="2021-02-08T00:00:00"/>\n        <UpdateDate Value="2021-02-09T00:34:57"/>\n        <InitialActivationDate Value="2021-02-08T00:34:56"/>\n        <LicenseVersion Value="6.x"/>\n        <ClientProvidedVersion Value="2018.4.30f1"/>\n        <AlwaysOnline Value="false"/>\n        <Entitlements>\n            <Entitlement Ns="unity_editor" Tag="UnityPersonal" Type="EDITOR" ValidTo="9999-12-31T00:00:00"/>\n            <Entitlement Ns="unity_editor" Tag="DarkSkin" Type="EDITOR_FEATURE" ValidTo="9999-12-31T00:00:00"/>\n        </Entitlements>\n    </License>\n<Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments"/><SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/><Reference URI="#Terms"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/></Transforms><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><DigestValue>m0Db8UK+ktnOLJBtHybkfetpcKo=</DigestValue></Reference></SignedInfo><SignatureValue>o/pUbSQAukz7+ZYAWhnA0AJbIlyyCPL7bKVEM2lVqbrXt7cyey+umkCXamuOgsWPVUKBMkXtMH8L\n5etLmD0getWIhTGhzOnDCk+gtIPfL4jMo9tkEuOCROQAXCci23VFscKcrkB+3X6h4wEOtA2APhOY\nB+wvC794o8/82ffjP79aVAi57rp3Wmzx+9pe9yMwoJuljAy2sc2tIMgdQGWVmOGBpQm3JqsidyzI\nJWG2kjnc7pDXK9pwYzXoKiqUqqrut90d+kQqRyv7MSZXR50HFqD/LI69h68b7P8Bjo3bPXOhNXGR\n9YCoemH6EkfCJxp2gIjzjWW+l2Hj2EsFQi8YXw==</SignatureValue></Signature></root>';
 process.env.UNITY_LICENSE = testLicense;
 
 const determineVersion = jest.spyOn(Versioning, 'determineVersion').mockImplementation(async () => '1.3.37');
-
 const determineUnityVersion = jest
   .spyOn(UnityVersioning, 'determineUnityVersion')
   .mockImplementation(() => '2019.2.11f1');
-
 const determineSdkManagerParameters = jest
   .spyOn(AndroidVersioning, 'determineSdkManagerParameters')
   .mockImplementation(() => 'platforms;android-30');

src/model/build-parameters.ts

@@ -55,15 +55,13 @@ class BuildParameters {
 
   static async create(): Promise<BuildParameters> {
     const buildFile = this.parseBuildFile(Input.buildName, Input.targetPlatform, Input.androidAppBundle);
-
     const unityVersion = UnityVersioning.determineUnityVersion(Input.projectPath, Input.unityVersion);
-
     const buildVersion = await Versioning.determineVersion(Input.versioningStrategy, Input.specifiedVersion);
-
     const androidVersionCode = AndroidVersioning.determineVersionCode(buildVersion, Input.androidVersionCode);
-
     const androidSdkManagerParameters = AndroidVersioning.determineSdkManagerParameters(Input.androidTargetSdkVersion);
 
+    // Todo - Don't use process.env directly, that's what the input model class is for.
+    // ---
     let unitySerial = '';
     if (!process.env.UNITY_SERIAL) {
       //No serial was present so it is a personal license that we need to convert
@@ -78,6 +76,7 @@ class BuildParameters {
       unitySerial = process.env.UNITY_SERIAL!;
     }
     core.setSecret(unitySerial);
+    // ---
 
     return {
       version: unityVersion,

src/model/input-readers/git-repo.ts

@@ -7,12 +7,14 @@ export class GitRepoReader {
   static GetSha() {
     return '';
   }
+
   public static async GetRemote() {
     return (await CloudRunnerSystem.Run(`git remote -v`))
       .split(' ')[1]
       .split('https://github.com/')[1]
       .split('.git')[0];
   }
+
   public static async GetBranch() {
     assert(fs.existsSync(`.git`));
     return (await System.run(`git branch`, [], {}, false)).split('*')[1].split(`\n`)[0].replace(/ /g, ``);

src/model/input-readers/github-cli.test.ts

@@ -2,8 +2,11 @@ import { GithubCliReader } from './github-cli';
 import * as core from '@actions/core';
 
 describe(`github cli`, () => {
-  it(`returns`, async () => {
+  // Todo - We can not assume that everyone has the GitHub cli installed locally.
+  it.skip(`returns`, async () => {
     const token = await GithubCliReader.GetGitHubAuthToken();
+
+    // Todo - use expect(result).toStrictEqual(something)
     core.info(token);
   });
 });

src/model/input.ts

@@ -147,15 +147,15 @@ class Input {
   }
 
   static get androidTargetSdkVersion() {
-    return core.getInput('androidTargetSdkVersion') || '';
+    return Input.getInput('androidTargetSdkVersion') || '';
   }
 
   static get sshAgent() {
     return Input.getInput('sshAgent') || '';
   }
 
   static async gitPrivateToken() {
-    return core.getInput('gitPrivateToken') || (await Input.githubToken());
+    return Input.getInput('gitPrivateToken') || (await Input.githubToken());
   }
 
   static get chownFilesTo() {

src/model/system.integration.test.ts

@@ -0,0 +1,46 @@
+import * as core from '@actions/core';
+import System from './system';
+
+jest.spyOn(core, 'debug').mockImplementation(() => {});
+jest.spyOn(core, 'info').mockImplementation(() => {});
+jest.spyOn(core, 'warning').mockImplementation(() => {});
+jest.spyOn(core, 'error').mockImplementation(() => {});
+
+afterEach(() => jest.clearAllMocks());
+
+describe('System', () => {
+  describe('run', () => {
+    /**
+     * Not all shells (e.g. Powershell, sh) have a reference to `echo` binary (absent or alias).
+     * To ensure our integration with '@actions/exec' works as expected we run some specific tests in CI only
+     */
+    describe('integration', () => {
+      if (!process.env.CI) {
+        it("doesn't run locally", () => {
+          expect(true).toBe(true);
+        });
+      } else {
+        it('runs a command successfully', async () => {
+          await expect(System.run('true')).resolves.not.toBeNull();
+        });
+
+        it('outputs results', async () => {
+          await expect(System.run('echo test')).resolves.toStrictEqual('test\n');
+        });
+
+        it('throws on when error code is not 0', async () => {
+          await expect(System.run('false')).rejects.toThrowError();
+        });
+
+        it('allows pipes using buffer', async () => {
+          await expect(
+            System.run('sh', undefined, {
+              input: Buffer.from('git tag --list --merged HEAD | grep v[0-9]* | wc -l'),
+              // eslint-disable-next-line github/no-then
+            }).then((result) => Number(result)),
+          ).resolves.not.toBeNaN();
+        });
+      }
+    });
+  });
+});

src/model/system.test.ts

@@ -1,57 +1,46 @@
 import * as core from '@actions/core';
+import * as exec from '@actions/exec';
 import System from './system';
 
 jest.spyOn(core, 'debug').mockImplementation(() => {});
 const info = jest.spyOn(core, 'info').mockImplementation(() => {});
 jest.spyOn(core, 'warning').mockImplementation(() => {});
 jest.spyOn(core, 'error').mockImplementation(() => {});
+const execSpy = jest.spyOn(exec, 'exec').mockImplementation(async () => 0);
 
-afterEach(() => {
-  jest.clearAllMocks();
-});
+afterEach(() => jest.clearAllMocks());
 
 describe('System', () => {
   describe('run', () => {
-    it('runs a command successfully', async () => {
-      await expect(System.run('true')).resolves.not.toBeNull();
-    });
-
-    it('outputs results', async () => {
-      await expect(System.run('echo test')).resolves.toStrictEqual('test\n');
-    });
-
-    it('throws on when error code is not 0', async () => {
-      await expect(System.run('false')).rejects.toThrowError();
-    });
-
-    it('throws when no arguments are given', async () => {
-      await expect(System.run('')).rejects.toThrowError();
-    });
-
-    it('outputs info', async () => {
-      await expect(System.run('echo test')).resolves.not.toBeNull();
-      expect(info).toHaveBeenLastCalledWith('test\n');
-    });
-
-    it('outputs info only once', async () => {
-      await expect(System.run('echo 1')).resolves.not.toBeNull();
-      expect(info).toHaveBeenCalledTimes(1);
-      expect(info).toHaveBeenLastCalledWith('1\n');
-
-      info.mockClear();
-      await expect(System.run('echo 2')).resolves.not.toBeNull();
-      await expect(System.run('echo 3')).resolves.not.toBeNull();
-      expect(info).toHaveBeenCalledTimes(2);
-      expect(info).toHaveBeenLastCalledWith('3\n');
-    });
-
-    it('allows pipes using buffer', async () => {
-      await expect(
-        System.run('sh', undefined, {
-          input: Buffer.from('git tag --list --merged HEAD | grep v[0-9]* | wc -l'),
-          // eslint-disable-next-line github/no-then
-        }).then((result) => Number(result)),
-      ).resolves.not.toBeNaN();
+    describe('units', () => {
+      it('passes the command to command line', async () => {
+        await expect(System.run('echo test')).resolves.not.toBeNull();
+        await expect(execSpy).toHaveBeenLastCalledWith('echo test', expect.anything(), expect.anything());
+      });
+
+      it('throws on when error code is not 0', async () => {
+        execSpy.mockImplementationOnce(async () => 1);
+        await expect(System.run('false')).rejects.toThrowError();
+      });
+
+      it('throws when no command is given', async () => {
+        await expect(System.run('')).rejects.toThrowError();
+      });
+
+      it('throws when command consists only of spaces', async () => {
+        await expect(System.run(' \t\n')).rejects.toThrowError();
+      });
+
+      it('outputs info', async () => {
+        execSpy.mockImplementationOnce(async (input, _, options) => {
+          options?.listeners?.stdout?.(Buffer.from(input, 'utf8'));
+          return 0;
+        });
+
+        await expect(System.run('foo-bar')).resolves.not.toBeNull();
+        expect(info).toHaveBeenCalledTimes(1);
+        expect(info).toHaveBeenLastCalledWith('foo-bar');
+      });
     });
   });
 });

src/model/system.ts

@@ -45,6 +45,10 @@ class System {
     };
 
     try {
+      if (command.trim() === '') {
+        throw new Error(`Failed to execute empty command`);
+      }
+
       const exitCode = await exec(command, arguments_, { silent: true, listeners, ...options });
       showOutput();
       if (exitCode !== 0) {