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

collector.collect() doesn't throw in 1.1.0 #36

Open markreid opened 6 years ago

markreid commented 6 years ago

Hi,

We've just updated Publication Collector to 1.1.0 and it's broken a few of our tests. Given that you've added support for Promises, I'd say this is probably something you'd consider "expected behaviour", but I was thinking you might want to update the changelog/history to note that it's a potentially breaking change.

Example of the problem:

// FooBarQux is a publication that has a check(argument, String) call for type-checking arguments
describe('FooBarQux subscription', () => {
    it('Takes a String argument', () => {
      const collector = new PublicationCollector();
      expect(() => collector.collect('OrganisationDetail'))
      .to.throw(/Match error/);

      expect(() => collector.collect('FooBarQux', {
        foo: 'bar',
      }))
      .to.throw(/Match error/);
    });
  });

This test has regressed because collector.collect() no longer re-throws errors that are thrown by the publication; instead it returns a promise and rejects it.

This is obviously expected behaviour and we've updated our test suite accordingly, but I can only assume that anybody else testing for publication errors has been doing something similar, so I'd argue that this is potentially a breaking change?

johanbrook commented 6 years ago

Hi @markreid. Thanks for reporting this in, and apologies for not noting this more clearly in the README. I'll fix that.

I believe the Meteor version resolver tool is quite conservative when it comes to updating packages, so most people will probably still be on existing versions of this package unless explicitly updated. If people don't wanna update their test code, they can still use an older version of this package.

Thanks again!

jankapunkt commented 6 years ago

Hi I ran over the same issue (I usually update all packages when there is a breaking change in the meteor core as in 1.6.1)

It broke all my tests that expected to throw on invalid parameters. For me it would be important to know why this is the new behavior (rejecting the promise) and how I can adapt my tests to that, so that I also get rid of the warnings:

UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2):#

Edit:

Just re-checked your REAMDE again, got it now.