Skip to content

Fix assertQuery changing to empty results after snapshot contains results - #191

Merged
mbrandonw merged 5 commits into
pointfreeco:mainfrom
rcarver:fix-assertQuery-change-to-empty-result
Sep 17, 2025
Merged

mbrandonw merged 5 commits into
pointfreeco:mainfrom
rcarver:fix-assertQuery-change-to-empty-result

Conversation

@rcarver

@rcarver rcarver commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Fixes the issue brought up in slack

image

@stephencelis

Copy link
Copy Markdown
Member

@rcarver Can you give an example of what this fixes? Maybe a test with an expected failure or something to that effect?

I think the fix we probably want to apply here is to always record an empty result set, something like:

assertQuery(Reminder.all) {
  """
  (No results.)
  """
}

Think you could cook that change up?

@rcarver

rcarver commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

@stephencelis sorry based on the comments in slack I assumed you'd have context. This fixes the following:

  1. Successfully record a snapshot of some results
  2. Change the database content such that no results are returned
  3. Run the test again, it continues to pass when it should fail ("no results")

Here's a diff I used to switch between .database() and .emptyDatabase() to confirm that the test fails as expected.

diff --git a/Tests/SharingGRDBTests/AssertQueryTests.swift b/Tests/SharingGRDBTests/AssertQueryTests.swift
index 73de34d..6dbe543 100644
--- a/Tests/SharingGRDBTests/AssertQueryTests.swift
+++ b/Tests/SharingGRDBTests/AssertQueryTests.swift
@@ -153,4 +153,40 @@ extension DatabaseWriter where Self == DatabaseQueue {
     }
     return database
   }
+  fileprivate static func emptyDatabase() throws -> DatabaseQueue {
+    let database = try DatabaseQueue()
+    try database.write { db in
+      try #sql(
+        """
+        CREATE TABLE "records" (
+          "id" INTEGER PRIMARY KEY AUTOINCREMENT,
+          "date" INTEGER NOT NULL DEFAULT 42
+        )
+        """
+      )
+      .execute(db)
+    }
+    return database
+  }
+}
+
+
+@Suite(
+  .dependency(\.defaultDatabase, try .database()),
+  .snapshots(record: .failed),
+)
+struct AssertQueryEmptyResultTests {
+  @Test func assertQueryBasic() throws {
+    assertQuery(
+      Record.all.select(\.id)
+    ) {
+      """
+      ┌───┐
+      │ 1 │
+      │ 2 │
+      │ 3 │
+      └───┘
+      """
+    }
+  }
 }

@rcarver

rcarver commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

Also clarified the 'no results' case

  @Test func assertQueryEmpty() throws {
    assertQuery(
      Record.all.where { $0.id == -1 }.select(\.id)
    ) {
      """
      (No results)
      """
    }
  }

@rcarver

rcarver commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

Hmm, looks like this got caught in the repo rename, let me know if you'd like me to recreate the PR

@mbrandonw

Copy link
Copy Markdown
Member

I just merged main to get everything caught up and re-recorded the SQLiteData tests which now properly shows "(no results)" in a bunch of tests.

@rcarver

rcarver commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

Thanks! lgtm

@acosmicflamingo

Copy link
Copy Markdown
Contributor

Looks good to me too!
image

@mbrandonw
mbrandonw merged commit 32bb1e0 into pointfreeco:main Sep 17, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants