[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.


================
Comment at: packages/Python/lldbsuite/test/decorators.py:194
             py_version[0], py_version[1], sys.version_info)
-        skip_for_macos_version = (macos_version is None) or (
+        skip_for_macos_version = (macos_version is None) or (platform.mac_ver()[0] == "") or (
             _check_expected_version(
----------------
shafik wrote:
> If I am reading the following code correctly this will default to skipping when `platform.mac_ver()[0]  == ""` shouldn't it be the other way around? 
Yes, you are reading it correctly. I can see either as being "right" - the question is how the decorator is supposed to be used.

Usually, it is paired with a skipUnlessDarwin (or similar), so skipping if it is not mac is "right".

On the other hand, for the test you added, I think you wanted it to run on other platforms and on mac only if the version requirement was met, so we should not skip if the platform is not Darwin. Was that your goal?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53208