[PATCH] D96371: [lit] Add --ignore-fail
Joel E. Denny via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 12 10:21:12 PST 2021
jdenny added a comment.
In D96371#2559299 <https://reviews.llvm.org/D96371#2559299>, @jhenderson wrote:
> In D96371#2556413 <https://reviews.llvm.org/D96371#2556413>, @thopre wrote:
>
>> I'm still not sure, I think a failure is a powerful motivator to fix something but patch LGTM otherwise. @jhenderson what do you think?
>
> Sorry, I'm a little bit busy with other reviews, so haven't had a chance to dig into this properly yet.
>
> If I follow what you're saying correctly, rather than check-all executing lit in a single run across multiple directories, in some cases, it is spawning a new lit process to run some test subset. Is that right?
Yes. In the current case, that subset is for runtimes specified in `LLVM_ENABLE_RUNTIMES`. I believe the last case I saw was `compiler-rt/test/gwp_asan/unit`, but I think that one has since been fixed.
> If so, I wonder if there's a better solution of getting check-all to actually run all the tests as one big test.
Agreed.
However, especially after these responses, I see `--ignore-fail` as a temporary workaround. By "temporary", I mean until the cmake config issue is fixed. I see it as a permanent lit feature because this isn't the first time our cmake config has had this issue, and I'm betting it won't be the last time. Otherwise, I wouldn't have proposed this patch upstream.
In D96371#2556413 <https://reviews.llvm.org/D96371#2556413>, @thopre wrote:
> I'm still not sure, I think a failure is a powerful motivator to fix something
Good point. Even so, I think there is still motivation to fix such cmake issues: because the exit status is zero when using `--ignore-fail`, trying to detect failures in a script can be ugly.
I've updated the review summary to account for everyone's feedback, which has honed my view. Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96371/new/
https://reviews.llvm.org/D96371
More information about the llvm-commits
mailing list