Skip to content
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

Diagnostic Aggregator Testing #4

Conversation

norro
Copy link

@norro norro commented Aug 22, 2019

This PR introduces a demo and two tests, but...:
One of the tests is supposed to test the analyzer creation of the aggregator from a yaml file. This one, however, is failing since the output it is supposed to match somehow only appears after the test times out.
The second test is succeeding, but only checks for output of the bond state machine, which somehow already appears before the test times out, but is not meaningful to test.

@Karsten1987: This kind-of fixes this as is shows how the tests should look like IMHO. Do you have an idea why the logging output is stalled until launch-testing sends a SIGINT to the process under test (aggregator_node)?
The stalling does not happen, if I start the aggregator_node with ros2 run (see the demo), so it might be me using ros2 launch-testing not in the intended way.

norro added 4 commits August 8, 2019 15:46
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
* Adds launch-based test, starting aggregator with a yaml configuration
* Test is not yet working, something wrong with process orchestration

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
* One passing test, just looking for output of the bond statemachine
* One failing test, looking for the actual analyzer output we want to
test for

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@norro norro mentioned this pull request Aug 22, 2019
norro added 3 commits August 22, 2019 10:15
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@norro norro changed the title WiP: Diagnostic Aggregator Testing Diagnostic Aggregator Testing Aug 26, 2019
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@norro
Copy link
Author

norro commented Aug 26, 2019

Four tests working, testing creation of various analyzers and analyzer groups from yaml specification.
@Karsten1987: This should fix ros#118

Copy link

@Karsten1987 Karsten1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me. I'd like to have the std::cout though addressed.

also I went ahead and pushed 087d391 to address some failing linters.

diagnostic_aggregator/src/aggregator.cpp Outdated Show resolved Hide resolved
* launch testing now working withou previous stdcout hack
* deleted deprecated (not working) tests

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@Karsten1987
Copy link

@norro I've seen you've updated your branch here. Is this good to be merged in your original PR?
I am about to cut a release for eloquent and was wondering whether that feature should be part of it.

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@norro
Copy link
Author

norro commented Dec 22, 2019

@Karsten1987 yes, this could/should be merged into the original PR, as it now has a small demo and working tests.

@Karsten1987 Karsten1987 merged commit baf5e98 into ros2_migrate_diagnostic_aggregator Dec 22, 2019
@norro norro deleted the ros2_migrate_diagnostic_aggregator_testing branch April 20, 2020 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants