Move test cache to build folder

[chapulina]

🦟 Bug fix

Summary

While debugging a test failure, I noticed that the tests in ign_src_TEST would fail if run from outside the build directory. So I fixed this by passing PROJECT_BINARY_PATH to all the places where the test_cache is accessed. In order to reduce duplication, I moved that logic to the SetUp function. While at it, I reduced more duplication by moving the IO functions to SetUp / TearDown.

The fix can be checked by running the test from a directory other than build, for example:

./build/ignition-fuel_tools5/bin/UNIT_ign_src_TEST

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[codecov]

Codecov Report

Merging #222 (2cb3f61) into ign-fuel-tools4 (e88d5e6) will not change coverage.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           ign-fuel-tools4     #222   +/-   ##
================================================
  Coverage            77.90%   77.90%           
================================================
  Files                   19       19           
  Lines                 2715     2715           
================================================
  Hits                  2115     2115           
  Misses                 600      600           

---

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 e88d5e6...2cb3f61. Read the comment docs.

[mjcarroll]

We don't currently depend on ign-common here, but if we did, it would be even safer to utilize TempDirectory to guarantee that the cache is destroyed between runs of the test.

[mjcarroll]

Must have been thinking of a different library, we do actually depend on ign-common.

[chapulina]

it would be even safer to utilize TempDirectory to guarantee that the cache is destroyed between runs of the test.

Good idea. We'd have to backport that to ign-common3 though, this PR is targeting Citadel.

One thing to point out though is that the cache isn't deleted here between tests. That's helpful when debugging test failures so the user can inspect the cache after running a test. I don't think it's extremely necessary though.

[mjcarroll]

One thing to point out though is that the cache isn't deleted here between tests. That's helpful when debugging test failures so the user can inspect the cache after running a test. I don't think it's extremely necessary though.

The TempDirectory class currently has a flag that will allow you to block the cleanup. This is only exposed in code, but may be useful to also have as an environment variable?

https://github.com/ignitionrobotics/ign-common/blob/ign-common4/include/ignition/common/TempDirectory.hh#L87-L97

[chapulina]

The TempDirectory class currently has a flag that will allow you to block the cleanup. This is only exposed in code, but may be useful to also have as an environment variable?

Oh nice! An env var could be helpful, I personally don't mind adding a line while debugging to avoid cleanup though, good to know it's exposed through the API