-
Notifications
You must be signed in to change notification settings - Fork 661
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
More roundabout phrase tests #2382
Conversation
std::string guide_branch = | ||
GetGuideBranchString(static_cast<uint32_t>(std::round(max_count / 2)), | ||
GetGuideBranchString(static_cast<uint32_t>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mandeepsandhu for flagging the integer division
// Create guide sign | ||
// Specify input in descending consecutive count order | ||
Signs signs = | ||
GetGuideSigns({std::make_tuple("US 322 West", 1, 1), std::make_tuple("US 22 West", 1, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a test case where the consecutive count is > 1, say 2 of them have 2
and the result shows both items getting selected when limit_by_consecutive_count
is set (for different values of. max_count
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mandeepsandhu pushed up more tests
Added more TestGetGuideString tests with no, 1, and multiple consecutive counts
TryGetGuideString( | ||
signs, 0, true, | ||
"US 322 West/US 22 West/Freedom Highway/Valhalla Highway/A 1/Remscheid/Wermelskirchen/Hückeswagen"); | ||
TryGetGuideString(signs, 4, true, "US 322 West/US 22 West/A 1/Remscheid"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is Remscheid
being returned here? I thought if limit_by_consecutive_count
is set, only entries with the same consecutive count are returned? In this case A 1
has a consecutive count of 1, while Remscheid
's is 0, which should make us drop Remscheid
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is
Remscheid
being returned here? I thought iflimit_by_consecutive_count
is set, only entries with the same consecutive count are returned? In this caseA 1
has a consecutive count of 1, whileRemscheid
's is 0, which should make us dropRemscheid
, no?
Sorry, ignore this question. Thats not consecutive count I was looking at, but the "is route" attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the unit tests. LGTM.
Issue
Signs::GetGuideString
unit testsSigns::GetGuideString
to properly round or truncate max_count for each sign typeexpect_instructions_at_maneuver_index
methodTasklist