-
Notifications
You must be signed in to change notification settings - Fork 1
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
[22317] Test service node listener #62
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Javier Gil Aviles <[email protected]>
Signed-off-by: Javier Gil Aviles <[email protected]>
c0460e3
to
4de5353
Compare
Signed-off-by: Javier Gil Aviles <[email protected]>
Signed-off-by: Javier Gil Aviles <[email protected]>
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.
This test suite will gracefully assert the feature's correct behavior, good work.
Leaving some suggestions though
sustainml_cpp/include/sustainml_cpp/core/RequestReplyListener.hpp
Outdated
Show resolved
Hide resolved
sustainml_cpp/include/sustainml_cpp/core/RequestReplyListener.hpp
Outdated
Show resolved
Hide resolved
sustainml_cpp/include/sustainml_cpp/nodes/AppRequirementsNode.hpp
Outdated
Show resolved
Hide resolved
/** | ||
* @brief This method sends a request to node via service for changing its configuration. | ||
* @param [in] req configuration request, contain which node and configuration file | ||
* @param [out] res response from the node with its new configuration |
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.
With the last update, this line should be removed and specify the return
instead
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.
I think this suggestion was not applied, I am missing the return
in the doxygen comment
sustainml_cpp/test/blackbox/common/BlackboxTestsResponseNodes.cpp
Outdated
Show resolved
Hide resolved
sustainml_cpp/test/blackbox/common/BlackboxTestsResponseNodes.cpp
Outdated
Show resolved
Hide resolved
Additionally, we should uncrustify the |
Signed-off-by: Javier Gil Aviles <[email protected]>
sustainml_cpp/include/sustainml_cpp/core/RequestReplyListener.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Javier Gil Aviles <[email protected]>
4a18663
to
d73e7f1
Compare
This PR add the blackbox tests for service node listener and the minimum needed implementation.