This is an archive of the discontinued LLVM Phabricator instance.

[libc][obvious] only test FILE on working platforms
ClosedPublic

Authored by michaelrj on Mar 23 2022, 5:02 PM.

Details

Summary

The main code for the FILE struct is only enabled on platforms that it
works on, but before this patch the tests were included unconditionally.

Diff Detail

Event Timeline

michaelrj created this revision.Mar 23 2022, 5:02 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 23 2022, 5:02 PM
michaelrj requested review of this revision.Mar 23 2022, 5:02 PM
sivachandra accepted this revision.Mar 23 2022, 5:10 PM

I thought a test was already skipped if it had missing deps. But, it looks like we skip a test only if it has skipped entrypoints: https://github.com/llvm/llvm-project/blob/main/libc/cmake/modules/LLVMLibCTestRules.cmake#L32.

While this change LGTM for now, I think a more robust way to skip tests would be by skipping if there are any missing deps. It can be done as a separate cleanup.

This revision is now accepted and ready to land.Mar 23 2022, 5:10 PM
lntue accepted this revision.Mar 24 2022, 4:24 AM

I thought a test was already skipped if it had missing deps. But, it looks like we skip a test only if it has skipped entrypoints: https://github.com/llvm/llvm-project/blob/main/libc/cmake/modules/LLVMLibCTestRules.cmake#L32.

While this change LGTM for now, I think a more robust way to skip tests would be by skipping if there are any missing deps. It can be done as a separate cleanup.

+1 on skipping on any missing dep in a separate cleanup.

This revision was automatically updated to reflect the committed changes.