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

Add isolated gmock test #4

Merged
merged 7 commits into from
Jun 13, 2019
Merged

Conversation

pbaughman
Copy link
Contributor

@pbaughman pbaughman commented Jun 10, 2019

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
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
…ator self-test

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

See comment on #3 (comment).

@pbaughman
Copy link
Contributor Author

pbaughman commented Jun 11, 2019

@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
Member

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
Member

wjwwood commented Jun 11, 2019

That’s fine with me.

Pete Baughman added 2 commits June 11, 2019 13:48
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@pbaughman
Copy link
Contributor Author

@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
Contributor Author

@wjwwood I think this is good to go

…cmake_ros

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@wjwwood
Copy link
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
Member

wjwwood commented Jun 13, 2019

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

@pbaughman
Copy link
Contributor Author

sigh on it!

Pete Baughman added 2 commits June 13, 2019 16:13
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@wjwwood
Copy link
Member

wjwwood commented Jun 13, 2019

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants