Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

Tests fail drastically when collection hooks is added #19

Closed mitar closed 10 years ago

mitar commented 10 years ago

Meteor official tests fail drastically when collection hooks package is added. This simply means that collection hooks are not suitable for production. Tests should not be failing. Using a default project created by meteor create, adding packages:

Run mrt test-packages and open browser. You will get a long list of tests which failed:

tinytest - accounts - expire numeric token
tinytest - passwords - basic login with password
tinytest - passwords - plain text passwords
tinytest - passwords - changing passwords
tinytest - passwords - new user hooks
tinytest - passwords - Meteor.user()
tinytest - passwords - allow rules
tinytest - passwords - tokens
tinytest - accounts emails - reset password flow
tinytest - accounts emails - verify email flow
tinytest - accounts emails - enroll account flow
tinytest - collection - update options, STRING
tinytest - collection - update options, MONGO

Without this package added tests work correctly.

mizzao commented 10 years ago

Note - this might have to do with #11, but I haven't got around to a full debugging yet, unfortunately.

What version of collection-hooks did you test with?

mitar commented 10 years ago

The one I got from Atmosphere.

mizzao commented 10 years ago

Well @matb33, at least this means we get a lot of tests for free, right? :)

matb33 commented 10 years ago

Sounds very specific to the Meteor.users collection, which is problematic to override in the first place because its creation is hardcoded. I do have a way of making it work somewhat, but I have a feeling the "somewhat" is related to the tests failing.

@mitar out of curiosity, is your app experiencing issues or just tests failing?

matb33 commented 10 years ago

The tests were failing because of two things. First, the mutation of the _id property on the document during the after insert hook (meaning two subsequent inserts of the same document that didn't specify an _id would fail in minimongo because the second insert had the _id attached to it by collection hooks).

Second, the collection hooks tests were not cleaning up after themselves, i.e. leaving before hooks on Meteor.users, which would cause some errors when Meteor ran their own tests against Meteor.users (firing those same hooks again but totally out of context...)

mitar commented 10 years ago

Great! Hurray for tests! :-)

matb33 commented 10 years ago

Well I can't release to Atmosphere -- my password doesn't work anymore (and I use LastPass so I'm not exactly making typos...) and there's no password reset flow available. Can't wait for Atmosphere v2 now...

mitar commented 10 years ago

@tmeasday, can you help here? @matb33 forgot the password for Atmosphere. :-(

matb33 commented 10 years ago

Well I managed to guess it -- it was "password", nice. Now Atmosphere tells me it's not my package haha... Man... I must have two accounts, that's the most logical explanation.

matb33 commented 10 years ago

@tmeasday Sorry to bother you about this, but I just can't figure out what other user I would have created this package under. I don't recall creating multiple accounts, but that's the only explanation I can think of.

tmeasday commented 10 years ago

Hey @matb33 - the username is matb33+atmosphere@gmail.com. Let me know how you go, I can change the username or password.

matb33 commented 10 years ago

I feel like such a noob asking for a password reset... @tmeasday, if you could kindly reset my password (email me whatever you choose over at matb33@gmail.com) I would appreciate it very much!

matb33 commented 10 years ago

Meteor tests are failing again after I added a few long-outstanding features/fixes. It's strange that they are failing -- it could be the way I've been writing tests with Tinytest (it's picky...) but I want to be sure, so re-opening and will continue to debug

rclai commented 9 years ago

Lol, this thread was funny to read.

mitar commented 9 years ago

Ah, tests. We developed this package to make such tests easier. It wraps Tinytest with helpful stuff. The main one I think for any database-based package is that you can interleave server and client test. You make something on the client, check on the server, go back to client, etc.

mizzao commented 9 years ago

I have a similar approach to that for setup/teardown methods, for example:

https://github.com/HarvardEconCS/turkserver-meteor/blob/master/tests/utils.coffee#L26

Will take a look at your package and see if it's worth replacing my ad hoc wrapper.