Feature/test state machine lib #1744
Conversation
| ASSERT_STREQ(state.getEnteredKey().c_str(), "") << "entered_key should be " << ""; | ||
|
|
||
|
|
||
| std::ostringstream oss; |
There was a problem hiding this comment.
This is a bit fragile, why do you need to check std::cout? You could create a fixture that gets updated when a state transition or whatever the state machine does.
There was a problem hiding this comment.
@esteve the tests have been updated, please have a look now. Thanks!
There was a problem hiding this comment.
Looks good. I'd go further and remove showStateName() from the State API, so as to separate the logic from the UI (in this case the console). It's only being used in one place https://github.com/sgermanserrano/Autoware/blob/feature/test_state_machine_lib/ros/src/common/libs/state_machine_lib/src/state_context.cpp#L142
| ASSERT_STREQ(state.getEnteredKey().c_str(), "") << "entered_key should be " << ""; | ||
|
|
||
|
|
||
| std::ostringstream oss; |
There was a problem hiding this comment.
Looks good. I'd go further and remove showStateName() from the State API, so as to separate the logic from the UI (in this case the console). It's only being used in one place https://github.com/sgermanserrano/Autoware/blob/feature/test_state_machine_lib/ros/src/common/libs/state_machine_lib/src/state_context.cpp#L142
|
@sgermanserrano I'd like to try GitLab CI of this pull request with the latest feature/rebuild_decision_maker #1609 . May I take over the target branch feature/test_PR_1609 and request review to @s-azumi ? @esteve Were the requested changes solved? |
|
@kfunaoka yes, I guess feature/test_PR_1609 needs to be updated with the latest changes. This is the pipeline for the latest commit on this PR https://gitlab.com/ServandoGS/Autoware/pipelines/40012712 |
…ture/test_state_machine_lib
|
@kfunaoka the pipeline succeeded after the update https://gitlab.com/ServandoGS/Autoware/pipelines/44809073 |
There was a problem hiding this comment.
I only have two comments, @sgermanserrano could you address them? Thanks. I'm approving the pull request so it can be merged after you make the changes.
Status
DEVELOPMENT
Description
Added intial set of unit tests for state_machine_lib.
Related PRs
#1609