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.

Sign up for GitHub

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

Jump to bottom

Update Mongoose recipe #2878

Merged
merged 3 commits into from
Oct 31, 2021
Merged

Update Mongoose recipe #2878

merged 3 commits into from
Oct 31, 2021

Conversation

puding-yum
Copy link
Contributor

Continue previous bad pr, I really sorry about that. Actually this is the changes that I mean before.

My problem is cannot get mongo uri with your current readme on https://github.com/avajs/ava/blob/main/docs/recipes/endpoint-testing-with-mongoose.md.
After doing some search I found update rules to get mongo uri but no update on readme.
So I made this update to make it clear.
Also if I am mistaken, please don't hesitate to reach me out or just ignore this pr ya. Thanks:).

@novemberborn
Copy link
Member

Continue previous bad pr, I really sorry about that. Actually this is the changes that I mean before.

For future reference you don't need to close the PR, you can push more changes to your branch and they'll show up here.

With your changes, the recipe now assumes that top-level await is supported. I don't think we can make that assumption. I think you may need to create the server within the test.before() hook?

@puding-yum
Copy link
Contributor Author

Oh yes it is, I updated my code.
and for reference, I found this solution on error message while trying to use your recepe, solution-link.

novemberborn
novemberborn approved these changes Oct 31, 2021
View reviewed changes
Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers @hamidbae

@novemberborn novemberborn changed the title update new phrase to get mongo uri on from mongomemoryserver Update Mongoose recipe Oct 31, 2021
@novemberborn novemberborn merged commit 3a0d102 into avajs:main Oct 31, 2021
puding-yum
puding-yum commented Nov 1, 2021
View reviewed changes
Copy link
Contributor Author

@puding-yum puding-yum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call t.context.mongod instead directly call mongod and test.after.always hook missing t param ?

docs/recipes/endpoint-testing-with-mongoose.md
// First start MongoDB instance
t.context.mongod = await MongoMemoryServer.create();
// And connect
await mongoose.connect(mongod.getUri());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got error when directly call mongod to get Uri. I think it should be t.context.mongod.getUri() ?

docs/recipes/endpoint-testing-with-mongoose.md
@@ -117,8 +116,8 @@ Finally disconnect from and stop MongoDB when all tests are done:

```js
test.after.always(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here I think I should add t as parameter ?

@novemberborn
Copy link
Member

Well spotted, thanks @hamidbae. Fixed in dc405ef.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@novemberborn novemberborn novemberborn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@puding-yum @novemberborn