This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][test] - Cleanup and split tests in tools/llvm-readobj folder.
ClosedPublic

Authored by grimar on Dec 10 2019, 6:47 AM.

Details

Summary

tools/llvm-readobj currently contains tests that are either general for
all file types or that mix file types inside. This patch refactors
these test and leaves only general tests in that folder. All other
tests were moved to ELF/COFF/MachO and wasm accordingly.

I tried to minimize amount of changes, so most of the test parts
remained unchanged (except comments added/changed).
Any further refactorings and improvements for
particular tests should be done independently from this patch.

Diff Detail

Event Timeline

grimar created this revision.Dec 10 2019, 6:47 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Dec 11 2019, 3:52 AM
llvm/test/tools/llvm-readobj/COFF/hex-dump.test
3

Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?

5

This comment doesn't make sense any more.

llvm/test/tools/llvm-readobj/COFF/sections-ext.test
2–3

The comment uses the long-form, but the test only uses the short-form. We should test both probably.

llvm/test/tools/llvm-readobj/COFF/sections.test
2

Delete "do"

llvm/test/tools/llvm-readobj/COFF/symbols.test
2

Delete "do"

llvm/test/tools/llvm-readobj/ELF/hex-dump.test
42

in an error...

llvm/test/tools/llvm-readobj/ELF/sections-ext.test
7

Either add a test showing that '-st' and '--st' are equivalent or update the comment to use double-dashes for consistency.

llvm/test/tools/llvm-readobj/ELF/sections.test
2

Delete "do".

5

--check-prefix (also below)

llvm/test/tools/llvm-readobj/ELF/symbols.test
3

another -> other
checks -> check
partucular -> particular (though I'd probably use "specific")

13

-elf-output-style -> --elf-output-style

llvm/test/tools/llvm-readobj/MachO/hex-dump.test
3

Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?

llvm/test/tools/llvm-readobj/MachO/relocations.test
2

Delete "do".

5

--check-prefix (also below)

llvm/test/tools/llvm-readobj/MachO/sections-ext.test
2–3

The comment uses the long-form, but the test only uses the short-form. We should test both probably.

6

--check-prefix (also below)

llvm/test/tools/llvm-readobj/MachO/sections.test
2

Delete "do"

321

--check-prefix (here and below)

llvm/test/tools/llvm-readobj/archive.test
4–10

%t.dir

llvm/test/tools/llvm-readobj/wasm/hex-dump.test
3

Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?

llvm/test/tools/llvm-readobj/wasm/symbols.test
2

for the --symbols

grimar updated this revision to Diff 233327.Dec 11 2019, 5:26 AM
grimar marked 27 inline comments as done.
  • Addressed review comments.
llvm/test/tools/llvm-readobj/COFF/hex-dump.test
3

I've added a single line check for a normal switch.

BTW, I think almost every test that was splitted can/should be improved.
The main intention of this patch is just to split the tests here and move them to
corresponding folders to open the road for further changes though.

5

Removed.

llvm/test/tools/llvm-readobj/COFF/sections-ext.test
2–3

Done.

llvm/test/tools/llvm-readobj/MachO/hex-dump.test
3

I've added a test.

llvm/test/tools/llvm-readobj/MachO/sections-ext.test
2–3

I've added tests for long-form.

llvm/test/tools/llvm-readobj/wasm/hex-dump.test
3

I've aaded a switch and updated the comment.

jhenderson accepted this revision.Dec 11 2019, 7:55 AM

LGTM, with one nit.

llvm/test/tools/llvm-readobj/COFF/symbols.test
2

Ping? This got missed.

This revision is now accepted and ready to land.Dec 11 2019, 7:55 AM
MaskRay accepted this revision.Dec 11 2019, 9:12 PM
MaskRay added inline comments.
llvm/test/tools/llvm-readobj/MachO/sections-ext.test
14

Optional: maybe we can delete --expand-relocs to make the tests less verbose. Some llvm-readobj commands above do not use this option.

llvm/test/tools/llvm-readobj/archive.test
83

sections: []

llvm/test/tools/llvm-readobj/basic.test
1

##

llvm/test/tools/llvm-readobj/thin-archive.test
86

sections: []
symbols: []

grimar marked 3 inline comments as done.Dec 12 2019, 12:22 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/COFF/symbols.test
2

Sorry!

llvm/test/tools/llvm-readobj/MachO/sections-ext.test
14

It comes from the original test. I'd do any functional changes/improvements separatelly.

This revision was automatically updated to reflect the committed changes.
grimar marked 4 inline comments as done.
llvm/test/tools/llvm-readobj/hex-dump.test