.each fails bluebird specs

[Nick McCready]

So I have been testing this lib and .each I think is outdated and reflected to how bluebird used to be. Anyway .each should force order.

    it("waits for returned promise before proceeding next", function() {
        var a = [promised(1), promised(2), promised(3)];
        var b = [];
        return Promise.resolve(a).each(function(value) {
            b.push(value);
            return Promise.delay(1).then(function(){
                b.push(value*2);
            });
        }).then(function(ret) {
            assert.deepEqual(b, [1,2,2,4,3,6]);
        });
    });

Anyway I got it to pass this spec and others by doing the following decorator on $q.

$delegate.each = (collection, cb) ->
        if !collection?.length
          return $delegate.resolve(collection)

        promises = [$delegate.resolve()]
        d = $delegate.defer()

        #to keep k, v integrity as _.sortBy removes keys
        collection = _.map collection, (v,k) -> {v,k}
        #sortBy keys
        invertedVals = _.sortBy(collection, (v, k) -> -1 * k)

        doNext = (index) ->
          if invertedVals.length
            obj = invertedVals.pop()
            $log.debug -> "doNext"
            $log.debug -> obj
            return next(obj, index)

          return $delegate.all(promises).then(d.resolve, d.reject)


        next = (obj, index) ->
          p = promises[index].then ->
            $log.debug -> "should resolve"
            $log.debug -> obj.v
            obj.v
          .then (v) ->
            $log.debug -> "resolved: #{v}"
            cb(v,obj.k, collection.length) #this could be a promise so call it prior to next step
          .then (res) ->
            doNext(index++)
            res

          promises.push(p)
          return p

        doNext(0)
        return d.promise

[Quentin Raynaud]

Already resolved by #3

[Quentin Raynaud]

By the way no you are correct, .each does not force order in our implementation for performance reasons. You can retrieve ordering intel using the arguments of your callback (value, index, length). Since you can use reduce to get a much more usable serialized each I never understood why this is the default behavior on bluebird.

Your example would be a lot more readable if you wrote :

Promise.reduce([promised(1), promised(2), promised(3)], function(acc, value) {
  acc.push(value);
  return Promise.delay(1).then(function() {
    acc.push(value * 2);
  }).return(acc);
}, []);

[Quentin Raynaud]

You can easily write a simpler .each passing your spec using this base :

$q.each = function(collection, cb) {
  return $q.reduce(collection, (acc, ...args) => cb.call(this, ...args), null).return(collection);
});

[Nick McCready]

The reason for forcing order is that you actually want your promises to happen in serial. It is still async overall but one promise can not happen/complete before the other.

[Quentin Raynaud]

I understand the reason, it's the API choice I do not agree with (like at all). map is not doing this and thus each should not either. Moreover, this each implementation is making double usage with reduce, at least according to me. Everyone expects each do be semantically identical to map except one is creating a new array and the other is not.

I was using each to do things like : myPromises.each(item => item.save()) and it is not doing at all what you would expect it to do after using map. I want to wait for each save but I want them to execute in parallel. Therefore you have to hack your way using map to achieve this : myPromises.map(item => item.save().return(item)). This is hacky and this looks positively bad.

But anyway, you can use my one liner to patch our implementation to get something complying to your spec. It will be a lot easier to maintain than your version ^^.

[Nick McCready]

I tried this and this still does not work. As the underlying reduce, and join do not do anything sequential either. I guess this is where we disagree.

$q.each = function(collection, cb) {
  return $q.reduce(collection, (acc, ...args) => cb.call(this, ...args), null).return(collection);
});

[Quentin Raynaud]

You are obviously wrong. Reduce is sequential by definition (since it needs the value of the previous run to execute the next one).

[Nick McCready]

Why are you throwing out slander? I am just trying to have a discussion. Anyway, I copied your code and the tests still fail. So saying I am obviously wrong gets nobody anywhere.

[Quentin Raynaud]

What tests ?

[Nick McCready]

Running "karma:functionals1_3" (karma) task
16 12 2016 12:42:44.169:INFO [karma]: Karma v1.3.0 server started at http://localhost:9876/
16 12 2016 12:42:44.169:INFO [launcher]: Launching browsers PhantomJS, Chrome with unlimited concurrency
16 12 2016 12:42:44.171:INFO [launcher]: Starting browser PhantomJS
16 12 2016 12:42:44.177:INFO [launcher]: Starting browser Chrome
16 12 2016 12:42:45.224:INFO [Chrome 55.0.2883 (Mac OS X 10.12.2)]: Connected on socket /#O0PwR8qw5paClS-QAAAC with id 7221837
Chrome 55.0.2883 (Mac OS X 10.12.2) newq.each should execute the callback for each element in order of resolution FAILED
    AssertionError: expected 4 to equal 0
        at tests/functionals/newq/each.js:22:25
        at dist/angular-extend-promises.js:5361:18
        at Promise.<anonymous> (dist/angular-extend-promises.js:5380:15)
        at Promise.<anonymous> (dist/angular-extend-promises.js:5223:16)
        at processQueue (bower_components/angular1.3/angular.js:13318:27)
        at bower_components/angular1.3/angular.js:13334:27
        at Scope.$eval (bower_components/angular1.3/angular.js:14570:28)
        at Scope.$digest (bower_components/angular1.3/angular.js:14386:31)
        at Object.play (tests/functionals/utils.js:31:22)
        at Context.<anonymous> (tests/functionals/newq/each.js:30:11)
Chrome 55.0.2883 (Mac OS X 10.12.2) newq.each should ignore the value returned by the callback FAILED
    AssertionError: expected [ Array(3) ] to deeply equal [ 4, 0, 2 ]
        at Assertion.assertEqual (node_modules/chai/chai.js:774:19)
        at Assertion.ctx.(anonymous function) (node_modules/chai/chai.js:4192:25)
        at Promise.<anonymous> (node_modules/chai-as-promised/lib/chai-as-promised.js:308:26)
        at processQueue (bower_components/angular1.3/angular.js:13318:27)
        at bower_components/angular1.3/angular.js:13334:27
        at Scope.$eval (bower_components/angular1.3/angular.js:14570:28)
        at Scope.$digest (bower_components/angular1.3/angular.js:14386:31)
        at Context.<anonymous> (tests/functionals/newq/each.js:41:16)
Chrome 55.0.2883 (Mac OS X 10.12.2): Executed 30 of 94 (2 FAILED) (0 secs / 0.059 secs)
PhantomJS 2.1.1 (Mac OS X 0.0.0) newq.each should execute the callback for each element in order of resolution FAILED
    expected 4 to equal 0
PhantomJS 2.1.1 (Mac OS X 0.0.0) newq.each should ignore the value returned by the callback FAILED
    expected [ Array(3) ] to deeply equal [ 4, 0, 2 ]
Chrome 55.0.2883 (Mac OS X 10.12.2) Promise.each should flow promises down each asynchronously after a newq.each FAILED
    Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
PhantomJS 2.1.1 (Mac OS X 0.0.0) Promise.each should flow promises down each asynchronously after a newq.each FAILED
    Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
Chrome 55.0.2883 (Mac OS X 10.12.2) Promise.each should flow promises down each asynchronously after a promise.each FAILED
    Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
PhantomJS 2.1.1 (Mac OS X 0.0.0) Promise.each should flow promises down each asynchronously after a promise.each FAILED
    Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
Chrome 55.0.2883 (Mac OS X 10.12.2): Executed 94 of 94 (4 FAILED) (4.549 secs / 4.201 secs)
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 94 of 94 (4 FAILED) (4.411 secs / 4.157 secs)
TOTAL: 8 FAILED, 180 SUCCESS
Warning: Task "karma:functionals1_3" failed. Use --force to continue.

[Nick McCready]

ahh i think the test just needs to be changed

[Nick McCready]

it('should execute the callback for each element in order of resolution', function(done) {

    var chain = createPromiseChain(newq);

    var values = chain.resolveNext(2, 4);
    values.splice(1, 0, 0);
    values = values.reverse();

    // values is [Promise(4), 0, Promise(2)]
    expected = [4,0,2];
    expect(newq.each(values, function(val, i) {
      expect(val).to.be.equal(expected[i]);
    }))
      .to.eventually.be.fulfilled
      .and.to.have.property('length', 3)
      .notify(done)
    ;

    chain.play($rootScope);
  });

modded works

[Quentin Raynaud]

This test is designed to check my spec of each yes. Not the bluebird spec. This just reflects that you patched my code. Seems like everything is working like a charm and that my oneliner did work. And I'm sorry if you took my words badly. It was not intended to be a slander. I was just stating what I took for a fact (I don't see being wrong as a problem either, everyone is time to time).

[Quentin Raynaud]

I'm happy to see you got everything sorted out. This lib is not really supported anymore though. I advise you to work on your own fork from now on if you want to make improvements on it since I won't spend more time improving or maintaining it. It seems to work well and it is properly tested so it should not break too easily.

I got out of my way to patch the most obvious quirks to your usage and to help you out. It's now possible for you to achieve correct results in a proper way. Now I'm out :-). I'm pretty sure you can find solutions to patch / improve my work from your project side from here on.