[PATCH] Path: Recognize COFF import library file magic.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Thu Nov 14 05:39:43 PST 2013


> As to llvm-readobj, I think we don't have to write a test to run
> llvm-readobj with an actual file reside on the file system. My test should
> be sufficient as a unit test for the function that's being modified in the
> patch. Actually I think this unit test would be better than tests using
> llvm-readobj because it's more focused on identity_magic's functionality.
> It's easy to write, understand, and debug.

But fails to show why we need a feature and cannot be compared with
native tools like dumpbin. At llvm we normally prefer smallish
integration tests than gtest style unit tests.

The case where unit tests are clearly better is when there is no easy
way to isolate what we want to test. For example, fixing a bug in
densemap. Anything but a unit test would be brittle in that case.

For file type identification, all that the unit test shows is that you
wrote the same number in two files. A test using llvm-readobj lets us
check the file with a native tool. Given that llvm-readobject must
identify the file, it is not brittle as the DenseMap example would be.

So, please, add a test running llvm-readobject.

Cheers,
Rafael



More information about the llvm-commits mailing list