3 ➡️ 4 logging

[mabelzhang]

Address the logging test failures in #148
A lot of changes so I'm opening a PR instead of just pushing to 3_to_4 branch.

The reason that that empty path test was failing, was because we don't allow the user to use <path>, but internally we inject <path> for the plugin to read. When the test specifies <path> in an SDF, it's ignored - which means if it exists, it isn't removed. Then when LogRecord sees the tag, it doesn't know whether that <path> came from the injection (legit) or the user (or test in this case, not legit). The plugin just goes ahead and records to it, even when it came from the user.

To avoid that confusion, we can either check for user-specified <path> and strip it before adding the legit one (70c79be), which I then realized looks silly, so I just changed the tag name we inject to <record_path>. Now there won't be confusion. I added a TODO item to put it in a Component.

I also realized a lot of tests needed to be updated so that they aren't using the removed <path> feature. Two are removed because we aren't allowing SDF <path> anymore, so those two cases are no longer different from the existing ones.

P.S. I’m seeing a lot of clang_tidy warnings in many files, I don’t know if we want to fix them as we’re doing 3_to_4.

[codecov]

Codecov Report

Merging #151 into 3_to_4 will not change coverage.
The diff coverage is 27.27%.

@@           Coverage Diff           @@
##           3_to_4     #151   +/-   ##
=======================================
  Coverage   65.90%   65.90%           
=======================================
  Files         128      128           
  Lines        6224     6224           
=======================================
  Hits         4102     4102           
  Misses       2122     2122           

Impacted Files	Coverage Δ
src/ign.cc	26.95% <0.00%> (+0.23%)	⬆️
src/ServerPrivate.cc	68.48% <22.22%> (-0.42%)	⬇️
src/systems/log/LogRecord.cc	34.38% <100.00%> (ø)

---

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 eec3984...7adf761. Read the comment docs.

[chapulina]

Elegant move with the <path> -> <record_path> change! Also thanks for removing all these tests we don't need anymore.

The OSX build failure seems unrelated, but we should keep an eye on it because the log_system test has several strategically placed ifndef __APPLE__s.