bpo-26187: Tests for sqlite3 trace callback by AnishShah · Pull Request #434 · python/cpython
| "Unicode data %s garbled in trace callback: %s" | ||
| % (ascii(unicode_value), ', '.join(map(ascii, traced_statements)))) | ||
|
|
||
| def CheckTraceCallbackContent(self): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should skip the test if sqlite3_prepare_v2 is not available.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test name doesn't explain what we actually testing here.
|
|
||
| def CheckTraceCallbackContent(self): | ||
| """ | ||
| Test that the statement are correct. Fix for bpo-26187 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring is not needed.
| con.execute(query) | ||
| con.commit() | ||
| queries.insert(1, "BEGIN ") | ||
| queries.append("COMMIT") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.