Also remove sections similarly for --strip-all, --discard-all, --strip-unneeded.
Details
Diff Detail
Event Timeline
test/tools/llvm-objcopy/COFF/strip-debug.test | ||
---|---|---|
1 | Are you planning on using this yaml file for other tests? Otherwise, it seems confusingly named. | |
3 | Nit, you're being inconsistent with using single and double-dashes for your options in this file. I prefer double, to avoid theoretical issues with grouping of single-letter options. | |
7 | What is SECTIONS-POST for, here and below? I can't see any check patterns for it. |
test/tools/llvm-objcopy/COFF/strip-debug.test | ||
---|---|---|
1 | I meant to use it for multiple tests, but it was enough with one, as all these switches turned out to do the same for sections. Will fix. |
test/tools/llvm-objcopy/COFF/strip-debug.yaml | ||
---|---|---|
1 ↗ | (On Diff #182362) | Does this test run? I thought it had to end in ".test" to run. |
test/tools/llvm-objcopy/COFF/strip-debug.yaml | ||
---|---|---|
1 ↗ | (On Diff #182362) | Oh crap, you're right. I'm used to working on tests in e.g. lld, where .yaml already is added to that list of suffixes, and where the convention is that if the file actually contains yaml input data, it's named .yaml, otherwise .test. I've already committed one test with a .yaml suffix though. Do you want me to add a lit.local.cfg that adds yaml to the list (kind of like https://github.com/llvm/llvm-project/blob/master/llvm/test/tools/yaml2obj/lit.local.cfg), or should I rename the existing test to .test? (I didn't notice since I've been running the tests with llvm-lit path/to/llvm-objcopy/COFF/*.) |
test/tools/llvm-objcopy/COFF/strip-debug.yaml | ||
---|---|---|
1 ↗ | (On Diff #182362) | I presume renaming to .test is easiest, and matches the ELF backend's tests, so I'll rename the existing test in one NFC commit and rename the newly added tests before I commit them as well. |
Accepted assuming the test case passes when renamed to .test :)
test/tools/llvm-objcopy/COFF/strip-debug.yaml | ||
---|---|---|
1 ↗ | (On Diff #182362) | SGTM -- I think just renaming these to .test is better. I wouldn't strongly object to allowing .yaml as a test case extension if others are used to it, but it seems a little weird to me... |
Are you planning on using this yaml file for other tests? Otherwise, it seems confusingly named.