Invalid syntax in inspect_fodder2.py (on Python 2.x)

[BenFilingBugs]

As pointed out, in #1 this install fine - apologies for misinterpreting the output. However we're using a complex toolchain (https://github.com/stackforge/anvil) to automatically convert a large number of python packages and their dependencies into system packages. This toolchain blows up when it gets to linecache2 because the default RPM toolchain attempts to byte-compile *.py, and it finds a syntax error in inspect_fodder2.py.

Please could you consider renaming that file to have a different extension? Otherwise we'll need to add a special case patch to our build, which is painful. It would also make the install not have any warnings, which is often considered desirable :-)

[harlowja]

+1 to this; seems like this is broken.

Here is a debug install via pip and python 2.6

$ pip install traceback2
Downloading/unpacking traceback2
  Downloading traceback2-1.4.0-py2.py3-none-any.whl
Downloading/unpacking linecache2 (from traceback2)
  Downloading linecache2-1.0.0-py2.py3-none-any.whl
Installing collected packages: traceback2, linecache2
Compiling /homes/harlowja/dev/os/taskflow/.venv/build/linecache2/linecache2/tests/inspect_fodder2.py ...
SyntaxError: ('invalid syntax', ('/homes/harlowja/dev/os/taskflow/.venv/build/linecache2/linecache2/tests/inspect_fodder2.py', 102, 25, 'def keyworded(*arg1, arg2=1):\n'))

Successfully installed traceback2 linecache2
Cleaning up...

It does seem like it gets installed, but it also seems like this should not be happening....

[rbtcollins]

So, I think the most appropriate thing to do here is for you to split the package in your tooling and elide the tests. You likely don't want those installed by an RPM installer anyway.

The file in question is test data used by the test suite, and I'm worried that the scale of changes needed to make it be optionally installed and have the test suite skip tests if its not installed, but never skip them when testing in source would be larger than the possible benefits - that or fragile and a maintenance burden. That said, if you wish to supply a tasteful patch that addresses the issue for you, I can certainly assess whether it is an issue upstream-wise.

The files contents are what they are because they are the upstream (cPython master) inspect test files, reused by the linecache tests. Another way of solving the issue would be to patch the cPython linecache tests to create files just-in-time, or use its own test file, or some such - we'd need to assess how much coverage we'd lose as a result.

[tarekziade]

Why not renaming the .py files to .pytest ? I mean, they are used in the test where you can rename them on the fly just when running the tests. That'd fix the issue

[rbtcollins]

As I said, I'm very happy to consider patches. Renaming as a specific strategy I'm skeptical of because its a spurious delta vs upstream cPython - I think the best thing to do would be to generate the files needed as temp files just-in-time, which is something we can commit to the stdlib test suite, and then we'll know exactly what coverage we have, rather than guessing based on what inspect needs from its test files.

[tarekziade]

As I said, I'm very happy to consider patches

ok, I'll push a PR

I think the best thing to do would be to generate the files needed as temp files just-in-time
which something we can commit to the stdlib test suite

I am skeptikal this can land in the stdlib since it's for backporting purpose but if it can it's the best solution for sure.

[tarekziade]

how do we run tests ? I am confused because I see different things (testr, unitest2, etc..)

[rbtcollins]

Unit2
On 17 Mar 2015 22:43, "Tarek Ziade" notifications@github.com wrote:

how do we run tests ? I am confused because I see different things (testr,
unitest2, etc..)

—
Reply to this email directly or view it on GitHub
#2 (comment)
.

[tarekziade]

see #4

[ddriddle]

I am having similar issues. I do not use the same toolchain as @tarekziade but I do use setuptools bdist_rpm command. If I run this command I receive the following two fatal errors.

$ python setup.py bdist_rpm
...

RPM build errors:
    File not found: /services/scratch/ddriddle/src/linecache2/build/bdist.linux-x86_64/rpm/BUILDROOT/linecache2-1.0.0-1.noarch/opt/rh/python27/root/usr/lib/python2.7/site-packages/linecache2/tests/inspect_fodder2.pyc
    File not found: /services/scratch/ddriddle/src/linecache2/build/bdist.linux-x86_64/rpm/BUILDROOT/linecache2-1.0.0-1.noarch/opt/rh/python27/root/usr/lib/python2.7/site-packages/linecache2/tests/inspect_fodder2.pyo
error: command 'rpmbuild' failed with exit status 1

Is it necessary to have the tests directory installed? It does not seem necessary. I have created a patch that adds a MANIFEST.in file to prune the tests directory (See #5.) This fix solves my problems.

[rbtcollins]

The sdist should include everything - wheels (and rpms etc) are a reasonable place to prune out the test suite when one doesn't want it included.

That said, I've already offered to help shepard a patch in the master repo (cPython) to address this, but AFAIK noone has put one up.

[ddriddle]

@rbtcollins I would be happy to submit a ticket and patch to the cPython repo unless @tarekziade would prefer to do it. I will work on that tomorrow unless @tarekziade objects.

[ddriddle]

@rbtcollins I submitted a patch to cPython. See issue #24054.

[andy-maier]

I ran into what I think is the same issue, with Python 2.6 on RHEL 6.7.

In the cpython repo, the patch by @ddriddle has made it into a change set fc56a0300cd4, which is only in the default branch at this point, i.e. it should be included in Python 3.6 (if I understand the process correctly).

That patch simply removed the test that uses the files specified in the TESTS variable (which includes the offending inspect_fodder2.py file). The remaining test (that was there before) is using the linecache.py module itself, and the abc.py module. That's certainly one way of addressing the issue.

Given that this apparently has been accepted as the upstream solution, would it make sense to also integrate that solution into linecache2, for consistency with cpython?