approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
318 stars 51 forks source link

Make library usable with Cmake add_subdirectory() of source tree. #4

Closed dimztimz closed 5 years ago

dimztimz commented 5 years ago

Downstream users would do the following:

  1. add_subdirectory(ApprovalTests.cpp/ApprovalTests)
  2. target_link_libraries(my_project ApprovalTests)

And then in sources they would include as

  1. #include <ApprovalTests/Approvals.h>

This is a common pattern. The tests are modified to use this pattern.

isidore commented 5 years ago

Not sure what exactly, but it broke the build :-(
Usually we pair on these kinds of things to make it easier. would you like to setup a pairing session?

dimztimz commented 5 years ago

I think i fixed and force pushed. AprovalTests needed to be defined as interface library.

claremacrae commented 5 years ago

Hi @dimztimz Thank you for making this change. I've been aware for some time that our cmake files could do with improving, to simplify the #include lines in the tests.

I'm currently building the changes on my machine in a few environments, so I can understand them.

dimztimz commented 5 years ago

This is a header only library, it should be added in cmake as interface library because there is nothing to be build. See https://cmake.org/cmake/help/v3.0/manual/cmake-buildsystem.7.html#interface-libraries

This is the proper way. As for visual studio, i don't know how to solve that. This pops up in a web search https://developercommunity.visualstudio.com/content/problem/232936/cmake-interface-library-project-shows-nothing-in-c.html

claremacrae commented 5 years ago

Note to self: This answer looks worth trying out for Visual Studio to support convenient development with interface libraries. https://stackoverflow.com/a/39889958/104370

claremacrae commented 5 years ago

Just to note that the script we use to create single-header releases still works fine.

claremacrae commented 5 years ago

OK, I can fix the Visual Studio builds convenience-thing as a separate step, and I definitely agree that this makes the library easier for other projects to use directly from source...

For me, all that remains is to understand the changes to <> I mentioned in my earlier comment, and then I think this is good to be merged.

dimztimz commented 5 years ago

They are mostly for example for how downstream users should include this library. The change was not necessary. The general convention is the following:

The tests are, in a way, different than the actual library.

For the exact rules for GCC see this https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html . Visual Studio most likely follows the same rules.

claremacrae commented 5 years ago

Thanks very much for doing this!