Skip to content

Adding test for actix-web#698

Closed
dsullivan7 wants to merge 1 commit intoSeaQL:masterfrom
dsullivan7:actix-web-test
Closed

Adding test for actix-web#698
dsullivan7 wants to merge 1 commit intoSeaQL:masterfrom
dsullivan7:actix-web-test

Conversation

@dsullivan7
Copy link
Copy Markdown

I'd like to add a test for actix-web, however, I found that the into_transaction_log function can't be called on the MockDatabase because AppState, more specifically conn, has already been consumed. I'm looking for guidance on how to avoid this problem, let me know what I can do.

@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented May 9, 2022

Can you convert this to draft?

@tyt2y3 tyt2y3 marked this pull request as draft May 9, 2022 14:22
@dsullivan7
Copy link
Copy Markdown
Author

@tyt2y3 No problem, good to know for the future.

Copy link
Copy Markdown
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

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

Hey @dsullivan7, sorry for the delay and thanks for the contributions!!

Recently, we're updating the examples to include mock testing for SeaORM's CRUD operations. Please check the PR at #890.

Basically, we decouple SeaORM CRUD operations from the web API. Then, we mock test the CRUD operations. This avoid the pitfall that most web API requires the app state to be cloneable but SeaORM connection isn't cloneable when mock feature is enabled.

I think what you did is interesting and it's a good demonstration on testing the point of integration between Actix, SeaORM and Tera. And it fill the missing gap that we didn't cover in #890 - testing the handlers of web API.

@dsullivan7
Copy link
Copy Markdown
Author

@billy1624 Thanks very much for the response. I'm going to close this PR. Glad to see more development on this, it's a great library.

@dsullivan7 dsullivan7 closed this Aug 17, 2022
@billy1624
Copy link
Copy Markdown
Member

Thanks again for contributing!! Let me know if you spot any room for improvements :)

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants