Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-40784: Fix sqlite3 deterministic test #20448

Merged
merged 8 commits into from
May 28, 2020

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 27, 2020

Co-authored-by: Victor Stinner <vstinner@python.org>
@vstinner
Copy link
Member

Maybe if would be safer to run tests on buildbots to check more SQLite versions, before merging this PR.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM if CI pass. As I wrote, it sounds worth it to spawn buildbots tests on this PR.

@erlend-aasland: Please stop touching your PR :-) So we can run more tests (buildbots are likely to use more various SQLite versions).

@vstinner
Copy link
Member

Oh, it may also help if someone could test manually with SQLite 3.32.

@erlend-aasland
Copy link
Contributor Author

Oh, it may also help if someone could test manually with SQLite 3.32.

Already did that, and it passes! 3.32.1 FWIW

@vstinner
Copy link
Member

Already did that, and it passes! 3.32.1 FWIW

Cool. Thanks!

@vstinner
Copy link
Member

This still fails. I've fixed CheckFuncNonDeterministic locally but not pushed it yet.

Yeah, the Ubuntu job of Azure Pipelines fails. It uses "sqlite3.sqlite_version: 3.11.0" according to its test.pythoninfo. Test failure:

FAIL: CheckFuncNonDeterministic (sqlite3.test.userfunctions.FunctionTests)
Non-deterministic functions cannot be used with partial indices
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/sqlite3/test/userfunctions.py", line 288, in CheckFuncNonDeterministic
    self.assertIn("non-deterministic functions prohibited", str(cm.exception))
AssertionError: 'non-deterministic functions prohibited' not found in 'functions prohibited in partial index WHERE clauses'

Honestly, I don't think that we should test the error message. Please simply remove the test the error message. IMHO excepting an exception is enough.

@erlend-aasland
Copy link
Contributor Author

Yeah, the Ubuntu job of Azure Pipelines fails. It uses "sqlite3.sqlite_version: 3.11.0" according to its test.pythoninfo.

Right. Deterministic functions were allowed in partial indices in sqlite v3.15.0 (2016-10-14). I'll find another way of testing this, then.

Honestly, I don't think that we should test the error message. Please simply remove the test the error message. IMHO excepting an exception is enough.

Agreed. Error messages are likely to change between versions.

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented May 27, 2020

@vstinner I suggest just keeping the old tests for SQLite pre v3.15.0. From the sqlite release history:

2014-02-03 (3.8.3) […] Added SQLITE_DETERMINISTIC […] providing applications with the ability to create new functions that can be factored out of inner loops when they have constant arguments.

Seven minor versions later:

2016-10-14 (3.15.0) […] Allow deterministic SQL functions in the WHERE clause of a partial index.

I'll push the change right away.

@encukou
Copy link
Member

encukou commented May 28, 2020

Thank you!
I'll start the buildbots so Victor can continue the review when he gets to the computer.

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 28, 2020
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit b969ae5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 28, 2020
@erlend-aasland
Copy link
Contributor Author

Thank you, @encukou!

@encukou
Copy link
Member

encukou commented May 28, 2020

Some failures might not be related to this change.

@vstinner
Copy link
Member

AMD64 Fedora Stable Clang Installed PR, PPC64LE Fedora Stable Clang Installed PR and s390x Fedora Clang Installed PR and x86 Gentoo Installed with X PR failed because of https://bugs.python.org/issue38488 regression (setuptools/chmod) which is unrelated.

AMD64 FreeBSD Non-Debug PR failures (test_asyncio, test_venv) are unrelated.

AMD64 Windows8.1 Refleaks PR failure (test_socket) is unrelated.

@vstinner
Copy link
Member

(this comment is unrelated to test_sqlite)

AMD64 Fedora Stable Clang Installed PR, PPC64LE Fedora Stable Clang Installed PR and s390x Fedora Clang Installed PR and x86 Gentoo Installed with X PR failed because of https://bugs.python.org/issue38488 regression (setuptools/chmod) which is unrelated.

I reverted the change which introduced this issue: see https://bugs.python.org/issue38488

AMD64 FreeBSD Non-Debug PR failures (test_asyncio, test_venv) are unrelated.

test_asyncio: test_sock_client_racing() failed, see https://bugs.python.org/issue30064. I created #20485 (merged) to skip the test.

The test_venv is also related to https://bugs.python.org/issue38488 that I reverted: I created https://bugs.python.org/issue40808

AMD64 Windows8.1 Refleaks PR failure (test_socket) is unrelated.

