feat: improve SQLite DB engine spec #24909
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24909 +/- ##
=======================================
Coverage 68.99% 68.99%
=======================================
Files 1906 1906
Lines 74122 74134 +12
Branches 8208 8208
=======================================
+ Hits 51140 51150 +10
- Misses 20859 20861 +2
Partials 2123 2123
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| assert rv.status_code == 200 | ||
| assert response == { | ||
| "function_names": [ | ||
| "abs", |
There was a problem hiding this comment.
wdyt about defining this list as a separate variable in the db engine spec and then importing it here, or do you want this to be explicitly updated here when the original list is updated?
There was a problem hiding this comment.
Yeah, I tend to avoid comparing results to imported variables in unit tests, since it can lead to mistakes.
SQLite is the default database for Superset and does not require additional dependencies/installation steps, which is really helpful for first time contributors or people that want to quickly test the application. This PR is really important. Thanks! |
| """ | ||
| Return function names. | ||
| """ | ||
| return [ |
There was a problem hiding this comment.
@betodealmeida I'm somewhat surprised sqlite3 doesn't provide these.
There was a problem hiding this comment.
Yeah, I wish it exposed more of the internals of SQLite!
mistercrunch
added
🏷️ bot
SUMMARY
Add more (all) time grains to the SQLite DB engine spec and also
get_function_names. Function names from:I know we don't take the SQLite DB engine spec seriously, but it's used as the base for GSheets and Shillelagh.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Added more unit tests.
ADDITIONAL INFORMATION