[PATCH] D97349: [flang] Change existing runtime tests to use gtest

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added a comment.

Hi @ashermancinelli ,

Many thanks for working on this! This is a great starting point, but I agree that it is too large to be merged as is. I suggest splitting this into smaller parts. I think that introducing a dedicated directory for GTest port is a fair compromise during the transition period.

A few thoughts:

- `CharacterTest.cpp` looks fairly small and could be a good starting point. Similarly for `ListInputTest.cpp`.
- Currently `CharacterTest.cpp` is not very GTest-idiomatic.  I see 3 methods being tested there (`CharacterAppend1`, `CharacterPad1`, `CharacterCompareScalar1`). I think that introducing a dedicated test case for each would make a lot of sense.
- In general I think that having a lot of small test cases would be cleaner than a handful of bigger cases that cover multiple APIs.
- Do all tests need `StartTests();` and `EndTests()` (thinking about `CharacterTest.cpp` again)? Also, shouldn't that be included in some test fixture <https://github.com/google/googletest/blob/master/docs/primer.md#test-fixtures-using-the-same-data-configuration-for-multiple-tests-same-data-multiple-tests>.

Otherwise this makes a lot of sense to me. I've also added a few other reviewers .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97349