[PATCH] D99614: [flang] Update list-input test to use GTest

Andrzej Warzynski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 12 03:37:48 PDT 2021


awarzynski added a comment.

Thank you for addressing my comments @ashermancinelli, this looks really good!  One general question, why make everything `static`? Is there any technical benefit? I feel that it reduces readability a bit. If it's not needed, I suggest removing it.



================
Comment at: flang/unittests/RuntimeGTest/CMakeLists.txt:9
   RuntimeCrashTest.cpp
+  CrashHandlerFixture.cpp
+  ListInputTest.cpp
----------------
Repetition


================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:65
+    ASSERT_EQ(std::strcmp(outputBuffers[j], expectedOutput), 0)
+        << "wanted outputBuffers[" << j << "]=" << expectedOutput << ", got '"
+        << outputBuffers[j] << "'\n";
----------------
Probably `expectedOutput`?


================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:92
+  static std::int64_t have[listInputLength]{-1, -2, -3, -4, 5, -6, 7, -8, 9};
+  static const std::int64_t want[listInputLength]{1, 2, 3, 3, 5, 6, 7, 8, 9};
+  for (j = 0; j < listInputLength; ++j) {
----------------
[nit] Perhaps `expectedOutput`? `want` is a bit problematic for me (you may `want` something, but does imply that you _can_ `expect` it?). I'm probably overthinking this!


================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:103
+  for (j = 0; j < listInputLength; ++j) {
+    ASSERT_EQ(have[j], want[j]) << "wanted have[" << j << "]==" << want[j]
+                                << ", got " << have[j] << '\n';
----------------
`wanted have`?
```
ASSERT_EQ(have[j], want[j]) << "want[" << j << "]==" << want[j]
`wanted have`?                                << ", have " << have[j] << '\n';

```
? (so that what's printed can easily be mapped to what's in the code). Similar comment for other tests.


================
Comment at: flang/unittests/RuntimeGTest/ListInputTest.cpp:191
+        std::make_tuple("3*2", std::vector<int>{2, 2, 2})), );
\ No newline at end of file

----------------
FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99614



More information about the llvm-commits mailing list