Page MenuHomePhabricator

[unittests] Add a helper function for getting an input file
ClosedPublic

Authored by labath on Jun 27 2017, 5:56 AM.

Details

Summary

Fetching an input file required about five lines of code, and this was
repeated in multiple unit tests, with slight variations. Add a helper
function for doing that into the lldbUtilityMocks module (which I rename
to lldbUtilityHelpers to commemorate the fact it includes more than
mocks)

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Jun 27 2017, 5:56 AM
eugene accepted this revision.Jun 27 2017, 1:52 PM

Great change. The only comment I have is about location of TestUtilities.cpp.
Utility\Helpers kinda implies that these are helpers for Utility tests which is not true.

I would move it up one level up lldb/unittests/TestUtilities.cpp

This revision is now accepted and ready to land.Jun 27 2017, 1:52 PM
zturner added inline comments.Jun 27 2017, 2:40 PM
unittests/Utility/Helpers/TestUtilities.cpp
17 ↗(On Diff #104145)

this should be const Twine &name. Also can you call this GetInputFilePath()? At first I was assuming it would return the contents of the file.

labath marked an inline comment as done.Jun 28 2017, 2:29 AM

The only comment I have is about location of TestUtilities.cpp.
Utility\Helpers kinda implies that these are helpers for Utility tests which is not true.

I would move it up one level up lldb/unittests/TestUtilities.cpp

Hm.. that makes sort of sense for this function.

However, right now I can't think of any other utility function that would go here. OTOH, I have a couple of ideas about the use of the XXX/Helpers/YYY.h pattern. For example, in Host/Helpers we could have a utility function which creates two connected sockets (already needed for a couple tests), and Core/Helpers could contain mocked versions of various Process/Target/... classes (as no higher level functionality can be tested without one of those around).

That's why I'd like to avoid creating a module if it's going to contain just this function. And with that in mind putting in in Utility/Helpers makes sense as that's the place everyone can pull it from without grabbing additional dependencies.

This revision was automatically updated to reflect the committed changes.