[PATCH] D98303: [flang] Update output format test to use GTest

Andrzej Warzynski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 28 12:52:45 PDT 2021


awarzynski accepted this revision.
awarzynski added a comment.

LGTM, thank you for working on this! I've left a couple of comments, but these are [nit]s (feel free to ignore!).



================
Comment at: flang/unittests/RuntimeGTest/NumericalFormatTest.cpp:98
+      staticDescriptorMaxRank, extent, CFI_attribute_pointer);
+  whole.Dump();
+  whole.Check();
----------------
awarzynski wrote:
> IMHO, it would be better to dump to `stderr` so that it is possible to separate the regular output from GTest and other logging like this.
To clarify, I meant `whole.Dump(stderr)` instead of `whole.Dump()`.


================
Comment at: flang/unittests/RuntimeGTest/NumericalFormatTest.cpp:41-44
+  // This return is ambiguous - did the format fail to be performed, or do the
+  // strings not match?
+  if (status)
+    return false;
----------------
ashermancinelli wrote:
> I would be interested in any suggestions for how to better handle this case.
Perhaps `ASSERT_EQ(status, 0)` would be better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98303



More information about the llvm-commits mailing list