This is an archive of the discontinued LLVM Phabricator instance.

[TLI checker] Add more tests
ClosedPublic

Authored by probinson on Dec 1 2021, 2:59 PM.

Details

Summary

D114478 identified testing gaps; this patch fills them.

Also, the response-file testing moved to the new test that does all the other multi-file tests; it made more sense there.

Diff Detail

Event Timeline

probinson requested review of this revision.Dec 1 2021, 2:59 PM
probinson created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2021, 2:59 PM

Nice use of numeric variables by the way!

llvm/test/tools/llvm-tli-checker/Inputs/empty-coff.yaml
1 ↗(On Diff #391145)

Any particular reason this and empty-elf.yaml aren't inlined directly in the corresponding tests that use them?

llvm/test/tools/llvm-tli-checker/error-cases.test
18

Any particular reason this line is separated from the other checks?

20–23

I'd be inclined to include the full message, i.e. something like error: [[FILE]]: not found (or equivalent).

24

Nit: looks like you've got an extra blank line at EOF?

llvm/test/tools/llvm-tli-checker/multi-file.yaml
35

I read / in this context as an "or" not "and" (and I could imagine others might initially read it as a directory separator).

119

Nit: looks like there's an additional blank line here?

llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
46–47

That's unfortunate. I don't suppose there's a way of doing some sort of fancy regex is there? (I can't immediately think of any).

probinson added inline comments.Dec 2 2021, 7:22 AM
llvm/test/tools/llvm-tli-checker/Inputs/empty-coff.yaml
1 ↗(On Diff #391145)

I was originally thinking I'd include them in the archive test, but then I didn't, so yes they should be inlined into the tests that use them.

llvm/test/tools/llvm-tli-checker/error-cases.test
18

Only that it isn't combined with other checks, and all the others are in combinations.

20–23

Right, I can use -D on the FileCheck command line to do that.

llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
46–47

A regex can have an exact count, but I don't think we can substitute a FileCheck numeric expression inside a regex, and that's what it would require.

probinson updated this revision to Diff 391326.Dec 2 2021, 7:41 AM
probinson marked 6 inline comments as done.

Address review comments.

This revision is now accepted and ready to land.Dec 2 2021, 7:43 AM
This revision was landed with ongoing or failed builds.Dec 2 2021, 8:19 AM
This revision was automatically updated to reflect the committed changes.