Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change collection.drop() to collection.deleteMany() #39

Merged
merged 1 commit into from
Jun 27, 2017

Conversation

bernardofd
Copy link
Contributor

The collection.drop() method completely erases a collection from the database, including its indexes. It's better to only remove the documents but keep the indexes, since the user might use them in their tests. The method in mongodb-native v2.0 that allows just that is collection.deleteMany(). This is a correction of PR #31, since there is no removeMany() method on the collection API

The `collection.drop()` method completely erases a collection from the database, including its indexes. It's better to only remove the documents but keep the indexes, since the user might use them. The method in mongodb-native that allows just that is [`collection.deleteMany()`](http://mongodb.github.io/node-mongodb-native/2.0/api/Collection.html#deleteMany)
@markstos
Copy link

I ran into test failures because of mixing indexes, so I agree something needs to be done to make sure that indexes continue to work after the fixture data is loaded.

@mingxinwei
Copy link

this is really useful, it should be merged

@powmedia
Copy link
Owner

What's an example of a situation where you want the indexes from a previous test? The indexes would be referring to documents that are no longer there, and in my experience for tests you want as clean a slate as possible each time.

@mingxinwei
Copy link

mingxinwei commented Jun 27, 2017

You could check this issue #37 .

In my practical unit test usage, when then indexes is droped also, its very easily to meet this error MongoError: cannot perform operation: a background operation is currently running for collection xxx. But after not deleting the index, i never encounter this error.

btw. Using deleteMany also speed up my unit test a lot.

What's an example of a situation where you want the indexes from a previous test? The indexes would be referring to documents that are no longer there, and in my experience for tests you want as clean a slate as possible each time.

@powmedia
Copy link
Owner

OK, however I think there must be another way of fixing the issue #37 without keeping the indexes around. One problem with not removing the indexes for example is if you are testing user account creation and you have a unique index on user.email - next time you run the tests you will get a duplicate key index error.

@markstos
Copy link

What's an example of a situation where you want the indexes from a previous test? The indexes would be referring to documents that are no longer there, and in my experience for tests you want as clean a slate as possible each time.

Mongo should be updating the index properly when documents are deleted, so the indexes should remain correct.

Code reasonably expects indexes to be present, not to manually create there. Without indexes, some operations will fail like some geo functions that require an index.

This package is going to drop indexes, it also needs restore them again.

@bernardofd
Copy link
Contributor Author

bernardofd commented Jun 27, 2017 via email

@powmedia
Copy link
Owner

Mongo should be updating the index properly when documents are deleted, so the indexes should remain correct.

Ah of course, OK will merge this PR

@powmedia powmedia merged commit 98b9cef into powmedia:master Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants