Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create wrapper ApprovalTests.hpp for projects that use this via add_subdirectory (or similar) to use #89

Closed
claremacrae opened this issue Jan 16, 2020 · 3 comments
Labels
enhancement infrastructure CMake, Builds, CI, and similar

Comments

@claremacrae
Copy link
Collaborator

For the background to this, see
approvals/ApprovalTests.cpp.StarterProject#9

It turns out that this will be really useful when documenting the CMake Integrations - see #69.

@claremacrae claremacrae added enhancement infrastructure CMake, Builds, CI, and similar labels Jan 16, 2020
@claremacrae claremacrae added this to the vNext release milestone Jan 16, 2020
claremacrae added a commit that referenced this issue Jan 16, 2020
See #89.

For now, this file is a manual copy of tests/GoogleTest_Tests/CompileAllIncludes.cpp.
Adding include guards and providing a mechanism to keep it update will be
done in later steps.
claremacrae added a commit that referenced this issue Jan 16, 2020
ApprovalTests.hpp which has the same contents, and will eventually
be updated by script.
See #89.
claremacrae added a commit that referenced this issue Jan 16, 2020
These were provided so that the documentation code snippets look correct for
users of the released single hpp.
We don't need them now, as we have a ApprovalTests.hpp in source.
See #89.
@claremacrae
Copy link
Collaborator Author

Unfortunately adding a single header for ApprovalTests has broken the Xcode CI builds that use unity.

https://github.com/approvals/ApprovalTests.cpp/runs/393170720
https://github.com/approvals/ApprovalTests.cpp/runs/393170749

Commenting out this one test fixes the build:

// begin-snippet: YouCanVerifyCombinationsOf2
TEST_CASE("YouCanVerifyCombinationsOf2")
{
    std::vector<std::string> v{"hello", "world"};
    std::vector<int> numbers{1, 2, 3};
    CombinationApprovals::verifyAllCombinations(
        [](std::string s, int i) { return std::make_pair(s, i); }, v, numbers);
}
// end-snippet

This is probably related to catchorg/Catch2#1405 "Fixes disagreement between operator<< available for SFINAE and actually usable"

claremacrae added a commit that referenced this issue Jan 16, 2020
See comments in #89 for the workaround to this.
For now, I've excluded CombinationTests.cpp from the unity builds,
so it will be compiled on its own.

I have confirmed the same problem occurs in XCode with unity builds
and the single header - so this will need investigation at some point.
@claremacrae
Copy link
Collaborator Author

The fix in a877b87 didn't entirely work. The code built, but tests in CombinationTests.cpp failed to find the source code location when in Ninja Unity builds - the file was compiled on its own, so must not have had whatever magic Ninja and Unity uses to get the path right when multiple files are compiled.

The next change - 8a94fa8 - built fine and the tests passed.

@claremacrae
Copy link
Collaborator Author

I've logged the need for a workaround to the build failure in #90 - so am closing this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement infrastructure CMake, Builds, CI, and similar
Projects
None yet
Development

No branches or pull requests

1 participant