This is an archive of the discontinued LLVM Phabricator instance.

[flang] Run non-gtest unit tests with lit.
ClosedPublic

Authored by DavidTruby on Jul 16 2020, 6:20 AM.

Details

Summary

As a corrollary, these tests are now run as part of the check-flang
target.

Diff Detail

Event Timeline

DavidTruby created this revision.Jul 16 2020, 6:20 AM
Herald added a project: Restricted Project. · View Herald Transcript
CarolineConcatto added a comment.EditedJul 20 2020, 6:59 AM

Thank you David for these patch.
Do you mind to explain why you had to create this new non-gtest unit in the patch.

flang/test/NonGtestUnit/lit.cfg.py
14

why here I don't need to tweak the path like it is done in flang/test/Unit/lit.cfg.py?

flang/unittests/Decimal/CMakeLists.txt
12

Should this be removed?

flang/unittests/Evaluate/CMakeLists.txt
7

Should this be removed?

26

where are the tests Leadz and PopPar? Are they running?

flang/unittests/Runtime/CMakeLists.txt
33

Why this one was not removed like the others?

DavidTruby marked 6 inline comments as done.Jul 20 2020, 7:39 AM

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.

flang/test/NonGtestUnit/lit.cfg.py
14

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.

flang/unittests/Decimal/CMakeLists.txt
12

This test is currently not run even with ctest. I've left it out of the port here for that reason.

15

This should be removed though!

flang/unittests/Evaluate/CMakeLists.txt
7

This library is common to the upcoming tests so it needs to stay here.

26

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.

flang/unittests/Runtime/CMakeLists.txt
33

Again, this test is not currently run under the old ctest framework so I've left it alone.

sscalpone accepted this revision.Jul 20 2020, 9:51 AM
This revision is now accepted and ready to land.Jul 20 2020, 9:51 AM

@sscalpone do you have any input as to whether the external-hello-world and thorough-test binaries should be run as unit tests? I'm not sure what to do with those binaries as currently they are built but never automatically tested.

external-hello-world

The external-hello-world requires input. Does lit handle that well?

thorough-test

The thorough-test runs for a long time. I say keep it, make sure it links, but don't run it as part of unit tests.

sameeranjoshi accepted this revision.Jul 22 2020, 4:17 AM

Added comments on tests that aren't run by default.

CarolineConcatto accepted this revision.Jul 24 2020, 6:45 AM
This revision was automatically updated to reflect the committed changes.