This is an archive of the discontinued LLVM Phabricator instance.

[lit, lldbsuite] Fixes for several tests LLDB tests for Python 3 or Windows
ClosedPublic

Authored by stella.stamenova on May 4 2018, 10:14 AM.

Details

Summary

In decorators.py, when opening streams, open them in text mode. In Py3, if they are not opened in text mode, the data is also expected to be binary, but we always use text data.
In TestLinuxCore, skip the tests that are not applicable on Windows
In the python api main.c, update the code to be compilable on Windows

Diff Detail

Repository
rL LLVM

Event Timeline

zturner added inline comments.
packages/Python/lldbsuite/test/python_api/hello_world/main.c
20 ↗(On Diff #145219)

On posix this is a number of seconds, but on Windows this is a number of milliseconds. So perhaps your macro above should be #define sleep(x) Sleep((x) * 1000). Thoughts?

packages/Python/lldbsuite/test/python_api/hello_world/main.c
20 ↗(On Diff #145219)

Good catch! The test is happily passing either way in our setup, but it will be better to update it. I'll make the change.

stella.stamenova marked 2 inline comments as done.
zturner accepted this revision.May 4 2018, 3:00 PM
This revision is now accepted and ready to land.May 4 2018, 3:00 PM

I don't have commit permissions. @zturner or @asmith, could you please commit this? Thanks!

I don't have commit permissions. @zturner or @asmith, could you please commit this? Thanks!

This would probably be as good a time as any for you to get them. I think you've made a number of useful contributions and probably passed the bar for getting commit access :)

asmith added a comment.May 4 2018, 6:15 PM

Will commit it Sunday if Zachary doesn’t do it before then.

stella.stamenova removed a reviewer: labath.
stella.stamenova removed a subscriber: llvm-commits.

Update the diff to contain all the changes (the last diff only had the hello world file).

This revision was automatically updated to reflect the committed changes.
labath added a subscriber: labath.May 8 2018, 12:58 AM
labath added inline comments.
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
41–69

I don't think it's fair to say that these "are not applicable to windows". These test debugging of (linux) core files, which should work same way on any platform (it already works on darwin, for example). So, it looks like there is something in the windows host layer preventing these from working. I'm not saying it has to be done now, but I think getting to the bottom of this would be very useful. Besides being able to run cross-platform tests, you'll probably also discover some genuine bugs that need fixing.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
41–69

I followed the pattern in the rest of the file (there are 3 more tests in this file all of which are disabled on Windows) - and it looked that all of the tests should either be enabled on Windows or not. If you think that they need to be enabled, I can file a bug and then add a reference in the file, so that they can all be fixed. Let me know.

Yes, the other tests are marked as skipped for the same reason as you have
skipped these: "for some reason, they don't work on windows". However, this
is definitely a bug. You should be able to inspect a linux core file on
windows (we have tests opening windows minidumps on linux, and they are
working fine).