bpo-26187: Tests for sqlite3 trace callback#434
Conversation
|
@AnishShah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Yhg1s, @ghaering, @berkerpeksag, @mdickinson and @serhiy-storchaka to be potential reviewers. |
| "Unicode data %s garbled in trace callback: %s" | ||
| % (ascii(unicode_value), ', '.join(map(ascii, traced_statements)))) | ||
|
|
||
| def CheckTraceCallbackContent(self): |
There was a problem hiding this comment.
We should skip the test if sqlite3_prepare_v2 is not available.
There was a problem hiding this comment.
The test name doesn't explain what we actually testing here.
|
|
||
| def CheckTraceCallbackContent(self): | ||
| """ | ||
| Test that the statement are correct. Fix for bpo-26187 |
| con.execute(query) | ||
| con.commit() | ||
| queries.insert(1, "BEGIN ") | ||
| queries.append("COMMIT") |
There was a problem hiding this comment.
These two lines can be removed. We only want to test that set_trace_callback doesn't produce any duplicate entries.
|
Thanks, Aviv. Next time please send your feedback as a comment and give the PR author some time to address your comments. Otherwise, we will end up with duplicate PRs. Also note that we always locally run tests and make sure that the test fails before the fix is applied so the chance of merging a broken test is minimal. |
|
Sorry about that @berkerpeksag. I actually started by writing a comment on this PR but found out that what I do is to write the all test again. I am still having hard time understanding what @AnishShah did here as I wrote a test case in the issue. |
No description provided.