This is an archive of the discontinued LLVM Phabricator instance.

[lldb-mi] Fix bugs in target-select-so-path.test
ClosedPublic

Authored by apolyakov on Sep 25 2018, 7:53 AM.

Details

Summary

This patch fixes hanging of the test in case of using python3, changes callback function that will be called if the timer ends, changes python interpreter to %python that is set up by llvm-lit.
Also, the test didn't work properly since it didn't contain a call of filecheck_proc.communicate(), that means that filecheck didn't run and its return code was equal to 0 in all cases.

Diff Detail

Event Timeline

apolyakov created this revision.Sep 25 2018, 7:53 AM
apolyakov updated this revision to Diff 166919.EditedSep 25 2018, 7:59 AM

Removed the shebang line since we call the script via python ... so that line doesn't matter.

tatyana-krasnukha added a comment.EditedSep 25 2018, 8:42 AM

Sorry, it was added in 3.2. Anyway, the test fails due to __init__() got an unexpected keyword argument 'pass_fds'.

Hmmm, maybe I mixing up something here, but the docs state pass_fds was added in 3.2:

New in version 3.2: The pass_fds parameter was added.

It seems I ran into an unrelated issue like this. If the revision works for others, you may ignore this.

apolyakov updated this revision to Diff 166936.Sep 25 2018, 9:31 AM

Changed python version required to use 'pass_fds' argument to 3.2.
I tested this patch with python 2.7 and 3.5.

Forget that the test calls default python, not which was passed to cmake. So, it is enough to replace (3,0) pair with (3,2) to fix.

lit/tools/lldb-mi/target/inputs/target-select-so-path.py
11

Here should be (3,2)

apolyakov marked an inline comment as done.Sep 25 2018, 9:43 AM

Now works with Python 3.1 too. LGTM.

apolyakov edited the summary of this revision. (Show Details)

Changed the test to use %python variable instead of python

teemperor accepted this revision.Sep 25 2018, 11:43 AM

Looks good and fixes the tests for me, so I think this is good to go. Thanks for the patch!

This revision is now accepted and ready to land.Sep 25 2018, 11:43 AM
labath accepted this revision.Sep 25 2018, 12:40 PM

LGTM. (BTW, python 3.1's EOL was 6 years ago)

This revision was automatically updated to reflect the committed changes.