This is an archive of the discontinued LLVM Phabricator instance.

[clang][NFC] reformats the SARIF diagnostic test so it's human readable
AcceptedPublic

Authored by cjdb on Mar 2 2023, 12:38 PM.

Details

Summary

The current FileCheck test output is very difficult to read, which makes
adding new tests to SARIF output rather painful. I'd ideally like to
replace this test with a far more robust form of JSON testing, but am
unable to allocate resources to paying down the tech debt at present.

Diff Detail

Event Timeline

cjdb created this revision.Mar 2 2023, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 12:38 PM
cjdb requested review of this revision.Mar 2 2023, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 12:38 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cjdb added inline comments.Mar 2 2023, 12:41 PM
clang/test/Frontend/sarif-diagnostics.cpp
31

This was split out for future tests where the COMMON stuff isn't written to file. Perhaps I should check it in as STDERR?

43

Aside from the reformatting, the only change to this file is the use of regex for the URIs.

cjdb added inline comments.Mar 2 2023, 12:47 PM
clang/test/Frontend/sarif-diagnostics.cpp
31

Further thoughts: everything is common, it's a case of where it's output that's not the same. I think with this in mind I should absolutely change COMMON to STDERR, and possibly CHECK to SARIF.

cjdb updated this revision to Diff 501955.Mar 2 2023, 1:00 PM

renames check identifiers

cjdb updated this revision to Diff 501982.Mar 2 2023, 2:09 PM

removes redundant line

cjdb added a subscriber: denik.Mar 2 2023, 3:25 PM
cjdb updated this revision to Diff 502019.Mar 2 2023, 4:10 PM

removes something from a future commit

This should be the final alteration pre-review.

aaron.ballman accepted this revision.Mar 3 2023, 6:53 AM

LGTM, this is incremental progress. Hopefully we won't be adding too many more RUN lines to this file though (sticking too many tests into one file is also tech debt).

This revision is now accepted and ready to land.Mar 3 2023, 6:53 AM
cjdb added a comment.Mar 3 2023, 10:32 AM

LGTM, this is incremental progress. Hopefully we won't be adding too many more RUN lines to this file though (sticking too many tests into one file is also tech debt).

Thanks! 100% of the RUN lines I intend to add are bug regressions, but there will be more RUN lines in the future.

shafik accepted this revision.Mar 6 2023, 10:52 AM