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

Doesn't compile on MinGW-w64 g++ #12

Closed isidore closed 5 years ago

isidore commented 5 years ago

This stackoverflow article Points out that there is a non-declared function in MinGW that approvaltests tries to use because it is windows.

You can fix this, by adding a small header file infront of approvaltests

File: MinGW.Patch.h

#ifdef __cplusplus

extern "C" {

#endif

#include <sec_api/stdlib_s.h> /* errno_t, size_t */

errno_t getenv_s(

    size_t     *ret_required_buf_size,

    char       *buf,

    size_t      buf_size_in_bytes,

    const char *name

);

#ifdef __cplusplus

}

#endif
claremacrae commented 5 years ago

Might an alternative to adding a patch be to fix our #ifdefs to prevent the windows code from being compiled in mingw - like we did with Cygwin?

claremacrae commented 5 years ago

I had a quick go on this before we pair on it on Friday, and I've found that:

This means that our way of detecting Unix-like environments on Windows for Cygwin doesn't work in mingw...

I've got a long way by using _MSC_VER instead...

There's also a problem of mingw not providing a valid POSIX mkdir() - as in https://stackoverflow.com/questions/12102147/too-many-arguments-to-function-int-mkdirconst-char - see the answer https://stackoverflow.com/a/14200178/104370

The changes need reviewing, but I can now get the code to build with mingw.

Sadly tests don't pass, as - I think - how Windows file paths are handled by mingw seems to be different from both native windows and cygwin.

claremacrae commented 5 years ago

The failing test is:

-------------------------------------------------------------------------------
Reporters Report Success Status
-------------------------------------------------------------------------------
...\ApprovalTests.cpp\ApprovalTests_Catch2_Tests\reporters\ReporterTests.cpp:39
...............................................................................

...\ApprovalTests.cpp\ApprovalTests_Catch2_Tests\reporters\ReporterTests.cpp:43: FAILED:
  REQUIRE( true == result )
with expansion:
  true == false
claremacrae commented 5 years ago

The symptom is in this code:

TEST_CASE("Reporters Report Success Status") {
    std::string knownCommand = SystemUtils::isWindowsOs() ? "C:\\Windows\\System32\\user.exe" : "echo";

With our current behaviour in mingw, SystemUtils::isWindowsOs() returns true, so knownCommand returns C:\Windows\System32\user.exe

When the failing test is run, this code in SystemLauncher::launch() returns false - and the front() value checked is the user.exe path mentioned above:

    bool launch(std::vector<std::string> argv) override
    {
        if (!exists(argv.front()))
        {
            return false;
        }
claremacrae commented 5 years ago

This issue was fixed in the v.3.4.1 release. Closing.