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

test_history failure #11372

Closed
toddrme2178 opened this issue Oct 8, 2018 · 16 comments · Fixed by #13276
Closed

test_history failure #11372

toddrme2178 opened this issue Oct 8, 2018 · 16 comments · Fixed by #13276
Labels
Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue. help wanted

Comments

@toddrme2178
Copy link
Contributor

toddrme2178 commented Oct 8, 2018

I am trying to package ipython 7.0.1 for openSUSE and I am getting the following error in the unit tests:

======================================================================
FAIL: IPython.core.tests.test_history.test_history
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/lib/python3.6/site-packages/IPython/core/tests/test_history.py", line 113, in test_history
    newhist[3]])
AssertionError: Lists differ: [(1, [51 chars]rn test'), (1, 3, "b='€Æ¾÷ß'"), (2, 1, 'z=5'), (2, 3, "k='p'")] != [(1, [51 chars]rn test'), (1, 3, "b='€Æ¾÷ß'"), (2, 3, "k='p'"), (2, 4, 'z=5')]

First differing element 3:
(2, 1, 'z=5')
(2, 3, "k='p'")

  [(1, 1, 'a=1'),
   (1, 2, 'def f():\n    test = 1\n    return test'),
   (1, 3, "b='€Æ¾÷ß'"),
-  (2, 1, 'z=5'),
-  (2, 3, "k='p'")]
?                 ^

+  (2, 3, "k='p'"),
?                 ^

+  (2, 4, 'z=5')]
-------------------- >> begin captured stdout << ---------------------
def f():
    test = 1
    return test
b='€Æ¾÷ß'
The following commands were written to file `/tmp/tmphhgt1b7l/tmpsytny8bh/test4.py`:
a=1
def f():
    test = 1
    return test
b='€Æ¾÷ß'

--------------------- >> end captured stdout << ----------------------
    """Fail immediately, with the given message."""
>>  raise self.failureException('Lists differ: [(1, [51 chars]rn test\'), (1, 3, "b=\'€Æ¾÷ß\'"), (2, 1, \'z=5\'), (2, 3, "k=\'p\'")] != [(1, [51 chars]rn test\'), (1, 3, "b=\'€Æ¾÷ß\'"), (2, 3, "k=\'p\'"), (2, 4, \'z=5\')]\n\nFirst differing element 3:\n(2, 1, \'z=5\')\n(2, 3, "k=\'p\'")\n\n  [(1, 1, \'a=1\'),\n   (1, 2, \'def f():\\n    test = 1\\n    return test\'),\n   (1, 3, "b=\'€Æ¾÷ß\'"),\n-  (2, 1, \'z=5\'),\n-  (2, 3, "k=\'p\'")]\n?                 ^\n\n+  (2, 3, "k=\'p\'"),\n?                 ^\n\n+  (2, 4, \'z=5\')]')
    

----------------------------------------------------------------------

It looks like the last two list elements have switched places but I don't know why that might be the case.


EDIT:

We can likely fix this issue in two steps:

  1. mark the test as skip (or known fail) for the range of sqlite versions that appear to be affected.
  2. actually figure out if this is a change in behavior worth fixing or if the test should be updated accordingly.
@Carreau
Copy link
Member

Carreau commented Oct 10, 2018

Which version of Python are you running that on ? Look at the version of sqlite as well.

@toddrme2178
Copy link
Contributor Author

toddrme2178 commented Oct 10, 2018

Here are the python and sqlite versions:

libsqlite3-0-3.25.0-1.1
python3-3.6.5-3.4

And some other I guess might be relevant:

bash-4.4-107.1
coreutils-8.30-1.2
gcc8-8.2.1+r264010-1.1
gettext-runtime-mini-0.19.8.1-9.1
gettext-tools-mini-0.19.8.1-9.1
glibc-2.27-6.1
libdb-4_8-4.8.30-36.5
libgdbm5-1.14.1-1.6
libgdbm_compat4-1.14.1-1.6
libncurses6-6.1-6.5
libreadline7-7.0-2.1
libstdc++6-8.2.1+r264010-1.1
libzmq5-4.2.5-2.1
linux-glibc-devel-4.18-1.1
ncurses-utils-6.1-6.5
python3-ipython_genutils-0.2.0-2.1
python3-jedi-0.12.1-1.1
python3-jsonschema-2.6.0-2.2
python3-jupyter_client-5.2.3-4.1
python3-jupyter_core-4.4.0-3.1
python3-jupyter_ipyparallel-6.2.2-6.27
python3-jupyter_ipywidgets-7.4.2-10.1
python3-jupyter_nbconvert-5.4.0-15.11
python3-jupyter_nbformat-4.4.0-3.1
python3-jupyter_notebook-5.7.0-8.3
python3-jupyter_qtconsole-4.4.1-5.2
python3-jupyter_widgetsnbextension-3.4
python3-nose-1.3.7-10.1
python3-pexpect-4.6.0-2.1
python3-pyparsing-2.2.0-2.1
python3-pyzmq-17.1.2-1.1
python3-setuptools-40.4.3-1.1
python3-simplegeneric-0.8.1-8.4
python3-simplejson-3.16.1-1.1
python3-six-1.11.0-4.1
python3-terminado-0.8.1-3.1
python3-testpath-0.4.1-4.1
python3-traitlets-4.3.2-4.1
python3-wcwidth-0.1.7-2.1

