@claremacrae I'll take a look. First, though, I just found to my surprise that using the new release of [Boost].µt (v1.1.6) with ApprovalTests commit f1caa93 works with the Ninja generator. My tests build and pass using my current toolchain, listed below.

• macOS 10.15 Catalina
• LLVM Clang 9.0.1 (MacPorts)
• CMake 3.16.2
• Ninja 1.9.0
• C++20

from approvaltests.cpp.

@claremacrae It is an in-source build.

from approvaltests.cpp.

@claremacrae I have a hunch that enabling CMAKE_UNITY_BUILD is what is causing my project to work. Could you try -DCMAKE_UNITY_BUILD=yes for your project?

from approvaltests.cpp.

Thanks for reporting this. It's true that __FILE__ not working with Ninja has cost this project a lot of time recently, and I'm keen to spare our users this cost as much as possible - the docs you link to were an attempt to do that.

Could you clarify what you improvement you are suggesting, please? Which documentation file(s) should be improvement, and with roughly what sort of content?

We tried to give feedback to users at compile-time or run-time on this issue, which obviously didn't work for you... Did you see this - I'm guessing not. Do we need to change the code to to point users to this troubleshooting page too?

from approvaltests.cpp.

@claremacrae, I'll do my best to organize my answers here.

First, I did not notice any compile-time feedback and while I did receive runtime feedback it wasn't exactly clear and required going through a couple of pages in the documentation to find the corresponding error message. I think a URL in the error message would be extremely helpful and perhaps going so far as suggesting to switch the generator if using Ninja as a workaround.

Second, because this issue does not effect all of the Test Frameworks (presumably Boost ut is the only one unaffected), the individual Using${TEST_FRAMEWORK_NAME}.md files should carry a simple note near the beginning that the Ninja generator will cause runtime errors.

The UsingCatch2.md could incorporate this note like so:

Using Approval Tests With Catch

Contents

• Getting Started With Catch 1 and 2
  • Starter Project
  • New Project
  • Existing Project - with CATCH_CONFIG_MAIN
  • Existing Project - with your main()

Getting Started With Catch 1 and 2

The Catch2 test framework works well with Approval Tests.

This section describes the various ways of using Approval Tests with Catch 2.

Note: ApprovalTests integration with Catch2 does not currently support the Ninja build generator and the approval tests will fail at runtime.

These steps also work with the earlier version, Catch 1, which is on the Catch 1.x branch, and is still provided for those on pre-C++11 compilers. (Please note that the Approval Tests library requires C++11 or newer, however).

Approval Tests requires that a file called catch.hpp is found.

(Before v7.0.0, it required Catch.hpp)

Starter Project

The quickest way to start experimenting with Approval Tests is to:

1. Download the project ApprovalTests.cpp.StarterProject - via the green "Clone or Download" button at the top-right of the project site.
2. Opening the project in the C++ IDE of your choice.

Each time we release a new version of Approval Tests, we update this project, so it always has the latest features.

New Project

Create a file main.cpp and add just the following two lines:

// main.cpp:
#define APPROVALS_CATCH // This tells Approval Tests to provide a main() - only do this in one cpp file
#include "ApprovalTests.hpp"

^{snippet source | anchor}

Existing Project - with CATCH_CONFIG_MAIN

If you have a Catch (1 or 2) project with your own main.cpp that contains the following lines, you will need to replace them with the code in the previous section.

#define CATCH_CONFIG_MAIN // remove these lines, and replace with Approval Tests lines
#include "catch.hpp"

Existing Project - with your main()

If you have supplied your own main() for Catch, you will need to teach it how to supply test names to Approval Tests.

You should make the following additions to your own source file that contains main().

// Add these two lines to the top of your main.cpp file:
#define APPROVALS_CATCH_EXISTING_MAIN
#include "ApprovalTests.hpp"

^{snippet source | anchor}

Back to User Guide

