Skip to content

Add isolated gmock test#4

Merged
wjwwood merged 7 commits intoros2:masterfrom
pbaughman:add_isolated_gmock_test
Jun 13, 2019
Merged

Add isolated gmock test#4
wjwwood merged 7 commits intoros2:masterfrom
pbaughman:add_isolated_gmock_test

Conversation

@pbaughman
Copy link
Copy Markdown
Contributor

@pbaughman pbaughman commented Jun 10, 2019 •
edited
Loading

While trying to isolated the rosbag2 tests, I noticed two things:

  1. Rosbag2 uses ament_add_gmock, not ament_add_gtest
  2. The tests for the domain_coordinator itself can be interfered with by tests using the domain_coordinator (if running in parallel)

Fix 1 by adding ament_cmake_ros_isolated_gmock (depends on this PR)

Fix 2 by having the tests for the domain coordinator run using a different set of ports.

Pete Baughman added 2 commits June 10, 2019 13:08
Add isolated version of ament_add_gmock
7ef8c96 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
Prevent add_ros_isolated_*_tests from interfering with domain_coordin…
df25e23 
…ator self-test

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@pbaughman pbaughman mentioned this pull request Jun 11, 2019
Closed
@dirk-thomas dirk-thomas mentioned this pull request Jun 11, 2019
Merged
@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jun 11, 2019

See comment on #3 (comment).

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Jun 11, 2019 •
edited
Loading

@dirk-thomas Yeah - if you're OK with moving everything into ament_cmake_ros, I'm happy to do it.

Is the assumption that if you're using ament_cmake_ros, you already deep enough into the dependency graph that it's safe to add both ament_cmake_pytest and ament_cmake_gtest even if you only plan on using one of them?

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jun 11, 2019

Is the assumption that if you're using ament_cmake_ros, you already deep enough into the dependency graph that it's safe to add both ament_cmake_pytest and ament_cmake_gtest even if you only plan on using one of them

Yeah, that would be my thinking. @wjwwood Since you merged this what do you think about reintegrating these into ament_cmake_ros?

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jun 11, 2019

That’s fine with me.

Pete Baughman added 2 commits June 11, 2019 13:48
Move isolated test functions to ament_cmake_ros
e1bf0e1 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
Move ament_cmake_ros_isolated_gmock into ament_cmake_ros too
4352af2 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@pbaughman pbaughman force-pushed the add_isolated_gmock_test branch from fba2d2d to 4352af2 Compare June 11, 2019 21:23
@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Jun 11, 2019

@dirk-thomas Ok - I've mashed everything into ament_cmake_ros now, too. Tested with isolated tests in rcl (gtest) rclpy (pytest) and rosbag2 (gmock)

The dependency graph of ament_cmake_ros used to look like this:

ament_flake8                            +  *       *     ***       *    ..
ament_package                            +* ....... ....    .......... ...
ament_cmake_core                          + ******* ****    ****.***** **.
ament_pep257                               +       *     ***        *   ..
ament_cmake_export_definitions              +               *   .        .
ament_cmake_export_include_directories       +              *   .        .
ament_cmake_export_libraries                  +      *      *   .        .
ament_cmake_export_link_flags                  +            *   .        .
ament_cmake_include_directories                 +     *     .   .        .
ament_cmake_libraries                            +  * *     *   .       ..
ament_cmake_python                                +    *    *   .        .
ament_copyright                                    +    ****   * .....*...
ament_cmake_export_dependencies                     +       *   .       *.
ament_cmake_export_interfaces                        +      *   .        .
ament_cmake_target_dependencies                       +     *   .        .
ament_cmake_test                                       +    ****.***** *..
ament_cppcheck                                          +        *      ..
ament_cpplint                                            +        *     ..
ament_lint_cmake                                          +  * . ..... ...
ament_xmllint                                              +         *  ..
ament_cmake                                                 +   *        *
ament_cmake_lint_cmake                                       + * ***** **.
ament_lint_auto                                               +          *
ament_cmake_copyright                                          + ***** **.
uncrustify_vendor                                               +     *...
ament_cmake_cppcheck                                             +      *.
ament_cmake_cpplint                                               +     *.
ament_cmake_flake8                                                 +    *.
ament_cmake_pep257                                                  +   *.
ament_cmake_xmllint                                                  +  *.
ament_uncrustify                                                      +*..
ament_cmake_uncrustify                                                 +*.
ament_lint_common                                                       +*
ament_cmake_ros                                                          +

After this PR it will look like this:

ament_flake8                            +   *        *     ****          *    ..
ament_package                            + *  ....... ....     ............. ...
gtest_vendor                              +  *                  *    .         .
ament_cmake_core                           +  ******* ****     *******.***** **.
ament_pep257                                +        *     ****           *   ..
gmock_vendor                                 +                       *         .
ament_cmake_export_definitions                +                *      .        .
ament_cmake_export_include_directories         +               *      .        .
ament_cmake_export_libraries                    +      *       *      .        .
ament_cmake_export_link_flags                    +             *      .        .
ament_cmake_include_directories                   +     *      .      .        .
ament_cmake_libraries                              +  * *      *      .       ..
ament_cmake_python                                  +    *     *      .        .
ament_copyright                                      +    *****     *  .....*...
ament_cmake_export_dependencies                       +        *      .       *.
ament_cmake_export_interfaces                          +       *      .        .
ament_cmake_target_dependencies                         +      *      .        .
ament_cmake_test                                         +     *******.***** *..
ament_cppcheck                                            +            *      ..
ament_cpplint                                              +            *     ..
ament_lint_cmake                                            +    *  .  ..... ...
ament_xmllint                                                +             *  ..
domain_coordinator                                            +                *
ament_cmake                                                    +      *        *
ament_cmake_gtest                                               +    *         *
ament_cmake_lint_cmake                                           +  *  ***** **.
ament_cmake_pytest                                                +            *
ament_lint_auto                                                    +           *
ament_cmake_copyright                                               +  ***** **.
ament_cmake_gmock                                                    +         *
uncrustify_vendor                                                     +     *...
ament_cmake_cppcheck                                                   +      *.
ament_cmake_cpplint                                                     +     *.
ament_cmake_flake8                                                       +    *.
ament_cmake_pep257                                                        +   *.
ament_cmake_xmllint                                                        +  *.
ament_uncrustify                                                            +*..
ament_cmake_uncrustify                                                       +*.
ament_lint_common                                                             +*
ament_cmake_ros                                                                +

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Jun 13, 2019

@wjwwood I think this is good to go

Remove files that got left behind when isolated_gmock moved to ament_…
ddf4957 
…cmake_ros

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jun 13, 2019

Quick CI up to ament_cmake_ros:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jun 13, 2019

Looks like there are some linting issues that need to be resolved.

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Jun 13, 2019

sigh on it!

Pete Baughman added 2 commits June 13, 2019 16:13
Fix lint errors
9b05c52 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
Fix python flake8
0e13423 
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jun 13, 2019 •
edited
Loading

Cool, new CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood wjwwood merged commit ea3bde3 into ros2:master Jun 13, 2019
@pbaughman pbaughman deleted the add_isolated_gmock_test branch June 14, 2019 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wjwwood wjwwood wjwwood approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pbaughman @dirk-thomas @wjwwood