@toddrme2178
Copy link
Contributor Author

The problem also appears to be happening in version 6.5. It looks like the problem first occurred when we switched from sqlite3 3.24.0 to 3.25.0.

@Carreau
Copy link
Member

Carreau commented Oct 13, 2018

actually for z=5 the second number in the tuple has changed. it's the line number (AFAICT).

So why 4 instead of 1... ? It may be that when we request unique we only request uniq wrt the 3rd column and sqlite is happy to change it's internal behavior and uniquify before sorting, thus returning the second iteration of z=5 ?

@toddrme2178
Copy link
Contributor Author

I missed that. But I don't really know anything about SQL so I don't know why it might be happening. I have confirmed the problem still occurs with sqlite 3.25.2, the latest version.

@Carreau
Copy link
Member

Carreau commented Oct 14, 2018 via email

@toddrme2178
Copy link
Contributor Author

I don't know how serious the bug is so I would defer to your judgement. Of course a real fix is preferable, but if you think the problem is minor enough we can go with a known fail for the time being.

@Carreau Carreau added this to the 7.1 milestone Oct 15, 2018
@Carreau Carreau added the Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue. label Oct 15, 2018
@Carreau
Copy link
Member

Carreau commented Oct 15, 2018

@LucianaMarques you were looking for something easy, that should't be too hard to add a "@skip_if" with a condition like... sqlite3.sqlite_version_info > (x, y, z)

We can delay fixing that to later.

@LucianaMarques
Copy link
Contributor

@Carreau thank you, I'll give it a try today!

@LucianaMarques
Copy link
Contributor

@Carreau I'm having trouble to use @skip_if, I have never used it and found no docs on it (or I'm not searching for it properly... ), do you have any tutorials/docs recommendations?

@dsblank
Copy link
Contributor

dsblank commented Oct 15, 2018

@LucianaMarques Whenever I start coding in a new area, I try to find examples in the current code. If you are on a Unix-based system you could use grep to search the codebase for examples of "skip_if" and, by analogy, apply to the current problem.

@Carreau
Copy link
Member

Carreau commented Oct 15, 2018

I think it's without underscore on IPython codebase.

For example there.

@dsblank have you tried RipGrep ? Really good: skip .git by default, search recursively by default, color highlight, and filter by file types. Fr example search for skipif only in python files:

$ rg @skipif -tpy
IPython/extensions/tests/test_autoreload.py
133:    @skipif(sys.version_info < (3, 6))

IPython/core/tests/test_interactiveshell.py
531:    @skipif(not hasattr(signal, 'SIGALRM'))

IPython/lib/tests/test_latextools.py
47:@skipif_not_matplotlib
62:@skipif_not_matplotlib

IPython/lib/tests/test_display.py
182:@skipif_not_numpy
 ~/dev/ipython[master ✗] $ rg @skip_if -tpy
IPython/lib/tests/test_clipboard.py
7:@skip_if_no_x11

IPython/utils/tests/test_path.py
102:@skip_if_not_win32
117:@skip_if_not_win32
157:@skip_if_not_win32
377:    @skip_if_not_win32
468:    @skip_if_not_win32

... and 10x faster on my machine.

@LucianaMarques
Copy link
Contributor

Thank you @Carreau and @dsblank , your suggestions were really helpful, I don't think I was previously familiar with this command.

I'll come back with a pull request soon.

@LucianaMarques
Copy link
Contributor

As promissed, my pull request.

@Carreau
Copy link
Member

Carreau commented Oct 18, 2018

Skip_if has been added, I'll left this one open to go to the root of the issue.

Thanks !

@Carreau Carreau modified the milestones: 7.1, 7.2 Oct 27, 2018
@Carreau Carreau modified the milestones: 7.2, 7.3 Dec 6, 2018
@Carreau Carreau removed this from the 7.3 milestone Jan 24, 2019
@jcbollinger
Copy link

For future reference and maybe a real fix sometime, this appears to be happening because ipython uses an SQL "GROUP BY" clause to squash duplicates, while also selecting and ordering by columns that are neither grouping columns nor aggregate functions (session and line). Neither SQL nor sqlite specify from which row of each resulting group the values for those so-called "bare" columns will be drawn, and apparently sqlite's actual behavior changed in that regard.

I tried a couple of variations on the generated SQL, with no success against sqlite3 3.26.0. There are certainly ways to do it, but there's a question of the size of the changes required and their effect on search performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue. help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants