jbeder / yaml-cpp

A YAML parser and emitter in C++

MIT License

4.92k stars 1.78k forks source link

Move CTest to the test entry #1181

Closed jelin-sh closed 1 year ago

jelin-sh commented 1 year ago

If CTest is a global reference, then YAML_CPP_BUILD_TESTS will be called and the compilation product will be generated regardless of whether YAML_CPP_BUILD_TESTS is enabled or not. So it makes more sense to put include(CTest) inside the if (YAML_CPP_BUILD_TESTS) statement.

if(YAML_CPP_BUILD_TESTS)
  include(CTest)
  add_subdirectory(test)
endif()

1180

jbeder commented 1 year ago

Fixes #1180

MatthijsBurgh commented 1 year ago

This one needs to be reverted.

I changed this, because no tests were run in CI. (#1170)

This PR reverts that all again. See https://github.com/jbeder/yaml-cpp/actions/runs/4480153307/jobs/7877729295?pr=1181#step:6:17 No tests were found!!!

So I suggest to revert this PR. And after it, @skjsnb can think about a different solution.

I don't see why the solution before this PR was an issue to any one. As it checks for YAML_CPP_BUILD_TESTS before including the test sub dir.

jbeder commented 1 year ago

Reverted, thanks for noticing. Open to any suggestions.

jelin-sh commented 1 year ago

@MatthijsBurgh I'm sorry for causing trouble to you. I indeed didn't think about the part concerning the CI process, so I agree with your opinion.

MatthijsBurgh commented 1 year ago

@jbeder thanks for the quick response