Repository: paularmstrong/build-tracker
Pull request #110 (2020-02-13 18:52:33)

Handle no parent revision
<!--
Thank you so much for contributing to open source and the Build Tracker project!
-->

# Problem

Fixes https://github.com/paularmstrong/build-tracker/issues/107

Implements https://github.com/paularmstrong/build-tracker/issues/107#issuecomment-585867432

# Solution

<!--
When trying to solve more solutions with Build Tracker, please keep in mind some of the following goals of the project:
* Be lightweight: small package sizes (single-digit KiBs, gzipped)
* Be easy: too many options in an API can become confusing
* Be clear: the intended purpose of every method should be as obvious as possible
* Is it easy to do this in "userland"? Would it be better off done there?
-->

Explain your approach. Sometimes it helps to justify your approach against some others that you didn't choose to explain why yours is better.

# TODO

- [x] 🤓 Add & update tests (always try to _increase_ test coverage)
- [x] 🔬 Ensure CI is passing (`yarn lint:ci`, `yarn test`, `yarn tsc:ci`)
- [x] 📖 Update relevant documentation



Patch:
diff --git a/src/cli/src/modules/config.ts b/src/cli/src/modules/config.ts
index 247d807..c8785ff 100644
--- a/src/cli/src/modules/config.ts
+++ b/src/cli/src/modules/config.ts
@@ -35,7 +35,11 @@ export default async function getConfig(path?: string): Promise<Config> {
   try {
     result = !path ? await explorer.search() : await explorer.load(path);
   } catch (e) {
-    throw new Error('Could not find configuration file');
+    if (e.code === 'ENOENT') {
+      throw new Error('Could not find configuration file');
+    } else {
+      throw e;
+    }
   }
 
   return {
diff --git a/src/fixtures/cli-configs/rc/.build-trackerrc-invalid.js b/src/fixtures/cli-configs/rc/.build-trackerrc-invalid.js
new file mode 100644
index 0000000..478d9ec
--- /dev/null
+++ b/src/fixtures/cli-configs/rc/.build-trackerrc-invalid.js
@@ -0,0 +1,1 @@
+throw new Error('test');
\ No newline at end of file
diff --git a/src/server/src/api/insert.ts b/src/server/src/api/insert.ts
index fd188d0..3160582 100644
--- a/src/server/src/api/insert.ts
+++ b/src/server/src/api/insert.ts
@@ -19,17 +19,27 @@ export const insertBuild = (
   queries.insert(build).then(() =>
     queries
       .byRevision(build.getMetaValue('parentRevision'))
-      .then(parentBuildData => {
-        const parentBuild = new Build(parentBuildData.meta, parentBuildData.artifacts);
-        return {
-          comparator: new Comparator({
-            artifactBudgets: artifactConfig.budgets,
-            artifactFilters: artifactConfig.filters,
-            builds: [build, parentBuild],
-            groups: artifactConfig.groups
-          })
-        };
-      })
+      .then(
+        parentBuildData => {
+          const parentBuild = new Build(parentBuildData.meta, parentBuildData.artifacts);
+          return [build, parentBuild];
+        },
+        error => {
+          if (error instanceof NotFoundError) {
+            return [build];
+          } else {
+            throw error;
+          }
+        }
+      )
+      .then(builds => ({
+        comparator: new Comparator({
+          artifactBudgets: artifactConfig.budgets,
+          artifactFilters: artifactConfig.filters,
+          builds,
+          groups: artifactConfig.groups
+        })
+      }))
       .then(context => {
         return onInserted(context.comparator).then(() => context);
       })
@@ -40,10 +50,6 @@ export const insertBuild = (
         });
       })
       .catch(error => {
-        if (error instanceof NotFoundError) {
-          res.send({ error });
-          return;
-        }
         res.send({ error });
       })
   );


Test patch:
diff --git a/src/cli/src/modules/__tests__/config.test.ts b/src/cli/src/modules/__tests__/config.test.ts
index 95d3e36..42165fb 100644
--- a/src/cli/src/modules/__tests__/config.test.ts
+++ b/src/cli/src/modules/__tests__/config.test.ts
@@ -31,6 +31,14 @@ describe('getConfig', () => {
     });
   });
 
