[PATCH] D65633: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 3]

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:482-486
+  // 4 additional sections, index 0, .symtab, .strtab and .shstrtab are added
+  // by yaml2obj.
+  EXPECT_EQ(
+      std::distance(MutableObject.section_begin(), MutableObject.section_end()),
+      6);
----------------
abrachet wrote:
> rupprecht wrote:
> > I think writing a test matcher would make this more self-documenting than a comment.
> > e.g.
> > `EXPECT_THAT(MutableObject, SectionsAre({".remove_me", ..., }));`
> > which would check both std::distance and each section name by iterating over all sections.
> > 
> > Writing a test helper (instead of a full blown matcher) would also work; you might not need a thorough matcher outside of this test.
> I created this `MatchesRange` template function. It's not as cool as what you were suggesting but I couldn't figure out how to make a gtest matcher.
You could consider something like this:
```
std::vector<StringRef> Sections;
transform(MutableObject.sections(), [](??? S) { return S->getName(); }, std::back_inserter(Sections));
EXPECT_THAT(Sections, testing::ElementsAre(".foo", ".bar", ...));
```
The reason I would prefer that over your current solution is because this will give you a mostly reasonable error message if the check fails, whereas here you'll just get "false is not true"(tm). (You could even make a utility function to hide the `transform` stuff and make the check a one-liner.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65633/new/

https://reviews.llvm.org/D65633