search

othiym23 / node-continuation-local-storage

implementation of https://github.com/joyent/node/issues/5243
BSD 2-Clause "Simplified" License
1.13k stars 207 forks source link

Fix broken tests #83

Closed Qard closed 7 years ago

Qard commented 8 years ago

This fixes a few minor problems with the test suite.

Qard commented 8 years ago

The final failing test appears to be an issue with async-listener, so I'll leave this open for now while I look into that.

DeTeam commented 7 years ago

@Qard any success with async-listener investigation?

othiym23 commented 7 years ago

Landed as 33d2529cadda8661d0310380d1a951f2677cae1f, although I might further tweak to remove

diff --git a/context.js b/context.js
index 7eb1cab..e30bb80 100644
--- a/context.js
+++ b/context.js
@@ -17,7 +17,7 @@ if (!process.addAsyncListener) require('async-listener');
 function Namespace(name) {
   this.name   = name;
   // changed in 2.7: no default context
-  this.active = null;
+  this.active = Object.create(null);
   this._set   = [];
   this.id     = null;
 }

@Qard, do you remember why this change was necessary? The comment right above that line seems to contradict the change.

Qard commented 7 years ago

To be honest, I don't entirely remember. I seem to recall something about some weirdness due to the this._set.push(this.active) in enter() pushing nulls into the list, but I don't remember why that was an issue. Looking at it now, I don't see any reason it can't just be null, assuming the tests pass.

othiym23 commented 7 years ago

Reverted in cfe82da. Thanks for the explanation, @Qard!