coopernurse / node-pool

Generic resource pooling for node.js
2.37k stars 259 forks source link

Pool.clear() promise should resolve with no value #252

Closed restjohn closed 5 years ago

restjohn commented 5 years ago

This is a simple one-liner, excepting the associated test. Previously, the Promise that Pool.clear() returned would resolve with an array of undefined values because that Promise was created using Promise.all(), which resolves with an array of all the component Promises' resolved values. This was a minor annoyance in some tape unit tests because I would do something like

test('something with a pool', function(t) {
  // ...
  const pool = genericPool.createPool(/* ... */);
  // ...
  pool.drain().then(() => {
    return pool.clear()
  })
  .then(t.end, t.fail);  
});

The above test would fail because that array of undefined values would be passed to t.end, which fails the test if invoked with a truthy argument. Of course this was easy to work around once I figured out what was actually happening, but in any event, resolving the clear() Promise with an array of undefined just seems undesirable.

restjohn commented 5 years ago

@DiegoRBaquero, I made the changes you suggested and pushed to my fork.

DiegoRBaquero commented 5 years ago

@sandfox

sandfox commented 5 years ago

LGTM! published in v3.6.1