Separate test utilities between modules

[kimci86]

Description

For now, test utilities for all SFML modules are compiled in a single library, which means for example that tests for the System module are linked with Graphics testing utilities. That was ok because apparently the linker ignores what is not used in a static library, but that looks like a hack to me. It starts to be a problem with linker errors in the work in progress feature/test_graphics branch.

To fix the issue, one solution would be to have a single test executable for all modules instead of one per module.
Another solution is to have specialized test utility libraries for each module.
This is the approach I took with this PR.

As a bonus, it then becomes simple to enable testing when not all modules are built, which was required since #2050.
#2050 was motivated by simplicity, so I hope my PR adds only a reasonable amount of complexity.

How to test this PR?

CI

[codecov]

Codecov Report

Merging #2170 (ffd061b) into master (aa82ea1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2170   +/-   ##
=======================================
  Coverage   13.90%   13.90%           
=======================================
  Files         189      189           
  Lines       15813    15813           
  Branches     4169     4169           
=======================================
  Hits         2199     2199           
  Misses      13461    13461           
  Partials      153      153           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa82ea1...ffd061b. Read the comment docs.

[ChrisThrasher]

My biggest issue with this PR is that it's solving two somewhat unrelated problems. The first is the problem that all tests are getting symbols from potentially unrelatedly libraries and that is prone to cause issues. At a minimum, it's just a bit ugly to link Graphics symbols into the System tests. The second problem is the desire to minimize compile time for those who aren't building all of SFML but still want to build the tests. I have previously explained why I don't like supporting this use case in #2050 so I won't repeat myself.

I can get behind solving the first problem if the implementation is good. My most significant request is that if we do make the test infrastructure more complicated, we do it with a clear plan that will lead to complexity leveling out soon. I hope that as tests get more sophisticated, our testing infrastructure won't need continuous hacks to keep up.

[kimci86]

Thank you for your feedback @ChrisThrasher.

My biggest issue with this PR is that it's solving two somewhat unrelated problems. The first is the problem that all tests are getting symbols from potentially unrelatedly libraries and that is prone to cause issues. At a minimum, it's just a bit ugly to link Graphics symbols into the System tests. The second problem is the desire to minimize compile time for those who aren't building all of SFML but still want to build the tests. I have previously explained why I don't like supporting this use case in #2050 so I won't repeat myself.

You are right. Regarding the first described problem, I see two solution:

• merging test executables into one.
• splitting the test utility library into several.

I don't have strong arguments for one or the other. My preference went to the second option. That seems more consistent with the fact that SFML is a collection of libraries.

If we choose the second solution like in this PR, then solving the second described problem is as simple as four if/endif pairs, so at this point, why not.
Indeed, contributors interested in running tests might usually build all modules, but in some rare cases such as working on the raspberry pi, disabling modules can save minutes of compilation time.

My most significant request is that if we do make the test infrastructure more complicated, we do it with a clear plan that will lead to complexity leveling out soon. I hope that as tests get more sophisticated, our testing infrastructure won't need continuous hacks to keep up.

As I see it, the infrastructure suggested in this PR seems generic enough so that we do not have to constantly hack the build system to write more expressive tests. At least, that is my guess.

[ChrisThrasher]

I'm still not in love with going back to optionally building tests but at least with this new scheme of having 1 util library per module, we can actually implement that in a reasonable fashion. I made a couple small remarks but not enough to stand in the way of my approval so long as most of the commits get squashed. Thanks, Kimci!

[ChrisThrasher]

I'm not sure we need these comments anymore since all the users of this function reside in the same file. Same goes for sfml_add_test's description comment.

[kimci86]

I don't see a big difference if the code using those functions is in the same file or not.
A small description doesn't hurt IMHO.

[ChrisThrasher]

What do you think about sfml_add_test_utils also using this same naming logic where instead of providing a target name, you just provide the module name?

[kimci86]

I was tempted to do it, but I think it might make the code a little less clear because it would be less obvious where the test utility library target names come from. But I don't feel strongly about it, I could be convinced otherwise.

Consider:

sfml_add_test_utils(sfml-system-test-utils ...)
target_include_directories(sfml-system-test-utils PUBLIC TestUtilities)

versus

sfml_add_test_utils(System ...)
target_include_directories(sfml-system-test-utils PUBLIC TestUtilities)

[kimci86]

Rebased and cleaned up commit history.