search

CMakePP / CMakePPLang

Object-oriented extension to the CMake language.
http://cmakepp.github.io/CMakePPLang/
Apache License 2.0
11 stars 4 forks source link

Port tests to new CMakeTest API #26

Closed AutonomicPerfectionist closed 2 years ago

AutonomicPerfectionist commented 2 years ago

This pull request ports all of the tests to the new CMakeTest API. It also fixes some test name conflicts as well as a few bugs that caused some tests to fail even after conversion.

List of bugs fixed:

  1. Incorrect signature for cpp_raise(), causing an error when in debug mode and encountering a cpp_raise() with varargs. Documentation also updated.
  2. Several incorrect usages of if statements causing some values to be double-evaluated. Only fixed what was required in type_of.cmake to fix incorrect type reporting. Extensive refactoring is required to fix all other instances. Warnings are sometimes printed due to these incorrect usages, only occur during first-time runs in non-CTest environments.
  3. tests/stress_test/stress_map.cmake appears to use an older CMakePPCore API. It has been updated to use the current API.
AutonomicPerfectionist commented 2 years ago

The tests in this pull request are failing because I haven't yet committed the tests/CMakeLists.txt, I had to override the FetchContent_declare() to point to my fork of CMakeTest because I encountered the same bug with not being able to locate the include file. Once my CMakeTest changes are pulled into upstream I can remove my override and push the updated CMakeLists.txt

AutonomicPerfectionist commented 2 years ago

@ryanmrichard all tests that have been ported are now succeeding as I've pushed the tests/CMakeLists.txt file. I did have to disable one because it is apparently expecting a different implementation of ct_assert_prints(). Rather than rewriting the test to function correctly I figured we should rather fix the implementation in CMakeTest, hence the bug report in CMakePP/CMakeTest#40.

ryanmrichard commented 2 years ago

@AutonomicPerfectionist that sounds good and thanks so much for doing this. Your comment makes it seem like it's ready to merge, but it's still set to draft status. Were you planning on adding anything else?

AutonomicPerfectionist commented 2 years ago

Yes, sorry, there's still a few tests left to port, they should be done soon

AutonomicPerfectionist commented 2 years ago

Two tests failed after being ported in tests/types/implicitly_convertible.cmake, the first tests whether user-defined types are convertable to strings and the second does the same but convertable to lists. I have simply disabled them for now while I finish porting the types tests, but I will go back and take a look at why they fail

AutonomicPerfectionist commented 2 years ago

@ryanmrichard all tests have been converted, the ones that failed have been fixed and re-enabled. In order for the tests in this PR to succeed we'll need to pull CMakePP/CMakeTest#43 into master.