-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
docs(sql.md): Add documentation for testing with database #3096
base: master
Are you sure you want to change the base?
Conversation
This was requested in nestjs#246 and since I was recently struggling to set this up in one of my own projects, I have it fresh in memory how to do this. If this was in the documentation when I was implementing this, it would have saved me a lot of time. There are many dodgy approaches to testing NestJS applications with a test database on the Internet and an official recommended way would be most helpful. Resolves nestjs#246
4c66eec
to
bd4e7e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is a possible approach, it's not one that I personally would suggest or want to promote. SQLite is a great file and in memory database, but it is rather limited and there are times that it doesn't support features that the actual database relies on.
If it is important to test against an actual database, I'd suggest we rather show how to use Docker/Docker-compose to run a similar database instance as production, then run migrations, and then run the tests
Thank you for your reply, @jmcdo29. I think the purpose is to show how to test with an actual database and I don't think showing how to set up docker-compose to run a database is within scope of the NestJS documentation. Using SQLite as an example makes sense, as it doesn't have any external dependencies. The focus is to show how to use a real database in a testing module, without using the database configured in the app, as it is quite sensible to have a dedicated database for testing. I would also point out that part of the reason for using something like TypeORM is that your code becomes database agnostic. I would assume that most projects could swap databases fairly easily, as they do not rely on any specific functionality. That should mean that they would be well served, using an in-memory SQLite database, during testing. Regardless, the point isn't to promote it as a practice, it just happens to be the best example (IMHO) to show how to set up your tests with a test database. I think users of mysql or postgres (etc.) will be able to figure out how to adapt it to their use case fairly easily. I think showing how to test against a production like database instance and how to run migration tests would be a great addition to the documentation, but I think it is out of scope for what I was trying to do. I was struggling to figure out how to test a module without having it try to connect to the database. For me, the key part is that you need to build you own testing module, either with a test database or mocks for your repositories. I tried both, but using a test database is simply less hassle. It was also requested in #246, so I decided to write the documentation from that perspective. |
The testing documentation could benefit from additional guidance, especially for those with little to no experience in testing. However, for more experienced users, these changes might not be as impactful unless the documentation also delves into testing strategies, which I believe could be improved. For reference, TestingContainers can be a helpful tool for setting up non-unit tests. |
@donnyroufs I definitely agree with that. I created a resource with the nest cli and NestJS has created one spec file for the controller and one for the service. On top of that, there are the e2e tests. What tests do I write where and why? Testing the controller, is pretty much the same as the e2e tests without the http layer, and in the end, they all call the service, which I could test directly as well. Some guidance would certainly be welcome, as writing the same test in three places seems silly. Even duplicating the controller tests in the e2e tests seems questionable. The app controller tests are duplicated in the e2e tests, so perhaps that is the recommended approach. I'm not sure I see the point though. Also, e2e doesn't seem to actually mean end to end here. It just means that your tests include the http layer. You are still testing against test modules where you can mock as much of the application as you want. It's really e2? tests. The other tests seem like they are a mix of unit and integration tests. You can also put e2e tests there, if you want, it's just a convention that you put them in a separate folder and execute them separately. There's no special functionality that's only enabled for e2e tests. I'm used to having separate test suites for unit, integration, and acceptance tests. Unit tests run fast, as they test each component in isolation. Integration tests are a bit slower, but generally much more useful. Acceptance tests are the slowest, as they run against an actual running deployment. Additionally, you can have smoke, migration, performance, security and other categories of tests, as needed for your application. This kind of categorisation makes it clear what goes where and why. The |
This was requested in #246 and since I was recently struggling to set this up in one of my own projects, I have it fresh in memory how to do this. If this was in the documentation when I was implementing this, it would have saved me a lot of time. There are many dodgy approaches to testing NestJS applications with a test database on the Internet and an official recommended way would be most helpful.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #246
What is the new behavior?
Documentation for using a test database in your testing modules.
Does this PR introduce a breaking change?
Other information