Directory separators handled incorrectly on Windows with Clang about approvaltests.cpp HOT 8 CLOSED

Thanks for letting me know about this - I'll have a go at reproducing it..

Any extra info that you could provide about how your build was set up would be really helpful...

We already have code for something like this with (I think) Mingw and Cygwin builds, which all vary in how they handle file paths and launching processes.

So before taking on another dependency, it would be worth looking at whether the fixes we have for those platforms can be generalised in any way to cover clang on Windows as well.

I think this also means it's time to set up a Windows/Clang CI build too...

from approvaltests.cpp.

My build was set-up using visual studio with the clang_cl_x64 environment. I used the source_location functionality (__builtin_FILE()) and the path provided was in unix style. ApprovalTests then creates a file with a name like this (on the binary's folder):

C__Folder_main.test0.approved

The patch is easy to make. This:

    static std::string getDirectorySeparator()
    {
        return isWindowsOs() ? "\\" : "/";
    }

can become this:

```cpp
    static std::string getDirectorySeparator()
    {
#if defined(__clang__)
        return "/";
#else
        return isWindowsOs() ? "\\" : "/";
#endif
    }

But I've seen cases were paths come with \ and / (past experience - don't have a code example now). To account for this, a more robust solution would be required to be implemented and tested against these types of weird scenarios, that differ depending on OS and compiler. That reason why I recommended the Filesystem lib. It's well tested (96% coverage), source compatible with std::filesystem (which enable for a smooth transition on the future), handles UTF-8 and would allow for removal of code from Approval Tests (less is better :) ).
Another option would be to create a filesystem header with just the functions Approval Tests needs. Seems it only needs exists, path, stem and extension functions, which can make for a small header.
What do you think?

from approvaltests.cpp.

I've tried reproducing this with the following:

1. Update to latest Visual Studio 2019
2. Install "C++ Clang tools for Windows (9.0.0 - x64/x86)
3. Launch Visual Studio
4. File > Open > Open Folder
5. Select my clone of Approval Tests.cpp and open it
6. Select "Manage Configurations" (from a drop-down in a toolbar)
7. Click the green "+"
8. Add "x64-Claren-Release"
9. Save the CMakeSettings.json file
10. Set the Startup Program to be DocTest_Tests.exe
11. Run

I then get output beginning with this:

[doctest] doctest version is "2.3.5"
[doctest] run with "--help" for options
===============================================================================
..\..\..\tests\DocTest_Tests\ApprovalTestTests.cpp(7):
TEST CASE:  YouCanUseAWriter

..\..\..\tests\DocTest_Tests\ApprovalTestTests.cpp(7): ERROR: test case THREW exception: Unable to create directory: ..\..\..\tests\DocTest_Tests\approval_tests\

===============================================================================
..\..\..\tests\DocTest_Tests\ApprovalTestTests.cpp(16):
TEST CASE:  YouCanSpecifyYourFileExtension

..\..\..\tests\DocTest_Tests\ApprovalTestTests.cpp(16): ERROR: test case THREW exception: Unable to create directory: ..\..\..\tests\DocTest_Tests\approval_tests\

This looks like it may be the same problem as is logged in #55

from approvaltests.cpp.

Hi @lp55

I don't think I've managed to reproduce this - when you have time, please could you give me the steps to reproduce it?

Looking at #29 it does seem that someone has previously been able to use this project with clang-cl - so I'm wondering what's different either with Visual Studio these days, or perhaps your set up now?

Googling also didn't leave me confident in what installer exactly to use for clang_cl_x64...

from approvaltests.cpp.

from approvaltests.cpp.

from approvaltests.cpp.

We added a help message in the v.7.0.0 release that points to this page - so hopefully if people see this problem, it will be easier to diagnose.

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

Leaving this open, as the issue itself has not been addressed.

from approvaltests.cpp.

Closing, as this is more than a year old, there have been no further reports, and we do have CI running the tests in Visual
Studio with clang.

So this is very likely to be correct.

from approvaltests.cpp.