0:41:43 load avg: 6.17 [206/424/1] test_socket failed (41.9 sec) -- running: test_multiprocessing_spawn (41 min 20 sec), test_regrtest (5 min 37 sec)
beginning 6 repetitions
123456
Warning -- threading._dangling was modified by test_socket
  Before: {<weakref at 0x000000B0E9AE4530; to '_MainThread' at 0x000000B0E9935E10>}
  After:  {<weakref at 0x000000B0EC3A1D10; to '_MainThread' at 0x000000B0E9935E10>, <weakref at 0x000000B0EC3A1D70; to 'Thread' at 0x000000B0EBEDD820>, <weakref at 0x000000B0EC3A1DD0; to 'Thread' at 0x000000B0EBEDD690>, <weakref at 0x000000B0EC3A1E30; to 'Thread' at 0x000000B0EBEDD730>, <weakref at 0x000000B0EC3A1E90; to 'Thread' at 0x000000B0EBEDD780>} 
test test_socket failed -- Traceback (most recent call last):
  File "D:\buildarea\pull_request.ware-win81-release.refleak\build\lib\test\test_socket.py", line 369, in _tearDown
    self.wait_threads.__exit__(None, None, None)
  File "D:\buildarea\pull_request.ware-win81-release.refleak\build\lib\contextlib.py", line 124, in __exit__
    next(self.gen)
  File "D:\buildarea\pull_request.ware-win81-release.refleak\build\lib\test\support\threading_helper.py", line 101, in wait_threads_exit
    gc_collect()
NameError: name 'gc_collect' is not defined

This one looks like a combo:

@erlend-aasland
Copy link
Contributor Author

Thanks for the review, @berkerpeksag & @vstinner !

@berkerpeksag
Copy link
Member

Thanks for the PR!

@erlend-aasland erlend-aasland deleted the fix-issue-40784 branch May 29, 2020 05:43
@vstinner vstinner added needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes labels May 29, 2020
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-20512 is a backport of this pull request to the 3.8 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label May 29, 2020
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2020
(cherry picked from commit c610d97)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@bedevere-bot
Copy link

GH-20513 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 29, 2020
miss-islington added a commit that referenced this pull request May 29, 2020
(cherry picked from commit c610d97)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
miss-islington added a commit that referenced this pull request May 29, 2020
(cherry picked from commit c610d97)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@vstinner
Copy link
Member

Thanks for fixing the encoding/copyright line ;-)

vstinner added a commit to fedora-python/cpython that referenced this pull request May 29, 2020
bpo-40784: Fix sqlite3 deterministic test (pythonGH-20448)

(cherry picked from commit c610d97)
(cherry picked from commit 00a240b)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request May 30, 2020
* 'master' of github.com:python/cpython: (497 commits)
  bpo-40061: Fix a possible refleak in _asynciomodule.c (pythonGH-19748)
  bpo-40798: Generate a different message for already removed elements (pythonGH-20483)
  closes bpo-29017: Update the bindings for Qt information with PySide2 (pythonGH-20149)
  bpo-39885: Make IDLE context menu cut and copy work again (pythonGH-18951)
  bpo-29882: Add an efficient popcount method for integers (python#771)
  Further de-linting of zoneinfo module (python#20499)
  bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros (pythonGH-20435)
  Indicate that abs() method accept argument that implement __abs__(), just like call() method in the docs (pythonGH-20509)
  bpo-39040: Fix parsing of email mime headers with whitespace between encoded-words. (pythongh-17620)
  bpo-40784: Fix sqlite3 deterministic test (pythonGH-20448)
  bpo-30064: Properly skip unstable loop.sock_connect() racing test (pythonGH-20494)
  Note the output ordering of combinatoric functions (pythonGH-19732)
  bpo-40474: Updated coverage.yml to better report coverage stats (python#19851)
  bpo-40806: Clarify that itertools.product immediately consumes its inpt (pythonGH-20492)
  bpo-1294959: Try to clarify the meaning of platlibdir (pythonGH-20332)
  bpo-37878: PyThreadState_DeleteCurrent() was not removed (pythonGH-20489)
  bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time (pythonGH-20493)
  bpo-40755: Add missing multiset operations to Counter() (pythonGH-20339)
  bpo-25920: Remove socket.getaddrinfo() lock on macOS (pythonGH-20177)
  bpo-40275: Fix test.support.threading_helper (pythonGH-20488)
  ...
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Jul 11, 2020
SQLite 3.38 introduced some breaking changes in Python 3.8.3 tests.
This is fixed by a patch for now (patch merged upstream)

python/cpython#20448
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants