Merge 'Remove SQLite headers dependency from compat tests' from dkaluza

* Add SQLITE_FLAGS env variable handling to compatibility tests as
SQLite does not handle `-q` flag
* Fix inconsistent SQLITE_NOTFOUND error code and add SQLITE_CANTOPEN
code
* Improve compatibility tests to display errors instead of hanging
indefinitely

Closes #599

CONTRIBUTING.md

@@ -78,7 +78,7 @@ make test
 To run the test suite with SQLite, type:
 
 ```
-SQLITE_EXEC=sqlite3 make test
+SQLITE_EXEC=sqlite3 SQLITE_FLAGS="" make test
 ```
 
 When working on a new feature, please consider adding a test case for it.

Makefile

@@ -9,6 +9,9 @@ SQLITE_EXEC ?= ./target/debug/limbo
 
 # Static library to use for SQLite C API compatibility tests.
 BASE_SQLITE_LIB = ./target/$(CURRENT_RUST_TARGET)/debug/liblimbo_sqlite3.a
+# Static library headers to use for SQLITE C API compatibility tests
+BASE_SQLITE_LIB_HEADERS = ./target/$(CURRENT_RUST_TARGET)/debug/include/limbo_sqlite3
+
 
 # On darwin link core foundation
 ifeq ($(UNAME_S),Darwin)
@@ -17,6 +20,8 @@ else
     SQLITE_LIB ?= ../../$(BASE_SQLITE_LIB)
 endif
 
+SQLITE_LIB_HEADERS ?= ../../$(BASE_SQLITE_LIB_HEADERS)
+
 all: check-rust-version check-wasm-target limbo limbo-wasm
 .PHONY: all
 
@@ -69,5 +74,5 @@ test-compat:
 .PHONY: test-compat
 
 test-sqlite3: limbo-c
-	LIBS="$(SQLITE_LIB)" make -C sqlite3/tests test
+	LIBS="$(SQLITE_LIB)" HEADERS="$(SQLITE_LIB_HEADERS)" make -C sqlite3/tests test
 .PHONY: test-sqlite3

sqlite3/include/sqlite3.h

@@ -13,7 +13,9 @@
 
 #define SQLITE_NOMEM 7
 
-#define SQLITE_NOTFOUND 14
+#define SQLITE_NOTFOUND 12
+
+#define SQLITE_CANTOPEN 14
 
 #define SQLITE_MISUSE 21
 

sqlite3/src/lib.rs

@@ -20,7 +20,8 @@ pub const SQLITE_ABORT: ffi::c_int = 4;
 pub const SQLITE_BUSY: ffi::c_int = 5;
 pub const SQLITE_NOMEM: ffi::c_int = 7;
 pub const SQLITE_INTERRUPT: ffi::c_int = 9;
-pub const SQLITE_NOTFOUND: ffi::c_int = 14;
+pub const SQLITE_NOTFOUND: ffi::c_int = 12;
+pub const SQLITE_CANTOPEN: ffi::c_int = 14;
 pub const SQLITE_MISUSE: ffi::c_int = 21;
 pub const SQLITE_ROW: ffi::c_int = 100;
 pub const SQLITE_DONE: ffi::c_int = 101;
@@ -121,7 +122,7 @@ pub unsafe extern "C" fn sqlite3_open(
             *db_out = Box::leak(Box::new(sqlite3::new(db, conn)));
             SQLITE_OK
         }
-        Err(_e) => SQLITE_NOTFOUND,
+        Err(_e) => SQLITE_CANTOPEN,
     }
 }
 

sqlite3/tests/Makefile

@@ -29,7 +29,7 @@ test: $(PROGRAM)
 
 %.o: %.c
 	$(E) "  CC      " $@
-	$(Q) $(CC) $(CFLAGS) -c $< -o $@
+	$(Q) $(CC) $(CFLAGS) -c $< -o $@ -I$(HEADERS)
 
 $(PROGRAM): $(OBJS)
 	$(E) "  LINK    " $@

testing/shelltests.py

@@ -1,9 +1,11 @@
 #!/usr/bin/env python3
 import os
+import select
 import subprocess
 
 # Configuration
 sqlite_exec = os.getenv("SQLITE_EXEC", "./target/debug/limbo")
+sqlite_flags = os.getenv("SQLITE_FLAGS", "-q").split(" ")
 cwd = os.getcwd()
 
 # Initial setup commands
@@ -21,16 +23,18 @@ INSERT INTO t VALUES (zeroblob(1024 - 1), zeroblob(1024 - 2), zeroblob(1024 - 3)
 
 def start_sqlite_repl(sqlite_exec, init_commands):
     # start limbo shell in quiet mode and pipe in init_commands
+    # we cannot use Popen text mode as it is not compatible with non blocking reads
+    # via select and we will be not able to poll for errors
     pipe = subprocess.Popen(
-        [sqlite_exec, "-q"],
+        [sqlite_exec, *sqlite_flags],
         stdin=subprocess.PIPE,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
-        universal_newlines=True,
         bufsize=0,
     )
     if init_commands and pipe.stdin is not None:
-        pipe.stdin.write(init_commands + "\n")
+        init_as_bytes = (init_commands + "\n").encode()
+        pipe.stdin.write(init_as_bytes)
         pipe.stdin.flush()
     return pipe
 
@@ -40,16 +44,43 @@ pipe = start_sqlite_repl(sqlite_exec, init_commands)
 
 
 def execute_sql(pipe, sql):
-    write_to_pipe(sql + "\n")
-    write_to_pipe("SELECT 'END_OF_RESULT';\n")
+    end_suffix = "END_OF_RESULT"
+    write_to_pipe(sql)
+    write_to_pipe(f"SELECT '{end_suffix}';\n")
+    stdout = pipe.stdout
+    stderr = pipe.stderr
 
-    output = []
+    output = ""
     while True:
-        line = pipe.stdout.readline().strip()
-        if line == "END_OF_RESULT":
+        ready_to_read, _, error_in_pipe = select.select([stdout, stderr], [], [stdout, stderr])
+        ready_to_read_or_err = set(ready_to_read + error_in_pipe)
+        if stderr in ready_to_read_or_err:
+            exit_on_error(stderr)
+
+        if stdout in ready_to_read_or_err:
+            fragment = stdout.read(select.PIPE_BUF)
+            output += fragment.decode()
+            if output.rstrip().endswith(end_suffix):
+                output = output.rstrip().removesuffix(end_suffix)
+                break
+
+    output = strip_each_line(output)
+    return output
+
+def strip_each_line(lines: str) -> str:
+    lines = lines.split("\n")
+    lines = [line.strip() for line in lines if line != ""]
+    return "\n".join(lines)
+
+
+def exit_on_error(stderr):
+    while True:
+        ready_to_read, _, have_error = select.select([stderr], [], [stderr], 0)
+        if not (ready_to_read + have_error):
             break
-        output.append(line)
-    return "\n".join(output).strip()
+        error_line = stderr.read(select.PIPE_BUF).decode()
+        print(error_line, end="")
+    exit(2)
 
 
 def run_test(pipe, sql, expected_output):
@@ -70,7 +101,8 @@ def write_to_pipe(line):
     if pipe.stdin is None:
         print("Failed to start SQLite REPL")
         exit(1)
-    pipe.stdin.write(line + "\n")
+    encoded_line = (line + "\n").encode()
+    pipe.stdin.write(encoded_line)
     pipe.stdin.flush()