Fix Windows Path Mangling

[zachcran]

Is this pull request associated with an issue(s)?
This issue was partially solved in #101. However, bugs related to it still remain and are reported in #109. This should fix #109 in a more robust way.

Description
Tests generated by CMakeTest using ct_add_dir() use the full path to the test file as a way to disambiguate the test "mini-projects" created in the build directory that are actually used to run the tests. On Windows, these path snippets are not properly being sanitized for their project name and folder name uses. This PR uses cpp_sanitize_string() from CMakePPLang to make convert the paths into file system-friendly strings where it is needed and cleans up a lot of the code in and associated with ct_add_dir().

One other issue that I am immediately running up against on Windows is the path length limitation (256 characters). You can opt-in to removing this limitation in Windows 10 or later, and attempt to limit object path and file lengths in CMake (see SO post and its answers), but even with all of that implemented I am running into compiler issues. It turns out that applications still have to be modified to take advantage of the path length being removed (see important note at the top of this section), which may be part of the issue. Additionally, to disable this max file path limitation you need administrator privileges, which will not be available to every user, and registry editing is always annoying and a dangerous task anyway.

Therefore, to even get the CMakeTest tests passing on Windows, I propose expanding this PR to include switching to a hash-based naming system based on the path strings (string(<HASH>). We can use the file contents of the test file as well, if needed (file(<HASH>).

TODOs

• Implement the hash-based naming system for tests.

[zachcran]

CMake only provides certain hashing algorithms and they all generate strings that are quite long. I want to truncate the hash string to as few characters as possible to shorten paths, while still reasonably avoiding collisions. Following GitHub's lead for referring to commits, I think truncating to 7 characters should be more than reasonable in any project to disambiguate tests. This would generate paths like build/tests/tests/9a67b54/7c546f8 instead of build/tests/tests/c__programs_cmakepp_workspace_cmaketest_tests_test_directory_2/test_same_name_diff_directory_smake.

As a reference for how many characters of a hash is probably enough to avoid collisions when generating tests, see this git repo that summarizes it. Also, check out the wiki about birthday attacks for a table about how the number of bits in a hash affect the chances of collision.

[zachcran]

It turns out that add_test() allows for test names with arbitrary characters. I think it is much more readable to give the test names as the actual paths instead of the sanitized versions. Consider:

C:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/cmake_test/add_test.cmake

versus

c__users_zachc_programs_cmakepp_workspace_cmaketest_tests_cmake_test_add_test_cmake

@ryanmrichard @AutonomicPerfectionist What do you think?

[AutonomicPerfectionist]

It turns out that add_test() allows for test names with arbitrary characters. I think it is much more readable to give the test names as the actual paths instead of the sanitized versions. Consider:

C:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/cmake_test/add_test.cmake

versus

c__users_zachc_programs_cmakepp_workspace_cmaketest_tests_cmake_test_add_test_cmake

@ryanmrichard @AutonomicPerfectionist What do you think?

Off the top of my head I think that should work, although I'm awfully fuzzy on the details of that segment of the code.

[zachcran]

Off the top of my head I think that should work, although I'm awfully fuzzy on the details of that segment of the code.

Thanks! It is fine in my testing (CMakeTest tests, CMaize tests, and the example project from #109 on both Windows and Linux). I wonder if it was just written that way because you were already sanitizing the paths to make the test directories, then just put those together to get the unique name.

[ryanmrichard]

It turns out that add_test() allows for test names with arbitrary characters. I think it is much more readable to give the test names as the actual paths instead of the sanitized versions. Consider:

C:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/cmake_test/add_test.cmake

versus

c__users_zachc_programs_cmakepp_workspace_cmaketest_tests_cmake_test_add_test_cmake

@ryanmrichard @AutonomicPerfectionist What do you think?

I think the top one is much easier to read. We may want to add the option to use relative paths (relative to the directory ct_add_dir was called on) for the test names to shorten that a bit more.

[ryanmrichard]

To fix CI, in the .github/.licenserc.yaml file add *.txt.in to the paths-ignore list (since you didn't modify that file as part of the PR, I can't suggest it without pulling the PR and I'm too lazy).

[zachcran]

We may want to add the option to use relative paths (relative to the directory ct_add_dir was called on) for the test names to shorten that a bit more.

To be clear, do you mean a CMake option so it could be set like -DCT_USE_REL_PATH_NAMES=ON?

[ryanmrichard]

We may want to add the option to use relative paths (relative to the directory ct_add_dir was called on) for the test names to shorten that a bit more.

To be clear, do you mean a CMake option so it could be set like -DCT_USE_REL_PATH_NAMES=ON?

I was thinking more at the ct_add_dir level. I'd argue that it's really the build system maintainer that should be choosing the name of the test cases. If they want to open it up to their user it's like two lines of CMake to introduce a CMake option and forward its value to ct_add_dir.

[zachcran]

I was thinking more at the ct_add_dir level. I'd argue that it's really the build system maintainer that should be choosing the name of the test cases. If they want to open it up to their user it's like two lines of CMake to introduce a CMake option and forward its value to ct_add_dir.

Sounds good to me. Where should the relative path be from? The easiest would be to just use the relative path to the test file from the added directory, which we already have computed, with the test directory provided to ct_add_dir() prepended. Without prepending the test directory, it would actually break the test_directory_2/test_same_name_diff_directory.cmake test.

[zachcran]

If we are concerned about building the tests from multiple projects at the same time, then the better way would probably be to use a relative path from the project root, with the project name at the front. That would provide much less chance of name collisions and specify the project while still being relatively short.

c:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/test_directory_2/test_same_name_diff_directory.cmake

versus

CMakeTest/tests/test_directory_2/test_same_name_diff_directory.cmake

Pytest does something similar by specifying tests as tests/subdir/test_file.py::TestCase::test_name, although building and running the tests aren't usually an issue in Python projects so it doesn't prepend a project name or anything.

[zachcran]

As an initial implementation, I used compromise between what I mentioned in my previous two comments: ${PROJECT_NAME}::${test_dir_name}/${relative_path_to_test_file} making

c:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/test_directory_2/test_same_name_diff_directory.cmake

into

CMakeTest::test_directory_2/test_same_name_diff_directory.cmake

and

c:/Users/zachc/programs/cmakepp_workspace/cmaketest/tests/cmake_test/test_same_name_diff_directory.cmake

into

CMakeTest::cmake_test/test_same_name_diff_directory.cmake

This should be sufficient to differentiate tests in different projects and different test directories from each other. I tested it on CMakeTest and CMaize's tests on both Linux and Windows with success.