tests: Add SQLite integration coverage from canonical sources - #2002
tests: Add SQLite integration coverage from canonical sources#2002thedataking wants to merge 5 commits into
Conversation
cce7db1 to
be8873d
Compare
Pin SQLite 3.53.4 from the official source mirror to provide the canonical source tree and upstream workloads for integration coverage.
Build libsqlite3.a with --disable-amalgamation so the integration harness translates the individual library source files. Generate sources before recording the compilation database to exclude host build tools. Enable R-tree support for the spatial workload.
Run definition reorganization and the name, import, label, cast, and literal cleanup transforms on translated SQLite, then compile the result to exercise refactoring across the library modules.
Run seven upstream speedtest workloads against the native and Rust archives before and after refactoring, comparing result counts and hashes. Compile the same native C callers against both libraries to exercise the public C ABI. Add fixed SQL assertions for transactions, triggers, foreign keys, blobs, JSON, numeric parsing, window functions, WAL persistence, and integrity. These cover behavior that the upstream workloads execute without hashing result rows.
Check out the SQLite submodule and add it to the integration project list so both Clang configurations exercise its translation, refactoring, and runtime behavior.
be8873d to
e53b57e
Compare
|
|
||
| { | ||
| TARGET_DIR=$(cargo ${TOOLCHAIN:+"$TOOLCHAIN"} metadata --no-deps --format-version 1 \ | ||
| | python3 -c 'import json, sys; print(json.load(sys.stdin)["target_directory"])') |
There was a problem hiding this comment.
jq might actually be shorter here.
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| cd "$SCRIPT_DIR/repo" | ||
|
|
||
| ./configure --disable-amalgamation --disable-shared --disable-readline \ |
There was a problem hiding this comment.
I saw it in the README, but it might be useful to also explain --disable-amalgamation here.
ahomescu
left a comment
There was a problem hiding this comment.
Review findings (7), posted inline. The two that matter most: the json and parsenumber testsets compare a constant hash, and --fail-on-error is unique to this conf.yml.
| cc -O2 -DSQLITE_ENABLE_RTREE -I. test/speedtest1.c "$TARGET_DIR/release/librepo.a" \ | ||
| -lm -ldl -lpthread -o "$WORK_DIR/rust" | ||
|
|
||
| for testset in main cte orm fp json rtree parsenumber; do |
There was a problem hiding this comment.
json and parsenumber testsets compare a constant hash. Neither testset calls speedtest1_run, so both binaries always print the same trivial Verification Hash and the diff cannot fail. parsenumber additionally discards every sqlite3_exec return code (speedtest1.c:2890-2908). A translated library that errors on or mis-evaluates every SELECT in parsenumber, or corrupts JSON output in json, still passes this stage. Only main/cte/orm/fp/rtree hash rows. Either drop those two testsets or add explicit checks for them to smoke.c, and reword the README sentence about every workload's hash being compared.
| @@ -0,0 +1,19 @@ | |||
| transpile: | |||
| autogen: true | |||
| tflags: --reorganize-definitions --disable-refactoring --fail-on-error | |||
There was a problem hiding this comment.
--fail-on-error is unique to this project. No other conf.yml passes it. With it, any Skipping declaration ... due to error (translator/mod.rs:716 exits 1) or invalid exported Clang AST (lib.rs:707 panics) aborts the whole transpile stage instead of stubbing and continuing. That class of warning is live on master today: libgit2's transpile.gen.sh.log shows Exported Clang AST was invalid. If one of the ~100 SQLite TUs hits an unexported node under either CI clang (15 or 18), sqlite fails hard while every other project degrades. Defensible as a regression guard for the #2001 fixes, but worth stating as a deliberate choice.
|
|
||
| static void expect_int(sqlite3 *db, const char *sql, int expected) { | ||
| sqlite3_stmt *stmt = NULL; | ||
| assert(sqlite3_prepare_v2(db, sql, -1, &stmt, NULL) == SQLITE_OK); |
There was a problem hiding this comment.
Side-effecting calls inside assert(). sqlite3_open, sqlite3_prepare_v2, sqlite3_step, sqlite3_finalize, sqlite3_close and the FK-violation sqlite3_exec are evaluated only when NDEBUG is undefined, while the rest of the file uses an explicit exit(1) path. Anyone compiling with -DNDEBUG gets db == NULL passed to sqlite3_exec (misleading SQLITE_MISUSE), and the FK-constraint and finalized-statements checks silently disappear. Latent today since test.sh uses plain cc -O2, but a CHECK macro that cannot be compiled out would give the file one failure path.
| cd "$SCRIPT_DIR/repo" | ||
|
|
||
| { | ||
| TARGET_DIR=$(cargo ${TOOLCHAIN:+"$TOOLCHAIN"} metadata --no-deps --format-version 1 \ |
There was a problem hiding this comment.
cargo metadata + inline python3 to find the target dir. This adds an undeclared python3 runtime dependency (the harness itself runs under uv run --script, which does not guarantee one on PATH) and a third convention for a lookup that json-c/test.sh and python2/test.sh do with ${CARGO_TARGET_DIR:-$SCRIPT_DIR/repo/target}/release/librepo.a plus an explicit "Rust archive not found" check. Suggest the json-c expression, or at least jq -r .target_directory.
| WORK_DIR=$(mktemp -d) | ||
| trap 'rm -rf "$WORK_DIR"' EXIT | ||
|
|
||
| for archive in "$SCRIPT_DIR/repo/libsqlite3.a" "$TARGET_DIR/release/librepo.a"; do |
There was a problem hiding this comment.
Four near-identical cc link lines. smoke.c is built in a loop over the two archives, but the two speedtest1.c compiles are written out by hand, and the later loop re-derives the native/rust naming. The flags appear four times, and libsqlite3.a is referenced both relatively (line 22) and as $SCRIPT_DIR/repo/libsqlite3.a (here). A single for impl in native rust loop that picks the archive and builds both smoke-$impl and $impl covers everything, and per-implementation db names remove the rm -f smoke.db* cleanup.
| stays native so the same caller exercises both libraries' public C ABI. | ||
|
|
||
| `smoke.c` also checks fixed SQL results, transaction/savepoint rollback, triggers, | ||
| foreign keys, blobs, JSON, numeric parsing, window functions, WAL persistence |
There was a problem hiding this comment.
WAL persistence is claimed but not checked. After reopening, smoke.c checks the audit row count and integrity_check but never PRAGMA journal_mode, so a translated pager that silently falls back to rollback-journal mode still passes. Either add expect_int(db, "SELECT journal_mode = 'wal' FROM pragma_journal_mode", 1) after the reopen, or soften this line.
| trap 'rm -rf "$WORK_DIR"' EXIT | ||
|
|
||
| for archive in "$SCRIPT_DIR/repo/libsqlite3.a" "$TARGET_DIR/release/librepo.a"; do | ||
| cc -O2 -I. "$SCRIPT_DIR/smoke.c" "$archive" -lm -ldl -lpthread \ |
There was a problem hiding this comment.
Hardcoded -lm -ldl -lpthread (low confidence). json-c/test.sh derives the list from RUSTFLAGS="--print native-static-libs"; rustc reports -lgcc_s -lutil -lrt -lpthread -lm -ldl -lc. This links on CI because glibc >= 2.34 merged the rest and cc adds libgcc_s implicitly, but an older host or a future native dependency of the transpiled crate would fail with undefined symbols and no hint why.
Add SQLite 3.53.4 to the integration harness and both Clang CI configurations, using the official source submodule and
--disable-amalgamation. Buildlibsqlite3.afrom individual translation units to exercise cross-file translation. Generate sources before recording the compilation database to exclude host build tools.Run seven upstream speedtest workloads against the native and translated libraries before and after refactoring, comparing result counts and hashes. Fixed SQL assertions cover transactions, triggers, foreign keys, blobs, JSON, numeric parsing, window functions, WAL persistence, and integrity, including behavior the upstream workloads execute without hashing results. Both libraries use the same native C callers to exercise their public C ABI.
Depends on the transpiler fixes in #2001.