This is an archive of the discontinued LLVM Phabricator instance.

[ASTImporter] Refactor unittests to be able to parameterize them in a more flexible way
ClosedPublic

Authored by martong on Jan 28 2019, 5:11 AM.

Details

Summary

Currently TestImportBase is derived from ParameterizedTestsFixture
which explicitly states that the gtest parameter can be only an
ArgVector. This is a limitation when we want to create tests which may
have different parameters.
E.g. we would like to create tests where we can combine different test
parameters. So, for example we'd like gtest to be able to provide
parameters of <std::tuple<ArgVector, const char *> instead of a simple
ArgVector.

Diff Detail

Repository
rC Clang

Event Timeline

martong created this revision.Jan 28 2019, 5:11 AM
balazske added inline comments.
unittests/AST/ASTImporterTest.cpp
15

Define of this is related to the new visibility tests, but not a problem is this remains here (there is another not strictly related thing: The new Import template).

shafik added inline comments.Jan 29 2019, 11:46 AM
unittests/AST/ASTImporterTest.cpp
464

Is this being used in this PR?

martong updated this revision to Diff 184694.Feb 1 2019, 1:55 AM
martong marked 2 inline comments as done.
  • Remove unrelated hunks
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 1:55 AM

Update: I just removed the unrelated changes.

shafik accepted this revision.Feb 6 2019, 4:37 PM

This looks reasonable to me but I don't have strong opinions on refactoring gtest tests.

This revision is now accepted and ready to land.Feb 6 2019, 4:37 PM
aaron.ballman accepted this revision.Feb 6 2019, 4:41 PM
aaron.ballman added a subscriber: aaron.ballman.

LGTM, though I am by no means a gtest expert.

a_sidorin accepted this revision.Feb 6 2019, 10:06 PM
This revision was automatically updated to reflect the committed changes.