-
Notifications
You must be signed in to change notification settings - Fork 38
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
[WIP] External dir, Test, Plugin doc updates following new plugin architecture #95
[WIP] External dir, Test, Plugin doc updates following new plugin architecture #95
Conversation
@@ -188,7 +188,7 @@ TEST(ExampleImpl, NoTest) | |||
|
|||
### Writing Integration Tests {#integration_tests} | |||
|
|||
DroneCore provides the `dronecore-integrationtests` application for running the integration tests and | |||
DroneCore provides the `integration_tests_runner` application for running the integration tests and |
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.
@julianoes Is this correct name now?
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.
yes, either run it with:
build/default/integration_tests/integration_tests_runner
or
make run_integration_tests
|
||
``` | ||
make run_unit_tests | ||
make run_unit_tests |
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.
@julianoes This runs only core unit tests. I tried make run_unit_tests EXTERNAL_DIR=../DroneCore_Extensions
but no luck.
How do we run unit tests for external libraries?
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, that's correct, that's currently broken.
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.
Is there an issue I can track this with?
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.
Or more specifically, what is the syntax, and I can update the docs even if it doesn't work yet.
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.
No, it's just not working right now and there is a lot of churn around this given the gRPC changes so not worth it to fix right now.
@@ -58,7 +58,7 @@ make posix gazebo | |||
|
|||
Then run the tests as shown: | |||
``` | |||
make run_integration_tests | |||
make run_integration_tests |
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.
@julianoes How do we run integration tests for external examples?
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.
Yer that's currently broken :(.
en/contributing/test.md
Outdated
@@ -68,7 +68,7 @@ make run_integration_tests | |||
Make sure you are connected to a vehicle and check the connection using e.g.: | |||
|
|||
``` | |||
make && build/default/dronecore-integrationtests --gtest_filter="SitlTest.TelemetryAsync" | |||
make && build/default/dronecore-integrationtests --gtest_filter="SitlTest.TelemetryAsync" |
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.
@julianoes This doesn't work.
- How do we run this? I tried
make && build/default/integration_tests --gtest_filter="SitlTest.TelemetryAsync"
but no luck - How do we do this for external integration tests?
- Similarly, all the gtest tricks in next section don't work - how do they need to be adapted?
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.
make && build/default/integration_tests/integration_tests_runner --gtest_filter="SitlTest.TelemetryAsync"
- WIP
- See 1.
|
||
The only difference is that all external tests should be compiled into their own "test runner". | ||
The test runner for the "external example" is defined in | ||
[/integration_tests/CMakeLists.txt]( https://github.com/dronecore/DroneCore/blob/{{ book.github_branch }}/external_example/integration_tests/CMakeLists.txt) (named `external_example_integration_tests_runner`). |
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.
So as above,I can see how test runner is created, but not how it "should" be launched.
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.
make && build/default/integration_tests/integration_tests_runner
@@ -91,5 +94,6 @@ sudo make default install # sudo needed to install files into system directori | |||
|
|||
### Locking/Unlocking the SDK | |||
|
|||
Functionality to deliver in: https://github.com/dronecore/DroneCore/pull/139 | |||
Functionality delivered in: https://github.com/dronecore/DroneCore/pull/139 |
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 am assuming that this API still doesn't need to be documented!
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.
Right.
en/contributing/plugins.md
Outdated
@@ -151,15 +151,15 @@ plugin **CMakeLists.txt** file (you can add multiple files). | |||
|
|||
The example plugin adds the **example_impl_test.cpp** unit test as shown below: | |||
|
|||
```cmake | |||
```cmake |
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.
@julianoes What do you have to update in CMakeLists.txt or other files to add unit tests for a)core b) core plugins c)external plugins (the text here is no longer correct).
For core plugins it looks like maybe you add to this one: https://github.com/dronecore/DroneCore/blob/develop/cmake/unit_tests.cmake#L23
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.
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.
Same way for external plugins? I only ask because if so, this is missing our unit test: https://github.com/dronecore/DroneCore/blob/develop/external_example/plugins/example/CMakeLists.txt
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.
Hi @julianoes
OK, so I updated the docs here. Can you please check them.
My concerns:
- This uses external example, so I am assuming that external example unit tests are added in the same way as normal plugins?
- The cmake scripts actually link unit_test_runner against the mission module here. Doesn't the runner have to link against all modules it uses? I would have expected this to also be specified in the cmakelists of each plugin.
OK, as external SDK testing broken I can't test it. I have created #102 to track the further work that will need to be done and will merge this PR so that the useful work that is complete is included. |
@julianoes This is WIP. I need some help from you (see inline queries) on how to run tests, in particular the external tests.
What this is waiting on: