Meteor-Community-Packages / meteor-publication-collector

Test a Meteor publication by collecting its output.
https://atmospherejs.com/johanbrook/publication-collector
MIT License
33 stars 20 forks source link

handle publication with no return value or that returns empty array #29

Closed ziedmahdi closed 7 years ago

ziedmahdi commented 7 years ago

This should fix #28

johanbrook commented 7 years ago

Great, thanks!

nkahnfr commented 7 years ago

Hi,

In my case this PR creates a bug rather than fixing one.

Moving the call to this.ready() outside the if block processing the case a publication is returning one or more cursors can break the workflow of publications using low-level interface (added/changed/removed). That kind of publications must call ready once the initial record set is complete and if a call to ready is made before the publication has done its job related test won't pass.

The initial code was a taken directly from the Meteor source code handling publications and I think should remain as it was to be compliant with the behavior described in Meteor documentation.

Please consider rollbacking that PR. (Personally I had to rollback to v1.0.7 to get my application tests worked again.) See my comment in #28 for a simple fix.

Thanks.

ziedmahdi commented 7 years ago

@nkahnfr where is your comment?

nkahnfr commented 7 years ago

@ziedmahdi Just posted.