Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native user_version support in Connection #1105

Merged
merged 8 commits into from
Jan 25, 2022
Merged

Native user_version support in Connection #1105

merged 8 commits into from
Jan 25, 2022

Conversation

nathanfallet
Copy link
Collaborator

No description provided.

@nathanfallet
Copy link
Collaborator Author

@jberkel Test fails on Linux because of something related to encodable, but I don't think that it is related to my changes. Do you have any idea?

Test Case 'QueryTests.test_update_encodable_with_nested_encodable' started at 2022-01-17 16:31:02.883
/home/runner/work/SQLite.swift/SQLite.swift/Tests/SQLiteTests/QueryTests.swift:396: error: QueryTests.test_update_encodable_with_nested_encodable : XCTAssertEqual failed: ("UPDATE "emails" SET "int" = 1, "string" = '2', "bool" = 1, "float" = 3.0, "double" = 4.0, "date" = '1970-01-01T00:00:00.000', "sub" = '{"date":-978307200,"int":1,"float":3,"string":"2","double":4,"bool":true}'") is not equal to ("UPDATE "emails" SET "int" = 1, "string" = '2', "bool" = 1, "float" = 3.0, "double" = 4.0, "date" = '1970-01-01T00:00:00.000', "sub" = '{"date":-978307200,"double":4,"int":1,"string":"2","bool":true,"float":3}'") - 
Test Case 'QueryTests.test_update_encodable_with_nested_encodable' failed (0.001 seconds)

@jberkel
Copy link
Collaborator

jberkel commented Jan 18, 2022

@nathanfallet the test is brittle because it compares the JSON as a string. On Linux the JSON serialization is different (order of fields). You could make the JSON serialization more deterministic with OutputFormatting.sortedKeys (but that's only iOS 11+, macOS 10.13+) or improve the test so that it does not do a by string comparison.

@nathanfallet
Copy link
Collaborator Author

@jberkel Right, I'm gonna inspect that. I need to understand why it changed since the last commit (because test was passing for the last commit on master)

/// Gets the user version of the database.
/// See SQLite [PRAGMA user_version](https://sqlite.org/pragma.html#pragma_user_version)
/// - Returns: the user version of the database
public func getUserVersion() throws -> Int32? {
Copy link
Collaborator

@jberkel jberkel Jan 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var userVersion: Int32? {
   get { }
   set { }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first, but I ended with problems with try. I can eventually ignore them with optionals

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it's ok to ignore the error here

@nathanfallet
Copy link
Collaborator Author

@nathanfallet the test is brittle because it compares the JSON as a string. On Linux the JSON serialization is different (order of fields). You could make the JSON serialization more deterministic with OutputFormatting.sortedKeys (but that's only iOS 11+, macOS 10.13+) or improve the test so that it does not do a by string comparison.

After inspection, it means that JSONEncoder().encode(value) gives different data each time, even with the same object passed as value?!

@jberkel
Copy link
Collaborator

jberkel commented Jan 18, 2022

After inspection, it means that JSONEncoder().encode(value) gives different data each time, even with the same object passed as value?!

Possibly, there are no guarantees by the method. I think we should do both: make the JSON output consistent (with sortedKeys) if possible, and avoid doing a string comparison in the test.

return nil
public var userVersion: Int32? {
get {
guard let userVersion = try? scalar("PRAGMA user_version") as? Int64 else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(try? scalar("PRAGMA user_version") as? Int64).map(Int32.init)

@nathanfallet nathanfallet merged commit 5680160 into master Jan 25, 2022
@nathanfallet nathanfallet deleted the userVersion branch January 25, 2022 14:21
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.

2 participants