bpo-26187: Tests sqlite3 trace callback no duplicate on shcema changing#461
Conversation
|
@palaviv, 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. |
berkerpeksag
left a comment
berkerpeksag
left a comment
There was a problem hiding this comment.
Please also add a note to Misc/NEWS and mention the v2 API change.
| @unittest.skipIf(sqlite.sqlite_version_info < (3, 3, 9), "sqlite3_prepare_v2 is not available") | ||
| def CheckTraceCallbackContent(self): | ||
| """ | ||
| Test that the statement are correct. Fix for bpo-26187 |
There was a problem hiding this comment.
Docstring is not needed here. Just add a comment with a reference to bpo-26187.
|
|
||
| import unittest | ||
| import sqlite3 as sqlite | ||
| from tempfile import NamedTemporaryFile |
There was a problem hiding this comment.
Nitpick: I'd prefer import tempfile.
| def trace(statement): | ||
| traced_statements.append(statement) | ||
|
|
||
| queries = ["create table foo(x)", |
There was a problem hiding this comment.
traced_statements, trace and queries can go outside of the with statements.
| """ | ||
| Test that the statement are correct. Fix for bpo-26187 | ||
| """ | ||
| with NamedTemporaryFile(suffix='.sqlite') as db_path: |
There was a problem hiding this comment.
NamedTemporaryFile is known to be problematic on Windows. Did you check that it cleanups the temp file?
There was a problem hiding this comment.
I will look what they do in other test's and copy it.
| con2.execute("create table bar(x)") | ||
| cur.execute(queries[1]) | ||
| self.assertEqual(traced_statements, queries) | ||
|
|
There was a problem hiding this comment.
Nitpick: Please delete the extra new line.
| @unittest.skipIf(sqlite.sqlite_version_info < (3, 3, 9), "sqlite3_prepare_v2 is not available") | ||
| def CheckTraceCallbackContent(self): | ||
| """ | ||
| bpo-26187 |
There was a problem hiding this comment.
I'd prefer a plain Python comment instead of a docstring:
# set_trace_callback() shouldn't produce duplicate content (bpo-26187)| Library | ||
| ------- | ||
|
|
||
| - bpo-26187: Test that sqlite trace callback is not called multiple |
| ------- | ||
|
|
||
| - bpo-26187: Test that sqlite trace callback is not called multiple | ||
| times when schema is changing. Fixed by bpo-9303 |
There was a problem hiding this comment.
Add two spaces at the end of a sentence.
| ------- | ||
|
|
||
| - bpo-26187: Test that sqlite trace callback is not called multiple | ||
| times when schema is changing. Fixed by bpo-9303 |
There was a problem hiding this comment.
Perhaps we can say something like "Indirectly fixed by switching to use sqlite3_prepare_v2() in bpo-9303."?
| check identity before checking equality when do comparisons. | ||
|
|
||
| - bpo-28682: Added support for bytes paths in os.fwalk(). | ||
| >>>>>>> upstream/master |
There was a problem hiding this comment.
This looks like a merge glitch :)
|
Looks good, thanks! I just fixed the merge conflict and will merge it when Travis is done. |
Bumps [pytest-cov](https://github.com/pytest-dev/pytest-cov) from 2.12.0 to 2.12.1. - [Release notes](https://github.com/pytest-dev/pytest-cov/releases) - [Changelog](https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst) - [Commits](pytest-dev/pytest-cov@v2.12.0...v2.12.1) --- updated-dependencies: - dependency-name: pytest-cov dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
4 participants
Test for issue 26187