[lldbsuite] Fix the mac version decorator to work on non-mac platforms

[stella.stamenova]

Summary

On non-mac platforms, mac_ver returns an empty string which when converted to LooseVersion has no "version" property. This causes a failure when the decorator executes. Instead, check whether the value returned from mac_ver is an empty string and avoid the LooseVersion comparison.

[shafik]

packages/Python/lldbsuite/test/decorators.py

194

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?

[stella.stamenova]

packages/Python/lldbsuite/test/decorators.py

194

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?

[jingham]

We do compose the decorators in a bunch of places (like Shafik's usage here). That will work more naturally if the categories that the decorators assert are as decoupled as possible. So the statement about macos version should only skip the test if the os is macos, and make no comment about other os's. That decouples the macos_version test from the oslist test, and allows something like the construct in the tests that Shafik wrote, which seems a pretty natural way to express "if on macOS, it has to be version > x, otherwise it has to be clang > y".

[stella.stamenova]

In D53208#1264056, @jingham wrote:

We do compose the decorators in a bunch of places (like Shafik's usage here). That will work more naturally if the categories that the decorators assert are as decoupled as possible. So the statement about macos version should only skip the test if the os is macos, and make no comment about other os's. That decouples the macos_version test from the oslist test, and allows something like the construct in the tests that Shafik wrote, which seems a pretty natural way to express "if on macOS, it has to be version > x, otherwise it has to be clang > y".

I updated the change to support that. Could you have a look?

[jingham]

That seems more convenient to me, it makes the mac version only relevant if you are on a mac.

packages/Python/lldbsuite/test/decorators.py

def fn(self):
                 'swig_version')) or (
                 _check_expected_version(
                     swig_version[0],
                     swig_version[1],
                     lldb.swig_version))
         skip_for_py_version = (
             py_version is None) or _check_expected_version(
             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(
                 macos_version[0],
                 macos_version[1],
                 platform.mac_ver()[0]))
 
         # For the test to be skipped, all specified (e.g. not None) parameters must be True.
         # An unspecified parameter means "any", so those are marked skip by default.  And we skip
         # the final test if all conditions are True.