test(mbtiles): Generate mbtiles dynamically from SQL files to increase debuggability and transparency

[sdf-jkl]

Working on this:

• Create several schema initialization .sql files, e.g. in mbtiles/sql/ dir, add init-flat.sql, init-flat-with-hash.sql, etc.
  • Use content of these files directly in the docs, rather than copy/paste it. See {{#include file.rs}}
  • Use these files in the test.sh before populating them with data
    -- Couldn't find where the mbtiles are being updated in tests.sh
  • Use these files in the unit tests
  • Update docs to use these files with the sqlite command to create a blank .mbtiles file
• Export all existing .mbtiles test files as .sql files, e.g. by using sqlite3's .dump command.
  • Remove any schema creation - and reuse the above ones. We may need to include the schema name in the file name to ensure the right schema is used... unless we can somehow transclude one file inside the other
    -- i am not sure if .read filename.sql can be used from another .sql file?
  • For image content, each tile content should be stored as an individual image file in a subdir - this way they can also be easily inspected. I think the readfile('...') sqlite function can be used to get the file content as part of the insert statement.
  • Same goes for the .mvt / .mlt content, except that sadly we have no way to store it in a viewable format - until someday we may have some simple json-mvt-mlt debug converter, or we could use ogr2ogr or another utility to store geojson - but this is not great because the output is less guaranteed.

Related #1846

[sdf-jkl]

@nyurik
I tested including .read in .sql files and it worked.

I created these test .sql files:
init.sql

CREATE TABLE meow (
    text TEXT
);

read.sql

.read init.sql

INSERT INTO meow VALUES ('meow');

rightaway.sql

CREATE TABLE meow (
    text TEXT
);

INSERT INTO meow VALUES ('meow');

Created .sqlite files to check their difference and there was no output:

sqlite3 read.sqlite < read.sql
sqlite3 rightaway.sqlite < rightaway.sql
sqldiff read.sqlite rightaway.sqlite

So we should be good with using .read inside .sql files.

[nyurik]

awesome work!! I noticed that all .sql files have tons of individual inserts. Is there a sqlite command to dump the entire table as a single insert? Something like this. We could of course modify the sql files by hand later, but if sqlite .dump can do it automatically, it will save some time.

INSERT INTO 'tablename' ('column1', 'column2') VALUES
    ('data1', 'data2'),
    ('data1', 'data2'),
    ('data1', 'data2'),
    ('data1', 'data2');

[sdf-jkl]

Unfortunately, there isn't one. Will have to do that manually (a small script should do the work)

[nyurik]

i don't think you want to do a script for this :) A simple search/replace, or a multi-cursor editor would do all this in a few minutes :)

[sdf-jkl]

Should we create a new just recipe for creating .mbtiles from .sql files?

We can integrate it in just test:

-test: start (test-cargo "--all-targets") test-doc test-int
+test: start prepare-fixtures (test-cargo "--all-targets") test-doc test-int

[nyurik]

I think it would make sense to have a recipe for mbtiles generation, yes. Just make sure it gets created in some directory in the .gitignore list (or add a new entry there).... I will have to think of a good dir structure.

One hope I had though was to avoid using the actual disk files in the unit tests. Instead, it would be good to use in-memory sqlite dbs, populate them with data, and pass them to martin components. See

martin/mbtiles/tests/copy.rs

Line 135 in a1250b9

let file = format!("file:{}_{}{}?mode=memory&cache=shared", stringify!($function), format_args!($($arg)*), $extra);

for an example how in-memory sqlite db can be created, and then opened as if it was a regular file. This method might not work from the start - we may rely on temp files at first.

[sdf-jkl]

I was thinking about generating them in the same folders the original mbtiles were and the new sql files are now ( tests/fixtures/{files, mbtiles}
And adding *.mbtiles to .gitignore

[nyurik]

that could be an interim step, agree

[nyurik]

the problem though is that users won't be able to run cargo test without doing an additional step first -- that's why I was hoping we won't rely on the files

[sdf-jkl]

I think it would make sense to have a recipe for mbtiles generation, yes. Just make sure it gets created in some directory in the .gitignore list (or add a new entry there).... I will have to think of a good dir structure.

One hope I had though was to avoid using the actual disk files in the unit tests. Instead, it would be good to use in-memory sqlite dbs, populate them with data, and pass them to martin components. See

martin/mbtiles/tests/copy.rs

Line 135 in a1250b9

let file = format!("file:{}_{}{}?mode=memory&cache=shared", stringify!($function), format_args!($($arg)*), $extra);

for an example how in-memory sqlite db can be created, and then opened as if it was a regular file. This method might not work from the start - we may rely on temp files at first.

I'll look into this more, before I resolve to use files.

[sdf-jkl]

Sorry for the delay in getting back to this.
From what I understand, the process for the unit tests should be:

1. Create the in-memory SQLite databases.
2. Use sqlx to populate them by reading the fixture files.
3. Access these databases within the unit tests.

I was wondering whether the goal is to initialize the .mbtiles databases once and reuse them across all unit tests, or if we should initialize the required .mbtiles databases fresh within each unit test?

[nyurik]

1. Create the in-memory SQLite databases.
2. Use sqlx to populate them by reading the fixture files.
3. Access these databases within the unit tests.

I was wondering whether the goal is to initialize the .mbtiles databases once and reuse them across all unit tests, or if we should initialize the required .mbtiles databases fresh within each unit test?

• Per test is much better -- each test should run independently, possibly even at the same time, so each in-memory file should either be anonymous :memory:, or when this is not possible (e.g. when a filename must be passed as input), use some temporary filename that is unique per test (it is still in memory, but uses some extra ? params like some other tests already)

• For (2 - populating data) - you can just read the whole script file and pass it to sqlite:

let script = fs::read_to_string("path/to/something.sql").unwrap();
sqlx::raw_sql(script).execute(&mut conn).await.unwrap();

[sdf-jkl]

I updated the summary::tests::summary unit test and it seems like the in-memory dbs do not have file_size.
The page_count was also different.

────────────┬───────────────────────────────────────────────────────────────────
    0       │-file_size: 49152
          0 │+file_size: ~
    1     1 │ mbt_type: Flat
    2     2 │ page_size: 4096
    3       │-page_count: 12
          3 │+page_count: 11
    4     4 │ tile_count: 196
    5     5 │ min_tile_size: 64
    6     6 │ max_tile_size: 1107
    7     7 │ avg_tile_size: 96.2295918367347
────────────┴───────────────────────────────────────────────────────────────────
Stopped on the first failure. Run `cargo insta test` to run all snapshots.

[nyurik]

good progress, thanks! I think this approach is workable, and moreover, we may want to greatly reduce the size of those .sql files by removing things we don't care about (i.e. we do not need to store detalied tilestats and possibly other things for at least some of the files. The rule of thumb: remove anything that is not tested for, but keep anything related to the core functionality of the .mbtiles files.

[nyurik]

I think its ok to modify the test with the new page_count and remove file_size - not that critical

[nyurik]

sorry, all my other projects use ci-test for all testing, making things more consistent in ci and for local dev. For now, just add it to that workflow, but for the future I think we should make CI and local dev to have the same recipe

[CommanderStorm]

Wow, you actually did it 🚀

I have a few minor things which I am just going to apply and one question about a code comment

[CommanderStorm]

What is going on here? Should we remove this line instead?

Suggested change

// "../tests/fixtures/mbtiles/json.mbtiles",

[sdf-jkl]

Thanks for the review! I'm pretty sure I commented it out since the mbtiles were not there anymore.
I think it makes sense to generate an in-memory db for this test to keep it exhaustive.

[sdf-jkl]

Self review

[sdf-jkl]

A lot of boilerplate code in this file, could be shrinked into a function/macro

[sdf-jkl]

Does this have to be wall of text?

[sdf-jkl]

Huge Binary in the script

[sdf-jkl]

Huge binary again

[CommanderStorm]

@sdf-jkl All of the issues that remain can/should be different PRs 😉
=> merging as is

This is great work, thank you so much

[sdf-jkl]

@CommanderStorm @nyurik Thanks for your help on this, this was definitely too big of a pr to go through and I should have split the work into smaller PRs way earlier.
Happy to get it done.