This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [test] Use skipUnlessDarwin for tests specific to Darwin
ClosedPublic

Authored by mgorny on Nov 7 2020, 8:20 AM.

Details

Summary

Use skipUnlessDarwin decorator for tests that are specific to Darwin,
instead of skipIf... for all other platforms. This should make it clear
that these tests are not supposed to work elsewhere. It will also make
these tests stop repeatedly popping up while I look for tests that could
be fixed on the platform in question.

Diff Detail

Event Timeline

mgorny requested review of this revision.Nov 7 2020, 8:20 AM
mgorny created this revision.
teemperor accepted this revision.Nov 7 2020, 9:45 AM
teemperor added a subscriber: teemperor.

Given that these are all Objective-C tests, this LGTM.

(Out of scope for this patch, but it would probably be cleaner if we replace all the skipUnlessDarwin uses because of Obj-C with a dedicated decorator. In theory Obj-C is available on other systems).

This revision is now accepted and ready to land.Nov 7 2020, 9:45 AM

Given that these are all Objective-C tests, this LGTM.

(Out of scope for this patch, but it would probably be cleaner if we replace all the skipUnlessDarwin uses because of Obj-C with a dedicated decorator. In theory Obj-C is available on other systems).

I was kinda wondering why it is done like this, I've presumed some ObjC-specific parts of LLDB work only on Darwin.

Given that these are all Objective-C tests, this LGTM.

(Out of scope for this patch, but it would probably be cleaner if we replace all the skipUnlessDarwin uses because of Obj-C with a dedicated decorator. In theory Obj-C is available on other systems).

I was kinda wondering why it is done like this, I've presumed some ObjC-specific parts of LLDB work only on Darwin.

I assume that's indeed the case given how much LLDB assumes about the implementation details of the Obj-C runtime.

Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2020, 10:27 AM
emaste added a comment.Nov 7 2020, 1:09 PM

In theory Obj-C is available on other systems

Yes, in particular it should be possible on FreeBSD. I agree that an Obj-C-specific decorator would be best but it's a very low priority.

If we put all of the objc tests into objc-specific directories (it looks like most of them are there already, but I spotted two outliers in this patch), then we could mark those directories with the existing "objc" category, and use the existing category-based skipping mechanism. I don't remember seeing the code for setting the objc category, so I don't know how it works, but it shouldn't be hard to make it skip this category on platforms which don't (currently) support objc.

If we put all of the objc tests into objc-specific directories (it looks like most of them are there already, but I spotted two outliers in this patch), then we could mark those directories with the existing "objc" category, and use the existing category-based skipping mechanism. I don't remember seeing the code for setting the objc category, so I don't know how it works, but it shouldn't be hard to make it skip this category on platforms which don't (currently) support objc.

Actually, I've started working on this but I don't know how to reliably detect all Darwin platforms, with all the weird simulator environments and so on, from lit.

If we put all of the objc tests into objc-specific directories (it looks like most of them are there already, but I spotted two outliers in this patch), then we could mark those directories with the existing "objc" category, and use the existing category-based skipping mechanism. I don't remember seeing the code for setting the objc category, so I don't know how it works, but it shouldn't be hard to make it skip this category on platforms which don't (currently) support objc.

Actually, I've started working on this but I don't know how to reliably detect all Darwin platforms, with all the weird simulator environments and so on, from lit.

Could you just do this via a feature check? For libc++ we try compiling a dummy file (grep for checkLibcxxSupport) and we could do the same with a very simple Obj-C program including NSObject.h.

Actually that might not work for testing remote platforms that support Obj-C from non-ObjC hosts, so I guess we have to check if we're on Darwin.

If we put all of the objc tests into objc-specific directories (it looks like most of them are there already, but I spotted two outliers in this patch), then we could mark those directories with the existing "objc" category, and use the existing category-based skipping mechanism. I don't remember seeing the code for setting the objc category, so I don't know how it works, but it shouldn't be hard to make it skip this category on platforms which don't (currently) support objc.

Actually, I've started working on this but I don't know how to reliably detect all Darwin platforms, with all the weird simulator environments and so on, from lit.

Could you just do this via a feature check? For libc++ we try compiling a dummy file (grep for checkLibcxxSupport) and we could do the same with a very simple Obj-C program including NSObject.h.

Doing this from lit might be nice (save time doing the check only once), but the current state-of-the-art is to determine the supported categories from within dotest (checkLibcxxSupport and friends).

Actually that might not work for testing remote platforms that support Obj-C from non-ObjC hosts, so I guess we have to check if we're on Darwin.

Moreover, I don't think that would actually reflect the state of the source code. I'd expect that debugging ObjC binary on some other platform would require an appropriate ObjCLanguageRuntime plugin (which we currently don't have).