======================================================================
FAIL: test_dump_traceback_threads (test.test_faulthandler.FaultHandlerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/test/test_faulthandler.py", line 365, in test_dump_traceback_threads
    self.check_dump_traceback_threads(None)
  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/test/test_faulthandler.py", line 361, in check_dump_traceback_threads
    self.assertRegex(output, regex)
AssertionError: Regex didn't match: '^Thread 0x[0-9a-f]+:\n(?:  File ".*threading.py", line [0-9]+ in [_a-z]+\n){1,3}  File "<string>", line 23 in run\n  File ".*threading.py", line [0-9]+ in _bootstrap_inner\n  File ".*threading.py", line [0-9]+ in _bootstrap\n\nCurrent thread XXX:\n  File "<string>", line 10 in dump\n  File "<string>", line 28 in <module>$' not found in 'Thread 0x0000000103a4a000:\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 184 in wait\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 330 in wait\n  File "<string>", line 23 in run\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 639 in _bootstrap_inner\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 616 in _bootstrap\n\nCurrent thread XXX:\n  File "<string>", line 10 in dump\n  File "<string>", line 28 in <module>'

======================================================================
FAIL: test_dump_traceback_threads_file (test.test_faulthandler.FaultHandlerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/test/test_faulthandler.py", line 369, in test_dump_traceback_threads_file
    self.check_dump_traceback_threads(filename)
  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/test/test_faulthandler.py", line 361, in check_dump_traceback_threads
    self.assertRegex(output, regex)
AssertionError: Regex didn't match: '^Thread 0x[0-9a-f]+:\n(?:  File ".*threading.py", line [0-9]+ in [_a-z]+\n){1,3}  File "<string>", line 23 in run\n  File ".*threading.py", line [0-9]+ in _bootstrap_inner\n  File ".*threading.py", line [0-9]+ in _bootstrap\n\nCurrent thread XXX:\n  File "<string>", line 8 in dump\n  File "<string>", line 28 in <module>$' not found in 'Thread 0x0000000105215000:\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 184 in wait\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 330 in wait\n  File "<string>", line 23 in run\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 639 in _bootstrap_inner\n  File "/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/...", line 616 in _bootstrap\n\nCurrent thread XXX:\n  File "<string>", line 8 in dump\n  File "<string>", line 28 in <module>'

----------------------------------------------------------------------
Ran 27 tests in 20.582s

FAILED (failures=2)
test test_faulthandler failed

The two failing tests call check_dump_traceback_threads which has a regex to find threading.py in the traceback but the path is truncated in the traceback so "threading.py" doesn't appear.

This might be obvious to some people, but for background purposes, this seems to be where the file name gets truncated (in the dump_frame() function):

    write(fd, "\"", 1);
    dump_ascii(fd, code->co_filename);
    write(fd, "\"", 1);

http://hg.python.org/cpython/file/ddf15cd9be4a/Python/traceback.c#l564

Out of curiosity, I wonder if we have a test to check the formatting of tracebacks for long file names, which would be nice to have.  I didn't see one.

Without investigating further, my instinct would be to prefer to truncate in the middle, if possible, otherwise truncate the head of the path, rather than truncate the tail.

faulthandler has arbitrary limits to avoid an unlimited loop if some
data are corrupted.

#define MAX_STRING_LENGTH 100
#define MAX_FRAME_DEPTH 100
#define MAX_NTHREADS 100

The string limit is maybe too short. MAX_STRING_LENGTH can be set to
500 characters, what do you think?

We may also add a function to change these limits at runtime.

> Without investigating further, my instinct would be to prefer to truncate in the middle,
> if possible, otherwise truncate the head of the path, rather than truncate the tail.

faulthandler does not trust anything, including the length of the
string. Change the limit to 500 characters should be safer.

Remember that faulthandler is called when something really bad
happens, like a segfault or an illegal instruction.

faulthandler may be less strict when it is called explicitly, ex:
faulthandler.dump_traceback().

> We may also add a function to change these limits at runtime.

This sounds like a good idea.  Is there already an issue for this?  If not, I could create one.

Regardless of whether MAX_STRING_LENGTH is increased, my instinct is that the test should work for long install paths independent of the setting's value.  I wonder if there is a way to adjust the test so that the file name can still be checked in such a scenario (i.e. so that we do not simply add "or '...'" logic to the regular expression).

>> We may also add a function to change these limits at runtime.
>
> This sounds like a good idea.  Is there already an issue for this?  If not, I could create one.

There is no such issue. You can create it: add me to the nosy list.

> Regardless of whether MAX_STRING_LENGTH is increased, my instinct is that the test should work for long install paths independent of the setting's value.  I wonder if there is a way to adjust the test so that the file name can still be checked in such a scenario (i.e. so that we do not simply add "or '...'" logic to the regular expression).

The purpose of the test is to check that the traceback is correct:
it's important to check that the filename ends with "threading.py". If
you don't know in which file the bug occured, the traceback is
useless.

Changing the MAX_STRING_LENGTH should be enough to fix this issue
before Python 3.3 final.

> There is no such issue. You can create it: add me to the nosy list.

I created issue 15479 to allow the limits to be changed at runtime.

> The purpose of the test is to check that the traceback is correct:
> it's important to check that the filename ends with "threading.py".

I agree with you.  That is why I argued against including "or '...'" logic in the current test.

> Changing the MAX_STRING_LENGTH should be enough to fix this issue
> before Python 3.3 final.

That is fine.  However, I should also point out that the current test is not sufficient to test on all systems/installs that this issue has been fixed.  It would be good to add such a test (i.e. one that can, for example, simulate a long install path).

Here is a patch to the tests that allows the tests to pass when the install path is long while still checking that file names show up correctly in the traceback.

One advantage of this patch's approach is that it provides a way to test the rendering of long paths on all systems/installs.  Simply create a test case by setting the scriptname to, for example--

scriptname = 'test_long_file_path_' + 200 * 'x'

This patch is just an illustration for discussion purposes and is not meant as a final patch.

Also, this patch does not fix the issue of the file names of long paths not getting rendered (which can be addressed by setting the limit to 500 as we discussed).  However, as I have noted this patch provides a way to test such a fix.

New changeset ff7fc6a91212 by Victor Stinner in branch 'default':
Issue #15463: the faulthandler module truncates strings to 500 characters,
http://hg.python.org/cpython/rev/ff7fc6a91212

> Issue #15463: the faulthandler module truncates strings to 500 characters,

It should be able for most setup. See the issue #15479 if you consider that something better should be implemented.

Long path names are handled differently on each platform, I don't want to add a test checking a path longer than 500 characters. You might add a new test for truncated path if limits become configurable (#15479).

I consider this issue as resolved.

Shouldn't a test be added to check that the issue has been fixed though (i.e. a test with a path longer than the original 100 characters)?

I provided a patch to show how this can easily be done.

I would be happy to write the test.

> I provided a patch to show how this can easily be done.

Your patch does not create a path longer than 100 characters. I didn't want to write such test, but I can review a patch adding such test.

The limit is not only applied on the filename, but also on the function name. It is maybe easier (more portable) to declare a function with a very long name:

exec(compile("def " + "f"*600 + "(): raise ValueError()\n" + "f"*600 + "()", "a", "exec"))

BTW, the applied patch does fix the problem originally reported. Thanks.

> I didn't want to write such test, but I can review a patch adding such test.

Thank you.  I will provide a patch for your review.

I think a test for the path may be better as that was the original issue experienced.

Attaching patch with test case for long script path.

This test failed on my system before the fix and passes after.

Is it okay for me to reopen this issue to review the patch to add a test case for file paths longer than 100 characters, or should I create a new issue for that?

New changeset 6e03f9b72c61 by Victor Stinner in branch 'default':
Issue #15463: Write a test for faulthandler truncating the name of functions
http://hg.python.org/cpython/rev/6e03f9b72c61

I added a test checking that faulthandler truncates strings to MAX_STRING_LENGTH characters using a very long function name. I consider this issue as done.

> I didn't want to write such test, but I can review a patch adding such test.

Victor, did you see and review my patch?  There is currently no test to check that a long file path (longer than 100 characters) will render, which was the original issue that was fixed.  The patch provides such a test.

> Victor, did you see and review my patch? There is currently no test to check that a long file path (longer than 100 characters) will render, which was the original issue that was fixed.  The patch provides such a test.

Yes, I saw your patch, but I don't want to create very long filename.
Not all platforms support paths longer than 500 characters. It is
exactly the same code to display function name and file names. I
prefer to use a very long function name, it's simple and portable.

My patch is different than yours: it checks that the string is
truncated, not that string shorter than the limit are not truncated.

> Yes, I saw your patch, but I don't want to create very long filename.
Not all platforms support paths longer than 500 characters.

I think you may have a slight misunderstanding.  My patch was not meant to test paths over 500 characters.  It tests a path *under* 500 characters (but slightly over 100 characters).  And it has the same style and portability characteristics as the existing ".*threading.py" tests.

> My patch is different than yours: it checks that the string is
truncated, not that string shorter than the limit are not truncated.

Yes, your patch is different in two ways. :) It tests function names instead of paths, and length > 500 instead of 100 < length < 500.  I think there is value in adding a file path test because code changes over time, so it may not always be using the same code path as function names.

I agree that it is okay to leave out a file path test for longer than 500 characters for the portability reasons you stated.