Page MenuHomePhabricator

[flang] Change existing runtime tests to use gtest
AbandonedPublic

Authored by ashermancinelli on Feb 24 2021, 7:01 AM.

Details

Summary

Updates the flang runtime unittests to use GTest. This patch should
likely not be accepted as-is, as there are temporarily two directories
for runtime tests such that new tests may be
written to use GTest without interfering with the old ones. Eventually,
old runtime tests directory should be removed.

The style guide indicates that exceptions should never be used,
however almost all of the the current runtime tests use exceptions.
This revision removes this need for exceptions.

I attempted to follow the style guide as closely as possible, however
some of the files flang/unittests/Runtime differ in style from other
unittest directories. In these cases, I leaned towards conforming with
the other existing unittests and not the precedent set by
flang/unittests/Runtime directory. For example, for file names I used
pascal case instead of kabob case, and test cases are in pascal case
instead of camel case.

Some tests require input and will likely not be able to remain in the
same directory as the other runtime tests since LLVM's CMake unittest
functions do not allow multiple targets in a directory (as discussed
on slack). Example is

This is my first revision, so please point out anything that doesn't
conform to best practices.

Diff Detail

Event Timeline

ashermancinelli requested review of this revision.Feb 24 2021, 7:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2021, 7:01 AM
ashermancinelli added a comment.EditedFeb 24 2021, 7:17 AM

@awarzynski Looks like I mistakenly abandoned the last revision for this. Should I keep this one open or should we stick to the abandoned one since you've already added other reviewers to it?

ashermancinelli abandoned this revision.Feb 24 2021, 7:34 AM

@awarzynski Looks like I mistakenly abandoned the last revision for this. Should I keep this one open or should we stick to the abandoned one since you've already added other reviewers to it?

Discussion has continued on the old revision (D97349) and I will prepare a new patch for this in smaller chunks. Closing this thread.