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. |
Lib/sqlite3/test/hooks.py
Outdated
| @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.
Lib/sqlite3/test/hooks.py
Outdated
| @@ -23,6 +23,7 @@ | |||
|
|
|||
| import unittest | |||
| import sqlite3 as sqlite | |||
| from tempfile import NamedTemporaryFile | |||
There was a problem hiding this comment.
Nitpick: I'd prefer import tempfile.
Lib/sqlite3/test/hooks.py
Outdated
| 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.
Lib/sqlite3/test/hooks.py
Outdated
| """ | ||
| 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.
Lib/sqlite3/test/hooks.py
Outdated
| 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.
Lib/sqlite3/test/hooks.py
Outdated
| @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)
Misc/NEWS
Outdated
| @@ -278,6 +278,10 @@ Extension Modules | |||
| Library | |||
| ------- | |||
|
|
|||
| - bpo-26187: Test that sqlite trace callback is not called multiple | |||
Misc/NEWS
Outdated
| @@ -278,6 +278,10 @@ Extension Modules | |||
| Library | |||
| ------- | |||
|
|
|||
| - 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.
Misc/NEWS
Outdated
| @@ -278,6 +278,10 @@ Extension Modules | |||
| Library | |||
| ------- | |||
|
|
|||
| - 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."?
Misc/NEWS
Outdated
| @@ -302,6 +306,7 @@ Library | |||
| 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>
Test for issue 26187