Bug: order-by=file is compiler-dependent

[DaanDeMeyer]

See https://github.com/DaanDeMeyer/doctest/runs/216609947.

Even more weird is that the output not only differs from the rest of the compilers but the output also differs between cl and clang-cl as well.

[onqtam]

I'll take a look as soon as possible but it could take 1-2 days... sorry.

[onqtam]

I noticed now that its not only the clang-cl builds which fail but also some cl builds: windows-2019-cl. If this was only a clang-cl problem I would have pointed you to check if this is the reason but if it happens with cl as well I'm clueless for now...

EDIT: you actually mentioned cl as well in the issue so it was never reported only for clang-cl...

[DaanDeMeyer]

When using clang-cl, __FILE__ expands to ..\\examples\\all_features/header.h for header.h and ..\\examples\\all_features\\main.cpp for all .cpp files.

When using clang++ (on Windows), __FILE__ expands to ../examples/all_features/header.h for header.h and ../examples/all_features/main.cpp for all .cpp files.

When using cl, __FILE__ expands to C:\\Users\\daan\\projects\\doctest\\examples\\all_features\\header.h for header.h and ..\\examples\\all_features\\main.cpp for all .cpp files.

So basically, we're going to have to do some sort of path normalization if we want reliable results from FileOrderComparator.

[onqtam]

Gosh... such "exciting" software engineering problems!
This issue is related to path normalization (on which I gave up on...): #45

one thing we could do is to use either a new hidden flag or no_path_in_filenames and something like skipPathFromFilename() so the output gets stable across compilers at least for testing doctest itself, and we could log a separate issue for fixing this properly later.

One of the pros of not normalizing paths (and using something like skipPathFromFilename()) is that we don't need to allocate memory for the filename strings - doctest just uses a const char* pointing to the memory in the binary with the constant.

EDIT: I'm open for feedback :|

[DaanDeMeyer]

Is it an option to change the default order_by value and just document the problem?

This feels like an edge case without a clean solution (at least until we can think of something) so I'd rather just avoid it from occurring for now.

EDIT: we could also just change the order_by value for the test for now.

[onqtam]

indeed I think changing the order to "name" or "suite" will do the trick!

And we should rename this issue to reflect the actual problem.

[DaanDeMeyer]

Should I change it just for the test or the default value? I think changing the default value is "the right thing to do" as the current behaviour is broken but users might be relying on the default order so it would be a breaking change.

I think changing the order just for the test is the safest thing for now. I'll also add documentation to the order-by option that it's compiler dependent.

[onqtam]

yes, changing it for the test only should be the safest thing to do.

[onqtam]

We could still keep this open, since the bug is still present..?

[DaanDeMeyer]

Of course, my bad.

[maidenone]

Ok this is horrible, but it solved our problem. Putting this here for others with simular problems.

We are compiling using cmake + clang + ninja on windows.

in all source and header files we only include the name: #include "file.hpp".

in the cmake file we declare include folders with:

target_include_directories(target PUBLIC "/proj/include/")

In our project, when we include a .hpp file from a .cpp file doctest get:

/proj/include/file.hpp

but if we include a .hpp from another .hpp it results in:

/proj/include\file.hpp

As we put tests in file.hpp, and include from .cpp and .hpp files those tests will run twice.
Our solution to this was to rewrite the path in the test_case constructor:

        m_type        = type;                         // line 4255 in doctest.h
        m_template_id = template_id;        // line 4256 in doctest.h

        for (size_t i = 0; i < m_file.size(); i++)
        {
            if (m_file[i] == '\\')
            {
                m_file[i] = '/';
            }
        }