Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix faulty assumption in INTEGRATION_log_system #1426

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Conversation

mjcarroll
Copy link
Contributor

🦟 Bug fix

Fixes #

Summary

I believe that gazebosim/gz-common#332 uncovered a faulty assumption in the log recorder test. ignLogDirectory should not be empty, it should be initialized to .ignition if the console logger is not properly initialized. This adjusts the assumption to be correct.

I've seen this pop up in CI (https://build.osrfoundation.org/job/ignition_gazebo-ci-pr_any-ubuntu_auto-amd64/8397/testReport/junit/(root)/LogSystemTest/LogDefaults/) and can reproduce locally on the main branch.

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.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll requested a review from chapulina as a code owner April 4, 2022 21:59
@mjcarroll mjcarroll self-assigned this Apr 4, 2022
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Apr 4, 2022
@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #1426 (e04f88b) into main (f6ba375) will decrease coverage by 0.01%.
The diff coverage is 14.81%.

@@            Coverage Diff             @@
##             main    #1426      +/-   ##
==========================================
- Coverage   62.34%   62.32%   -0.02%     
==========================================
  Files         317      317              
  Lines       24419    24418       -1     
==========================================
- Hits        15224    15219       -5     
- Misses       9195     9199       +4     
Impacted Files Coverage Δ
.../gui/plugins/transform_control/TransformControl.cc 7.36% <ø> (+0.01%) ⬆️
...lization_capabilities/VisualizationCapabilities.cc 3.66% <0.00%> (ø)
src/rendering/RenderUtil.cc 36.94% <0.00%> (ø)
src/systems/contact/Contact.hh 100.00% <ø> (ø)
src/systems/particle_emitter/ParticleEmitter.cc 55.40% <0.00%> (ø)
src/systems/scene_broadcaster/SceneBroadcaster.hh 100.00% <ø> (ø)
src/systems/thruster/Thruster.cc 92.30% <ø> (-0.06%) ⬇️
...ems/collada_world_exporter/ColladaWorldExporter.hh 100.00% <100.00%> (ø)
...ems/kinetic_energy_monitor/KineticEnergyMonitor.hh 100.00% <100.00%> (ø)
src/systems/user_commands/UserCommands.hh 100.00% <100.00%> (ø)
... and 4 more

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 8c690ff...e04f88b. Read the comment docs.

@chapulina chapulina merged commit 360d573 into main Apr 6, 2022
@chapulina chapulina deleted the log_system_fix branch April 6, 2022 00:56
@Blast545
Copy link
Contributor

Blast545 commented Jun 8, 2022

Blast545 pushed a commit that referenced this pull request Jun 9, 2022
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Blast545 added a commit that referenced this pull request Jun 9, 2022
Signed-off-by: Michael Carroll <michael@openrobotics.org>

Co-authored-by: Michael Carroll <michael@openrobotics.org>
@j-rivero j-rivero mentioned this pull request Sep 16, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants