unjs / db0

๐Ÿ“š Lightweight SQL Connector
https://db0.unjs.io
MIT License
171 stars 18 forks source link

test: mock database interface #124

Open beanc16 opened 4 days ago

beanc16 commented 4 days ago

๐Ÿ”— Linked issue resolves https://github.com/unjs/db0/issues/123

โ“ Type of change

๐Ÿ“ Checklist

pi0 commented 4 days ago

Thanks for the PR. I'm trying to understand benefits and what extra (real) coverage it is adding.

It seems replacing actual functionality of databases with our "expected" behavior (which can vary from real behavior).

123

which appears to limit the ability to enable wider test coverage due to connection requirements.

A simple in-mem sqlite connection would enable us to run any kind of unit tests (that are not testing connectors but other functionalities) unless I'm wrong?

beanc16 commented 3 days ago

Thanks for the PR. I'm trying to understand benefits and what extra (real) coverage it is adding.

It seems replacing actual functionality of databases with our "expected" behavior (which can vary from real behavior).

For me, it's moreso about the distinction between unit vs integration tests. What's here prior to this PR works great as integration tests, since they connect directly to the database. Though, unit tests are inherently meant to test something and get the same result every time with no external dependencies (like a database connection).

What sparked my PR was that I was kind of surprised that I wouldn't be able to mock or run tests in any way in an isolated manner without having all types of database connections set up first. This PR makes it so you can test changes without a database connection and make sure things work (assuming this is something we want). Though, if we want integration tests to ensure connections actually work 100% of the time, I'd think we want unit vs integration tests to be separate so it's clear as to what type of test is what.

pi0 commented 3 days ago

Thanks for explanation. I see value of unit tests but what are we exactly going to "unit" test with this PR?