Fix loading nested include with custom attributes #789
Conversation
Signed-off-by: Jenn Nguyen <[email protected]>
github-actions
bot
added
Gazebo 1️1️
jennuine
changed the title
Codecov Report
@@ Coverage Diff @@
## sdf9 #789 +/- ##
==========================================
+ Coverage 87.46% 87.56% +0.10%
==========================================
Files 63 63
Lines 9893 9894 +1
==========================================
+ Hits 8653 8664 +11
+ Misses 1240 1230 -10
Continue to review full report at Codecov.
|
test/integration/sdf_custom.cc
Outdated
| world2->Element()->FindElement("mysim:description"); | ||
| ASSERT_NE(nullptr, customDesc1); | ||
| ASSERT_NE(nullptr, customDesc2); | ||
| EXPECT_EQ(customDesc1->ToString(""), customDesc2->ToString("")); |
There was a problem hiding this comment.
Can we compare the result of customDesc2->ToString with the expected string literal instead of trusting customDesc1->ToString is working properly? Here and below. Not to make to verbose, I would only do it on the comparisons that involve custom elements.
Signed-off-by: Jenn Nguyen <[email protected]>
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1 |
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1 |
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1 |
🦟 Bug fix
Closes #502
Summary
When working on tests for #502 , I found a bug with nested includes where the included file contained custom attributes. In the parser's
addNestedModelcall, after the string replacement for flattening, the_includeSDFelement was cleared but not it's attributes. This resulted in the custom attribute being added twice and the duplicate attribute would be left with an empty value. This PR fixes the issue by clearing the attributes before callingreadStringand adds tests to ensure custom elements/attributes are preserved when loading a sdformat file and reloading the string output.Before PR
Check out this PR and comment out
_includeSDF->RemoveAllAttributes()from:https://github.com/ignitionrobotics/sdformat/blob/1c6c0a55d07cc75057f8cc44c5a25b85f155ee8e/src/parser.cc#L1375-L1377
Then run:
The error is:
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.