test(cls): fix flaky test and remove relevant tracing code (#12165)

lib/hooks.js

@@ -33,7 +33,6 @@ const hookTypes = {
   beforeFindAfterOptions: { params: 1 },
   afterFind: { params: 2 },
   beforeCount: { params: 1 },
-  transactionCreated: { params: 1, sync: true, noModel: true },
   beforeDefine: { params: 2, sync: true, noModel: true },
   afterDefine: { params: 1, sync: true, noModel: true },
   beforeInit: { params: 2, sync: true, noModel: true },

lib/sequelize.js

@@ -644,11 +644,11 @@ class Sequelize {
       const query = new this.dialect.Query(connection, this, options);
 
       try {
-        await this.runHooks('beforeQuery', options, query, sql);
+        await this.runHooks('beforeQuery', options, query);
         checkTransaction();
         return await query.run(sql, bindParameters);
       } finally {
-        await this.runHooks('afterQuery', options, query, sql);
+        await this.runHooks('afterQuery', options, query);
         if (!options.transaction) {
           await this.connectionManager.releaseConnection(connection);
         }
@@ -1100,7 +1100,6 @@ class Sequelize {
     }
 
     const transaction = new Transaction(this, options);
-    this.runHooks('transactionCreated', transaction);
 
     if (!autoCallback) {
       await transaction.prepareEnvironment(false);

test/integration/cls.test.js

@@ -64,23 +64,21 @@ if (current.dialect.supports.transactions) {
         });
       });
 
-      it('does not leak variables to the outer scope', function() {
+      it('does not leak variables to the outer scope', async function() {
         // This is a little tricky. We want to check the values in the outer scope, when the transaction has been successfully set up, but before it has been comitted.
         // We can't just call another function from inside that transaction, since that would transfer the context to that function - exactly what we are trying to prevent;
 
         let transactionSetup = false,
           transactionEnded = false;
 
-        this.sequelize.transaction(() => {
+        const clsTask = this.sequelize.transaction(async () => {
           transactionSetup = true;
-
-          return delay(500).then(() => {
-            expect(this.ns.get('transaction')).to.be.ok;
-            transactionEnded = true;
-          });
+          await delay(500);
+          expect(this.ns.get('transaction')).to.be.ok;
+          transactionEnded = true;
         });
 
-        return new Promise(resolve => {
+        await new Promise(resolve => {
           // Wait for the transaction to be setup
           const interval = setInterval(() => {
             if (transactionSetup) {
@@ -88,22 +86,19 @@ if (current.dialect.supports.transactions) {
               resolve();
             }
           }, 200);
-        }).then(() => {
-          expect(transactionEnded).not.to.be.ok;
+        });
+        expect(transactionEnded).not.to.be.ok;
 
-          expect(this.ns.get('transaction')).not.to.be.ok;
+        expect(this.ns.get('transaction')).not.to.be.ok;
 
-          // Just to make sure it didn't change between our last check and the assertion
-          expect(transactionEnded).not.to.be.ok;
-        });
+        // Just to make sure it didn't change between our last check and the assertion
+        expect(transactionEnded).not.to.be.ok;
+        await clsTask; // ensure we don't leak the promise
       });
 
-      it('does not leak variables to the following promise chain', function() {
-        return this.sequelize.transaction(() => {
-          return Promise.resolve();
-        }).then(() => {
-          expect(this.ns.get('transaction')).not.to.be.ok;
-        });
+      it('does not leak variables to the following promise chain', async function() {
+        await this.sequelize.transaction(() => {});
+        expect(this.ns.get('transaction')).not.to.be.ok;
       });
 
       it('does not leak outside findOrCreate', function() {

test/integration/support.js

@@ -2,29 +2,14 @@
 
 const Support = require('../support');
 
-const runningQueries = new Map();
-const runningTransactions = new Map(); // map transaction option to queries.
+const runningQueries = new Set();
 
 before(function() {
-  this.sequelize.addHook('transactionCreated', t => { // tracking race condition, remove me if no longer present.
-    t.trace = new Error().stack;
+  this.sequelize.addHook('beforeQuery', (options, query) => {
+    runningQueries.add(query);
   });
-  this.sequelize.addHook('beforeQuery', (options, query, sql) => {
-    runningQueries.set(query, options);
-    if (options.transaction) {
-      const queryList = runningTransactions.get(options.transaction.id);
-      if (queryList) {
-        queryList.push(sql);
-      } else {
-        runningTransactions.set(options.transaction.id, [sql]);
-      }
-    }
-  });
-  this.sequelize.addHook('afterQuery', (options, query, sql) => {
+  this.sequelize.addHook('afterQuery', (options, query) => {
     runningQueries.delete(query);
-    if (options.transaction && ['COMMIT', 'ROLLBACK'].includes(sql)) {
-      runningTransactions.delete(options.transaction.id);
-    }
   });
 });
 
@@ -38,17 +23,7 @@ afterEach(function() {
   }
   let msg = `Expected 0 running queries. ${runningQueries.size} queries still running in ${this.currentTest.fullTitle()}\n`;
   msg += 'Queries:\n\n';
-  for (const [query, options] of runningQueries) {
-    msg += `${query.uuid}: ${query.sql}\n`;
-    if (options.transaction) {
-      const relatedTransaction = runningTransactions.get(options.transaction.id);
-      if (relatedTransaction) {
-        msg += options.transaction.trace;
-        msg += 'In transaction:\n\n';
-        msg += relatedTransaction.join('\n');
-      }
-    }
-  }
+  msg += [...runningQueries].map(query => `${query.uuid}: ${query.sql}`).join('\n');
   throw new Error(msg);
 });