Fix some promise problems

* new Promise constructor antipattern around promises * unhandled rejection caused by calling `end()` on mocked client
ab-pm · Nov 8, 2019 · 5cdb2b6 · 5cdb2b6
1 parent 5683dce
commit 5cdb2b6
Show file tree

Hide file tree

Showing 18 changed files with 164 additions and 211 deletions.
diff --git a/docs/migrations.md b/docs/migrations.md
@@ -41,7 +41,7 @@ Another way how to perform some async operation is to return [Promise](https://p
 
 ```javascript
 exports.up = function(pgm) {
-  return new Promise(resolve => {
+  return new Promise((resolve, reject) => {
     // doSomethingAsync
     resolve();
   });

diff --git a/test/db-test.js b/test/db-test.js
@@ -44,7 +44,7 @@ describe('lib/db', () => {
     });
 
     it('pg.Client should be called with connection string', () => {
-      sandbox.stub(pgMock, 'Client');
+      sandbox.stub(pgMock, 'Client').returns({ end() {} });
       db = Db('connection_string');
       expect(pgMock.Client).to.be.calledWith('connection_string');
     });

diff --git a/test/migrations/005_table_test.js b/test/migrations/005_table_test.js
@@ -3,42 +3,41 @@ const table = require('./004_table');
 const schema = process.env.SCHEMA || 'public';
 
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select(
-        `SELECT obj_description(c.oid) as "comment"
-                FROM pg_class c join pg_namespace n ON (c.relnamespace = n.oid)
-                WHERE c.relname = 't2' and c.relkind = 'r' and n.nspname = '${schema}'`
-      )
-      .then(([{ comment }]) =>
-        comment === table.comment ? null : reject(new Error('Comment not set'))
-      )
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_reference;')
-          .then(() => pgm.db.query('INSERT INTO t2(id2) VALUES (1);'))
-          .then(() => reject(new Error('Missing reference clause')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_reference;'))
-      )
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_not_null;')
-          .then(() =>
-            pgm.db.query('INSERT INTO t1(created) VALUES (current_timestamp); ')
-          )
-          .then(() => reject(new Error('Missing not null clause')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_not_null;'))
-      )
-      .then(() =>
-        pgm.db.query(
-          "INSERT INTO t1(string) VALUES ('something') RETURNING id;"
+  pgm.db
+    .select(
+      `SELECT obj_description(c.oid) as "comment"
+              FROM pg_class c join pg_namespace n ON (c.relnamespace = n.oid)
+              WHERE c.relname = 't2' and c.relkind = 'r' and n.nspname = '${schema}'`
+    )
+    .then(([{ comment }]) => {
+      if (comment !== table.comment) throw new Error('Comment not set');
+    })
+    .then(() =>
+      pgm.db
+        .query('SAVEPOINT sp_reference;')
+        .then(() => pgm.db.query('INSERT INTO t2(id2) VALUES (1);'))
+        .then(
+          () => Promise.reject(new Error('Missing reference clause')),
+          () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_reference;')
         )
-      )
-      .then(({ rows: [{ id }] }) =>
-        pgm.db.query(`INSERT INTO t2(id2) VALUES (${id});`)
-      )
-      .then(resolve)
-  );
+    )
+    .then(() =>
+      pgm.db
+        .query('SAVEPOINT sp_not_null;')
+        .then(() =>
+          pgm.db.query('INSERT INTO t1(created) VALUES (current_timestamp);')
+        )
+        .then(
+          () => Promise.reject(new Error('Missing not null clause')),
+          () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_not_null;')
+        )
+    )
+    .then(() =>
+      pgm.db.query("INSERT INTO t1(string) VALUES ('something') RETURNING id;")
+    )
+    .then(({ rows: [{ id }] }) =>
+      pgm.db.query(`INSERT INTO t2(id2) VALUES (${id});`)
+    );
 
 exports.down = pgm => {
   pgm.sql('DELETE from t2');

diff --git a/test/migrations/010_column_test.js b/test/migrations/010_column_test.js
@@ -1,22 +1,23 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_check;')
-          .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (1);'))
-          .then(() => reject(new Error('Missing check clause')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;'))
-      )
-      .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (20);'))
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_unique;')
-          .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (20);'))
-          .then(() => reject(new Error('Missing not unique clause')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_unique;'))
-      )
-      .then(resolve)
-  );
+  Promise.resolve()
+    .then(() =>
+      pgm.db
+        .query('SAVEPOINT sp_check;')
+        .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (1);'))
+        .then(
+          () => Promise.reject(new Error('Missing check clause')),
+          () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;')
+        )
+    )
+    .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (20);'))
+    .then(() =>
+      pgm.db
+        .query('SAVEPOINT sp_unique;')
+        .then(() => pgm.db.query('INSERT INTO t1(nr) VALUES (20);'))
+        .then(
+          () => Promise.reject(new Error('Missing not unique clause')),
+          () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_unique;')
+        )
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/013_column_alter_test.js b/test/migrations/013_column_alter_test.js
@@ -1,14 +1,10 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_smallint;')
-          .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (2147483647);'))
-          .then(() => reject(new Error('Type not updated')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_smallint;'))
-      )
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_smallint;')
+    .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (2147483647);'))
+    .then(
+      () => Promise.reject(new Error('Type not updated')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_smallint;')
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/015_add_constraint_test.js b/test/migrations/015_add_constraint_test.js
@@ -1,15 +1,11 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_check;')
-          .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (30);'))
-          .then(() => reject(new Error('Missing check clause')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;'))
-      )
-      .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (21);'))
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_check;')
+    .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (30);'))
+    .then(
+      () => Promise.reject(new Error('Missing check clause')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;')
+    )
+    .then(() => pgm.db.query('INSERT INTO t1(nmbr) VALUES (21);'));
 
 exports.down = () => null;
diff --git a/test/migrations/026_set_type_attribute_test.js b/test/migrations/026_set_type_attribute_test.js
@@ -1,14 +1,10 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_smallint;')
-          .then(() => pgm.db.query("select (ROW(2147483647, 'x')::obj).id;"))
-          .then(() => reject(new Error('Type not updated')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_smallint;'))
-      )
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_smallint;')
+    .then(() => pgm.db.query("select (ROW(2147483647, 'x')::obj).id;"))
+    .then(
+      () => Promise.reject(new Error('Type not updated')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_smallint;')
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/032_drop_type_attribute_test.js b/test/migrations/032_drop_type_attribute_test.js
@@ -1,14 +1,10 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_attr;')
-          .then(() => pgm.db.query("select (ROW(1, 'x')::obj).str;"))
-          .then(() => reject(new Error('Attribute was not removed')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_attr;'))
-      )
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_attr;')
+    .then(() => pgm.db.query("select (ROW(1, 'x')::obj).str;"))
+    .then(
+      () => Promise.reject(new Error('Attribute was not removed')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_attr;')
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/034_drop_type_test.js b/test/migrations/034_drop_type_test.js
@@ -1,16 +1,12 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_drop;')
-          .then(() =>
-            pgm.db.query('CREATE TEMPORARY TABLE t_list_3 (l list_for_drop);')
-          )
-          .then(() => reject(new Error('Type was not removed')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_drop;'))
-      )
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_drop;')
+    .then(() =>
+      pgm.db.query('CREATE TEMPORARY TABLE t_list_3 (l list_for_drop);')
+    )
+    .then(
+      () => Promise.reject(new Error('Type was not removed')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_drop;')
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/041_function_test.js b/test/migrations/041_function_test.js
@@ -1,10 +1,6 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select('SELECT add(1,2) as r')
-      .then(([{ r }]) =>
-        r === 3 ? resolve() : reject(new Error('Function does not work'))
-      )
-  );
+  pgm.db.select('SELECT add(1,2) as r').then(([{ r }]) => {
+    if (r !== 3) throw new Error('Function does not work');
+  });
 
 exports.down = () => null;
diff --git a/test/migrations/044_trigger_test.js b/test/migrations/044_trigger_test.js
@@ -1,10 +1,8 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select('INSERT INTO tt (a) VALUES (1) RETURNING a')
-      .then(([{ a }]) =>
-        a === 2 ? resolve() : reject(new Error('Trigger does not work'))
-      )
-  );
+  pgm.db
+    .select('INSERT INTO tt (a) VALUES (1) RETURNING a;')
+    .then(([{ a }]) => {
+      if (a !== 2) throw new Error('Trigger does not work');
+    });
 
 exports.down = () => null;
diff --git a/test/migrations/047_domain_check.js b/test/migrations/047_domain_check.js
@@ -1,14 +1,10 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.resolve()
-      .then(() =>
-        pgm.db
-          .query('SAVEPOINT sp_check;')
-          .then(() => pgm.db.query('INSERT INTO td (d) VALUES (11);'))
-          .then(() => reject(new Error('Check on domain was not set')))
-          .catch(() => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;'))
-      )
-      .then(resolve)
-  );
+  pgm.db
+    .query('SAVEPOINT sp_check;')
+    .then(() => pgm.db.query('INSERT INTO td (d) VALUES (11);'))
+    .then(
+      () => Promise.reject(new Error('Check on domain was not set')),
+      () => pgm.db.query('ROLLBACK TO SAVEPOINT sp_check;')
+    );
 
 exports.down = () => null;
diff --git a/test/migrations/050_sequence_test.js b/test/migrations/050_sequence_test.js
@@ -1,10 +1,8 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select('INSERT INTO ts DEFAULT VALUES RETURNING id;')
-      .then(([{ id }]) =>
-        id === 10 ? resolve() : reject(new Error('Bad sequence value'))
-      )
-  );
+  pgm.db
+    .select('INSERT INTO ts DEFAULT VALUES RETURNING id;')
+    .then(([{ id }]) => {
+      if (id !== 10) throw new Error('Bad sequence value');
+    });
 
 exports.down = () => null;
diff --git a/test/migrations/052_sequence_alter_test.js b/test/migrations/052_sequence_alter_test.js
@@ -1,10 +1,8 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select('INSERT INTO ts DEFAULT VALUES RETURNING id;')
-      .then(([{ id }]) =>
-        id === 20 ? resolve() : reject(new Error('Bad sequence value'))
-      )
-  );
+  pgm.db
+    .select('INSERT INTO ts DEFAULT VALUES RETURNING id;')
+    .then(([{ id }]) => {
+      if (id !== 20) throw new Error('Bad sequence value');
+    });
 
 exports.down = () => null;
diff --git a/test/migrations/055_operator_test.js b/test/migrations/055_operator_test.js
@@ -1,10 +1,8 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    pgm.db
-      .select('SELECT ROW(1,2)::complex + ROW(3,4)::complex as sum;')
-      .then(([{ sum }]) =>
-        sum === '(4,6)' ? resolve() : reject(new Error('Bad sequence value'))
-      )
-  );
+  pgm.db
+    .select('SELECT ROW(1,2)::complex + ROW(3,4)::complex as sum;')
+    .then(([{ sum }]) => {
+      if (sum !== '(4,6)') throw new Error('Bad sequence value');
+    });
 
 exports.down = () => null;
diff --git a/test/migrations/058_policy_test.js b/test/migrations/058_policy_test.js
@@ -1,24 +1,19 @@
 exports.up = pgm =>
-  new Promise((resolve, reject) =>
-    Promise.all([
-      pgm.db.query("INSERT INTO tp(user_name) VALUES ('admin');"),
-      pgm.db.query("INSERT INTO tp(user_name) VALUES ('alice');"),
-      pgm.db.query("INSERT INTO tp(user_name) VALUES ('bob');")
-    ])
-      .then(() => pgm.db.query('set role admin;'))
-      .then(() => pgm.db.select('SELECT * FROM tp;'))
-      .then(
-        ({ length }) =>
-          length === 3 || reject(new Error('Policy is not enforced'))
-      )
-      .then(() => pgm.db.query('set role alice;'))
-      .then(() => pgm.db.select('SELECT * FROM tp;'))
-      .then(
-        ({ length }) =>
-          length === 1 || reject(new Error('Policy is not enforced'))
-      )
-      .then(() => pgm.db.query('reset role;'))
-      .then(resolve)
-  );
+  Promise.all([
+    pgm.db.query("INSERT INTO tp(user_name) VALUES ('admin');"),
+    pgm.db.query("INSERT INTO tp(user_name) VALUES ('alice');"),
+    pgm.db.query("INSERT INTO tp(user_name) VALUES ('bob');")
+  ])
+    .then(() => pgm.db.query('set role admin;'))
+    .then(() => pgm.db.select('SELECT * FROM tp;'))
+    .then(({ length }) => {
+      if (length !== 3) throw new Error('Policy is not enforced');
+    })
+    .then(() => pgm.db.query('set role alice;'))
+    .then(() => pgm.db.select('SELECT * FROM tp;'))
+    .then(({ length }) => {
+      if (length !== 1) throw new Error('Policy is not enforced');
+    })
+    .then(() => pgm.db.query('reset role;'));
 
 exports.down = () => null;