[PATCH] D83946: [flang] Run non-gtest unit tests with lit.

[David Truby via Phabricator via llvm-commits]

DavidTruby marked 6 inline comments as done.
DavidTruby added a comment.

>   Do you mind to explain why you had to create this new non-gtest unit in the patch.

Assuming you mean the new folder; lit requires a separate folder in /test for each test configuration you want to use. Since I'm adding a new configuration here to simply run a binary and report the error code, we need a separate folder for that than the Unit folder.



================
Comment at: flang/test/NonGtestUnit/lit.cfg.py:13
+config.test_format = lit.formats.ExecutableTest()
+
+path = os.path.pathsep.join((config.flang_libs_dir, config.llvm_libs_dir,
----------------
CarolineConcatto wrote:
> why here I don't need to tweak the path like it is done in flang/test/Unit/lit.cfg.py?
I believe in the case of the gtest tests, gtest needs to know where those tests are so they are added to PATH. We don't need that here as we just run the test binaries directly.


================
Comment at: flang/unittests/Decimal/CMakeLists.txt:11
 
 target_link_libraries(thorough-test
   FortranDecimal
----------------
CarolineConcatto wrote:
> Should this be removed?
This test is currently not run even with ctest. I've left it out of the port here for that reason.



================
Comment at: flang/unittests/Decimal/CMakeLists.txt:16
 
 add_test(NAME Sanity COMMAND quick-sanity-test)
----------------
This should be removed though!


================
Comment at: flang/unittests/Evaluate/CMakeLists.txt:7
 
 target_link_libraries(FortranEvaluateTesting
   LLVMSupport
----------------
CarolineConcatto wrote:
> Should this be removed?
This library is common to the upcoming tests so it needs to stay here.


================
Comment at: flang/unittests/Evaluate/CMakeLists.txt:26
 
-# These routines live in lib/Common but we test them here.
-add_test(UINT128 uint128-test)
-add_test(Leadz leading-zero-bit-count-test)
-add_test(PopPar bit-population-count-test)
-
-add_executable(expression-test
-  expression.cpp
-)
-
-target_link_libraries(expression-test
+add_flang_nongtest_unittest(expression
   FortranCommon
----------------
CarolineConcatto wrote:
> where are the tests Leadz and PopPar? Are they running?
They're added above (lines 11 and 16) and are just called leading-zero-bit-count.test and bit-population-count.test under the new testing framework.


================
Comment at: flang/unittests/Runtime/CMakeLists.txt:32
 
 target_link_libraries(external-hello-world
   FortranRuntime
----------------
CarolineConcatto wrote:
> Why this one was not removed like the others?
Again, this test is not currently run under the old ctest framework so I've left it alone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83946