+  test('propagate errors', () => {
+    return getConfig(
+      path.join(path.dirname(require.resolve('@build-tracker/fixtures')), 'cli-configs/rc/.build-trackerrc-invalid.js')
+    ).catch(e => {
+      expect(e.message).toMatch('test');
+    });
+  });
+
   test('throws if no configuration found', () => {
     return getConfig('tacos').catch(e => {
       expect(e.message).toMatch('Could not find configuration file');
diff --git a/src/server/src/api/__tests__/insert.test.ts b/src/server/src/api/__tests__/insert.test.ts
index d351fa8..81af5e6 100644
--- a/src/server/src/api/__tests__/insert.test.ts
+++ b/src/server/src/api/__tests__/insert.test.ts
@@ -6,6 +6,7 @@ import Build from '@build-tracker/build';
 import Comparator from '@build-tracker/comparator';
 import express from 'express';
 import { insertBuild } from '../insert';
+import { NotFoundError } from '@build-tracker/api-errors';
 import request from 'supertest';
 
 const build = new Build({ branch: 'master', revision: 'abc', parentRevision: 'def', timestamp: Date.now() }, [
@@ -65,27 +66,103 @@ describe('insert build handler', () => {
     });
   });
 
-  describe('onInserted', () => {
-    test('called with a comparator of the inserted branch against its parentRevision', () => {
-      const handleInsert = jest.fn(() => Promise.resolve());
-      const handler = insertBuild(queries, config, handleInsert);
-      app.post('/test', handler);
+  describe('parent revision', () => {
+    describe('onInserted', () => {
+      test('called with a comparator of the inserted branch against its parentRevision', () => {
+        const handleInsert = jest.fn(() => Promise.resolve());
+        const handler = insertBuild(queries, config, handleInsert);
+        app.post('/test', handler);
 
-      return request(app)
-        .post('/test')
-        .send({ meta: build.meta, artifacts: build.artifacts })
-        .set('Content-Type', 'application/json')
-        .set('Accept', 'application/json')
-        .then(() => {
-          expect(handleInsert).toHaveBeenCalled();
-          // @ts-ignore
-          const [comparator] = handleInsert.mock.calls[0];
-          // @ts-ignore
-          expect(comparator.builds.map(build => build.getMetaValue('revision'))).toEqual([
-            build.getMetaValue('revision'),
-            parentBuild.getMetaValue('revision')
-          ]);
-        });
+        return request(app)
+          .post('/test')
+          .send({ meta: build.meta, artifacts: build.artifacts })
+          .set('Content-Type', 'application/json')
+          .set('Accept', 'application/json')
+          .then(() => {
+            expect(handleInsert).toHaveBeenCalled();
+            // @ts-ignore
+            const [comparator] = handleInsert.mock.calls[0];
+            // @ts-ignore
+            expect(comparator.builds.map(build => build.getMetaValue('revision'))).toEqual([
+              build.getMetaValue('revision'),
+              parentBuild.getMetaValue('revision')
+            ]);
+          });
+      });
+    });
+  });
+
+  describe('no parent revision', () => {
+    let app, config, queries;
+    beforeEach(() => {
+      queries = {
+        byRevision: jest.fn(() => Promise.reject(new NotFoundError())),
+        insert: jest.fn(() => Promise.resolve('1234'))
+      };
+      config = {
+        artifacts: {},
+        queries
+      };
+      app = express();
+      app.use(bodyParser.json());
+    });
+
+    describe('insert', () => {
+      test('inserts the build', () => {
+        const handler = insertBuild(queries, config);
+        app.post('/test', handler);
+
+        return request(app)
+          .post('/test')
+          .send({ meta: build.meta, artifacts: build.artifacts })
+          .set('Content-Type', 'application/json')
+          .set('Accept', 'application/json')
+          .then(() => {
+            expect(queries.insert).toHaveBeenCalled();
+          });
+      });
+    });
+
+    describe('response', () => {
+      test('returns a JSON serialized Comparator and summary', () => {
+        const handler = insertBuild(queries, config);
+        app.post('/test', handler);
+
+        const comparator = new Comparator({ builds: [build] });
+
+        return request(app)
+          .post('/test')
+          .send({ meta: build.meta, artifacts: build.artifacts })
+          .set('Content-Type', 'application/json')
+          .set('Accept', 'application/json')
+          .then(res => {
+            expect(res.body.comparatorData).toEqual(comparator.serialize());
+            expect(res.body.summary).toEqual(comparator.toSummary());
+          });
+      });
+    });
+
+    describe('onInserted', () => {
+      test('called with a comparator of the inserted branch against its parentRevision', () => {
+        const handleInsert = jest.fn(() => Promise.resolve());
+        const handler = insertBuild(queries, config, handleInsert);
+        app.post('/test', handler);
+
+        return request(app)
+          .post('/test')
+          .send({ meta: build.meta, artifacts: build.artifacts })
+          .set('Content-Type', 'application/json')
+          .set('Accept', 'application/json')
+          .then(() => {
+            expect(handleInsert).toHaveBeenCalled();
+            // @ts-ignore
+            const [comparator] = handleInsert.mock.calls[0];
+            // @ts-ignore
+            expect(comparator.builds.map(build => build.getMetaValue('revision'))).toEqual([
+              build.getMetaValue('revision')
+            ]);
+          });
+      });
     });
   });
 });