[PATCH] D114684: Add ScopedPrinter unit tests

[Jayson Yan via Phabricator via llvm-commits]

Jaysonyan marked 5 inline comments as done.
Jaysonyan added inline comments.


================
Comment at: llvm/unittests/Support/ScopedPrinterTest.cpp:86
+TEST_F(ScopedPrinterTest, PrintIndent) {
+  auto ResetIndentFunc = [](raw_string_ostream &OS, ScopedPrinter &W) {
+    W.printIndent();
----------------
jhenderson wrote:
> Variable names in this test seem wrong.
> 
> I wonder if you should change all the expected strings to be called `ExpectedOut`? And similarly perhaps the functions could have a common name (I don't have any concrete suggestions yet)?
My mistake, I've updated all expect strings to be `ExpectedOut` (and will update json versions to be `JSONExpectedOut`) and the function names to be all called `PrintFunc` which also matches the alias for the `function_ref` inside the test fixture.


================
Comment at: llvm/unittests/Support/ScopedPrinterTest.cpp:392
+      const std::vector<std::string> StringList = {"foo", "bar", "baz"};
+      const bool BoolList[] = {true, false};
+      const std::vector<uint64_t> Unsigned64List = {
----------------
jhenderson wrote:
> //Mutters about silly `std::vector<bool>` standard specialization...//
Haha I learned about this quirk while working on this change. 


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

https://reviews.llvm.org/D114684