Lastly, I think the specific troubleshooting page should indicate that The Problem is not just tied to MSVC's use of the Ninja generator but any use of the Ninja generator. Of course, it should be noted that Boost ut is exempt from this issue. It's also a bit difficult to have to click through so many links, perhaps the specific troubleshooting page for this issue should really just be inside the main troubleshooting page. I like to do to text searches within a document to find my specific issue... and this didn't work very well in this case.

from approvaltests.cpp.

@jwillikers Thanks very much for the suggestions - really helpful!

from approvaltests.cpp.

I've been able to reproduce this on a Mac in CLion, by creating a configuration that passes -G Ninja to make....

I do get the expected error message at run-time - this is the output for the Catch2_Tests:

../tests/Catch2_Tests/writers/StringWriterTests.cpp:49: FAILED:
due to unexpected exception with message:
  

  *****************************************************************************
  *******
  *
  *
  * Welcome to Approval Tests.
  *
  * There seems to be a problem with your build configuration.
  * We cannot find the test source file at:
  *   ../tests/Catch2_Tests/writers/StringWriterTests.cpp
  *
  * For details on how to fix this, please visit:
  * https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/
  TroubleshootingMisconfiguredBuild.md
  *
  *
  *****************************************************************************
  *******

Really sadly, the URL is wrapped - so it looks like it's linking to this:

https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/

When it is actually wanting to think to this exact page:

https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/TroubleshootingMisconfiguredBuild.md

The DocTest_Tests output is not wrapped, so the URL is correct.

GoogleTest_Tests output also not wrapped.

UT_Tests tests do pass.

Correction: I cannot get the UT_Tests to pass when Ninja is the generator. This is using HomeBrew gcc:

-- The C compiler identification is GNU 9.2.0
-- The CXX compiler identification is GNU 9.2.0

At least it does not wrap the URL in the help message.

from approvaltests.cpp.

I can make all the tests pass by adding the full source path the CMake command line argument in the CLion CMake settings' "CMake options:" box:

-S /Users/clare/Documents/develop/ApprovalTests/ApprovalTests.cpp -G Ninja

It's not a very pretty workaround, but it's probably worth noting...

I can't reproduce this fix now - I wonder whether CLion switched back to running another configuration after I edited the settings...

from approvaltests.cpp.

Another workaround in the CLion CMake settings' "Generation path:" box is to use a directory outside of the source location, e.g. ../cmake-out-of-source-ninja

This will clutter up the parent directory, and would quickly cause conflicts with other repos in the same directory - but it does confirm what we saw on Windows, which is that if you tell Ninja to do out-of-source builds, it switches to absolute paths.

from approvaltests.cpp.

This may be useful for documenting the Visual Studio workaround:
https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=vs-2019

from approvaltests.cpp.

This is an example of a Visual Studio configuration file CMakeSettings.json file that uses Ninja, and does find the source paths correctly...

https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=vs-2019    {
      "name": "x64-Clang-Release",
      "generator": "Ninja",
      "configurationType": "RelWithDebInfo",
      "buildRoot": "${projectDir}\\..\\builds\\ApprovalTests.cpp\\${workspaceHash}\\${name}",
      "installRoot": "${projectDir}\\install\\${name}",
      "cmakeCommandArgs": "",
      "buildCommandArgs": "-v",
      "ctestCommandArgs": "",
      "inheritEnvironments": [ "clang_cl_x64" ],
      "variables": []
    }

It works because the "buildRoot" is set to a location outside of projectDir:

"${projectDir}\\..\\builds\\ApprovalTests.cpp\\${workspaceHash}\\${name}",

The unfortunate thing about it is that for the output folder structure to be meaningful, I manually added ApprovalTests.cpp to the folder path - and that's error-prone...

I wanted to see if there was a way to get the folder name automatically, and there doesn't seem to be a way: https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=vs-2019

There is an alternative of using ${workspaceHash} - which guarantees unique folder names, but gives names like:

4a85990e-08e9-43ee-bf2d-02e612f44969

from approvaltests.cpp.

Okay, I think I am actually starting to understand what is going on now 😵 So depending on a number of factors including build-generator, IDE, and compiler, the __FILE__ macro may expand to relative paths, likely due to the __FILE__ macro having the common behavior of expanding to the source file name as given in the call to the compiler or in the cases of MSVC, only populating __FILE__ with the file name unless the \FC option is given. 😪 Am I on the right track now?

from approvaltests.cpp.

Yes, you're on the right track!

I think I'd summarise it as follows:

• Some compilers only put a relative path in to __FILE__, if the filename they are given on the command line was relative
• Visual C++ has a way to over-ride this and force absolute paths, if given /FC
• On all platforms, the Ninja program:
  • Gives the compiler relative paths, if the build tree is inside the source tree
  • Gives the compiler absolute paths if build tree is outside the source tree
• Visual Studio recently changed its default generator to Ninja, making the problem much more common.

I believe that the reason why your lovely CMake check passed, with Ninja as a generator, is that your command invoked the compiler with an absolute path to the source file. Or, to put it another way, I don't think the compiler was invoked by Ninja, so the problem was not observed...

from approvaltests.cpp.

@claremacrae Thank you! I think we are on the same page now.

from approvaltests.cpp.

@jwillikers You mention a couple of times here that Boost.UT works for you with Ninja, and I can't reproduce that.

For these builds, is the build-tree outside the source-tree? (in which case, it's expected to pass because Ninja then uses full paths)

If the build is inside the source, then there's a mystery - and something else going on that I don't understand, and would like to document!

from approvaltests.cpp.

Things I plan to do:

doc/mdsource/TroubleshootingMisconfiguredBuild.source.md

• Update docs to refer to Ninja relative paths being a problem on all platforms, not just MSVC
• Emphasize using Ninja buildspace outside of source tree
• Embed compile-time error text - to show the error message.
• Copy in text from this comment
• Enable out-of-source Ninja CI builds to confirm the fix
• Note that we have added /FC to our CMake, so behaviour will be correct for any use of this project via add_subdirectory() or similar.
• Eliminate white-space around the embedded error text files

doc/mdsource/Using*.source.md

• Note in each of the test frameworks that they do not play well with Ninja unless build is outside of source tree

doc/mdsource/Troubleshooting.source.md

• Embed test error text - to show the error message and help those searching.
• Embed compile-time error text - to show the error message and help those searching.
• Mention Ninja generator in body

General stuff:

• Documentation
  • A link on Features
  • A link on build/relnotes_X.X.X.md

from approvaltests.cpp.

@claremacrae I believe I did get Ninja / ApprovalTests / Boost.UT working, but now it is definitely not working. It might have been a change in upstream Boost.UT or perhaps I had CLion using Makefiles when I thought it was using Ninja.

from approvaltests.cpp.

@jwillikers Super - that's really helpful, thanks!

from approvaltests.cpp.

@jwillikers, Hi Jordan. Here's a link to the changes I've made for this...

6de41c2...8044ba6

If you're tempted to look at it, I would suggest only looking at the changed files in doc/*.md...

The one thing I didn't get to the bottom of was this:

while I did receive runtime feedback it wasn't exactly clear and required going through a couple of pages in the documentation to find the corresponding error message. I think a URL in the error message would be extremely helpful and perhaps going so far as suggesting to switch the generator if using Ninja as a workaround.

The initial run-time help message that shows when the Ninja in-source-build problem happens has always pointed to this, which was the correct page, even if its content needed improving:

https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/TroubleshootingMisconfiguredBuild.md#top

(There is a similar help message for if the main() is misconfigured, and that does point to an unhelpful page... I may log an issue for that separately)

from approvaltests.cpp.

@jwillikers Thanks - was it an in-source-tree build or out-of-source tree?

from approvaltests.cpp.

@jwillikers Wow, that’s brilliant. I’ll have a go at reproducing your setup tomorrow. I haven’t used MacPorts before, so it will be interesting to try it out.

from approvaltests.cpp.

@jwillikers

... I just found to my surprise that using the new release of [Boost].µt (v1.1.6) with ApprovalTests commit f1caa93 works with the Ninja generator. My tests build and pass using my current toolchain, listed below.

• macOS 10.15 Catalina
• LLVM Clang 9.0.1 (MacPorts)
• CMake 3.16.2
• Ninja 1.9.0
• C++20

I've tried hard to reproduce this, and must be missing something.

Mostly I get:

* We cannot find the test source file at:
*   unknown

So I don't even get the incorrect relative source code location.

I also had to add this to CMake to even get the Boost v1.1.6 header to compile with /opt/local/bin/clang++-mp-9.0

if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
    target_compile_options(ut INTERFACE
            -Wno-c99-extensions # Needed for Boost.ut, at least in v1.1.6
            )
endif()

This is all in the main ApprovalTests.cpp build - to insulate myself from all the CMake changes that Boost.UT makes...

Versions in the above output:

ApprovalTests.cpp: f1caa93
Boost.ut: v1.1.6
Ninja: 1.9.0

Info from my CMake run that gave the above output:

CMake Version = 3.16.2
CMake Generator = Ninja
CMAKE_CXX_COMPILER_ID = Clang
CMAKE_CXX_COMPILER_VERSION = 9.0.1
CMAKE_CXX_COMPILER = /opt/local/bin/clang++-mp-9.0

from approvaltests.cpp.

With:

• ApprovalTests.cpp 1ed7551 (the current version)
• Which contains Boost.ut 8004290d195b08eda531eb015ddcda2e11d41ce9

And with this patch in third_party/ut/CMakeLists.txt

if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
    target_compile_options(ut INTERFACE
            -Wno-c99-extensions # Needed for Boost.ut, at least in v1.1.6
            )
endif()

And these CMake choices:

CMake Version = 3.16.2
CMake Generator = Ninja
CMAKE_CXX_COMPILER_ID = Clang
CMAKE_CXX_COMPILER_VERSION = 9.0.1
CMAKE_CXX_COMPILER = /opt/local/bin/clang++-mp-9.0

I get for example

* We cannot find the test source file at:
*   ../tests/UT_Tests/ApprovalTestTests.cpp

This is all in CLion, so the build directory in the above case is:
ApprovalTests.cpp/cmake-build-debug-clang-mp-90-brew-ninja/

If I just change the generator to "Unix Makefiles" then the tests pass.

from approvaltests.cpp.

@jwillikers Yes, with the -Wno-c99-extensions patch, and CMake settings, this passes:

ApprovalTests.cpp/cmake-build-debug-clang-mp-90-brew-ninja-unity/tests/UT_Tests/UT_Tests

CMake Version = 3.16.2
CMake Generator = Ninja
CMAKE_CXX_COMPILER_ID = Clang
CMAKE_CXX_COMPILER_VERSION = 9.0.1
CMAKE_CXX_COMPILER = /opt/local/bin/clang++-mp-9.0
CMAKE_UNITY_BUILD = yes

:-)

from approvaltests.cpp.

@jwillikers I'm thinking I will add a note to the docs saying that if you can enable Unity builds on CMake 3.16 or above, it's one way to support Ninja with in-tree builds... With a link to this article: CMake 3.16 added support for precompiled headers & unity builds - what you need to know

from approvaltests.cpp.

@jwillikers I've just pushed a really big improvement to the troubleshooting docs for the Ninja issue

Huge thanks for all your help on this issue... That page has gone from "forget about Ninja" to "here are some good ways to fix it..."

And having set up CI builds for Ninja, and Ninja with Unity builds, I see that the latter has cut build times by 75%, so that's a win... I've changed my development builds to take advantage of this.

I'm going to close this issue now, in the hope that this is good enough now...

Please do say if you spot anything that's wrong or misleading....

Thanks again!

from approvaltests.cpp.