Skip to content

Add tests convening low level apis#4

Closed
kkharji wants to merge 1 commit intomasterfrom
add_low_level_tests
Closed

Add tests convening low level apis#4
kkharji wants to merge 1 commit intomasterfrom
add_low_level_tests

Conversation

@kkharji
Copy link
Copy Markdown
Owner

@kkharji kkharji commented Dec 30, 2020

  • cover all sql operations.

assert(conn ~= nil, "It shouldn't be nil")
-- assert(sql.connect() == nil, "It return handler")
assert(conn:close() ~= nil, "It should close connection")
-- TODO(tami): We should have an api for checking connection status
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes. Most definitely

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

int sqlite3_status(int op, int *pCurrent, int *pHighwater, int resetFlag); and

  int sqlite3_status64(
    int op,
    sqlite3_int64 *pCurrent,
    sqlite3_int64 *pHighwater,
    int resetFlag
  );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

But thats not connection thats status of operation i think 🤔

Copy link
Copy Markdown
Collaborator

@Conni2461 Conni2461 Dec 30, 2020

Choose a reason for hiding this comment

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

int sqlite3_db_status(sqlite3*, int op, int *pCur, int *pHiwtr, int resetFlg); for db
int sqlite3_stmt_status(sqlite3_stmt*, int op,int resetFlg); for statement

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Oh we can check for both the statement and the status. thats important for the exec function

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

For example it would be cool to error out to the user that the issue is with his sql statement

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah we should use both

@kkharji kkharji force-pushed the add_low_level_tests branch 7 times, most recently from b1b4d53 to 2b5b4f3 Compare December 30, 2020 22:55
@kkharji kkharji force-pushed the add_low_level_tests branch from 2b5b4f3 to 21efbcf Compare December 30, 2020 23:01
@kkharji
Copy link
Copy Markdown
Owner Author

kkharji commented Jan 1, 2021

closed in favor of #5 which will cover the tests created here.

@kkharji kkharji closed this Jan 1, 2021
kkharji added a commit that referenced this pull request Aug 19, 2021
kkharji added a commit that referenced this pull request Aug 19, 2021
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.

3 participants