Skip to content

Say that duplicate SubTree names are checked across the whole tree #1222

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.

Sign up for GitHub

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

Open
dv-picknik wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Say that duplicate SubTree names are checked across the whole tree #1222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/xml_parsing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1236,8 +1236,8 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree(
if(sub->instance_name == subtree_path)
{
throw RuntimeError("Duplicate SubTree path detected: '", subtree_path,
"'. Multiple SubTree nodes with the same 'name' attribute "
"under the same parent are not allowed. "
"'. SubTree nodes in the same tree cannot share a 'name' "
"attribute, even under different parent nodes. "
"Please use unique names or omit the 'name' attribute "
"to auto-generate unique paths.");
}
Expand Down
44 changes: 43 additions & 1 deletion tests/gtest_subtree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ TEST(SubTree, SubstringTreeIDsAreNotRecursive)
// Test for Groot2 issue #56: duplicate _fullpath when multiple subtrees have the same name
// https://github.com/BehaviorTree/Groot2/issues/56
//
// When two SubTree nodes under the same parent have the same "name" attribute,
// When two SubTree nodes in the same tree have the same "name" attribute,
// tree creation should fail with a clear error message.
TEST(SubTree, DuplicateSubTreeName_Groot2Issue56)
{
Expand Down Expand Up @@ -879,6 +879,48 @@ TEST(SubTree, DuplicateSubTreeName_ErrorMessage)
}
}

// The duplicate check covers the whole tree, so the message must not suggest
// that moving one SubTree under another parent fixes it.
TEST(SubTree, DuplicateSubTreeName_DifferentParents)
{
// clang-format off
static const char* xml_text = R"(
<root BTCPP_format="4" main_tree_to_execute="MainTree">
<BehaviorTree ID="MainTree">
<Fallback>
<Sequence>
<SubTree ID="Task" name="my_task"/>
</Sequence>
<Sequence>
<SubTree ID="Task" name="my_task"/>
</Sequence>
</Fallback>
</BehaviorTree>

<BehaviorTree ID="Task">
<AlwaysSuccess/>
</BehaviorTree>
</root>
)";
// clang-format on

BehaviorTreeFactory factory;

try
{
(void)factory.createTreeFromText(xml_text);
FAIL() << "Expected RuntimeError to be thrown";
}
catch(const RuntimeError& e)
{
std::string msg = e.what();
EXPECT_TRUE(msg.find("Duplicate SubTree path") != std::string::npos)
<< "Error message should mention 'Duplicate SubTree path'. Got: " << msg;
EXPECT_TRUE(msg.find("same parent") == std::string::npos)
<< "Error message should not limit the rule to one parent. Got: " << msg;
}
}

// Test that unique names under the same parent work correctly
TEST(SubTree, UniqueSubTreeNames_WorksCorrectly)
{
Expand Down
Loading