Run tests in current binary directory, not global source directory by rotu · Pull Request #206 · ament/ament_cmake

rotu · 2019-10-22T20:31:16Z

For consistency with CMake add_test, run tests in the project binary directory, not the top-level CMakeLists.txt

ament_cmake_test/cmake/ament_add_test.cmake

dirk-thomas · 2019-10-23T04:34:22Z

CI builds:

Linux
Linux-aarch64
macOS
Windows

Please also sign-off the commits to pass DCO.

rotu · 2019-10-23T04:36:10Z

Those CI failures aren't as bad as I figured. Want me to go ahead with CMAKE_CURRENT_BINARY_DIR or stick with the "safer" source dir?

dirk-thomas · 2019-10-23T04:37:52Z

Want me to go ahead with CMAKE_CURRENT_BINARY_DIR

Yes, because that is what matches CMake standard behavior.

Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io>

This removed the dependency on pytest and prevents an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

More inline with how other packages do it and avert an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

Prevent anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

Avert breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

dirk-thomas · 2019-10-23T18:05:29Z

Want me to go ahead with CMAKE_CURRENT_BINARY_DIR

Yes, because that is what matches CMake standard behavior.

I am sorry but I misread the value. I thought the goal was to change it to CMAKE_CURRENT_SOURCE_DIR. I would be ok with that since it only affect the use case where the package is built in a sub project.

I don't think changing the value to CMAKE_CURRENT_BINARY_DIR now is feasible since we have passed the API freeze for the upcoming release and this is certainly a breaking change.

rotu · 2019-10-23T18:10:17Z

...That's why I asked 👎

This PR can hold off until F. The supporting PRs can be safely merged now anyway.

dirk-thomas · 2019-10-23T18:12:19Z

This PR can hold off until F.

How about creating a separate PR to change it to CMAKE_CURRENT_SOURCE_DIR which could be merged right away? Independent of this PR later being accepted for F.

rotu · 2019-10-23T18:19:10Z

How about creating a separate PR to change it to CMAKE_CURRENT_SOURCE_DIR which could be merged right away? Independent of this PR later being accepted for F.

Now that I've done the supporting work for CMAKE_CURRENT_BINARY_DIR, I don't see the value in an intermediate PR. If you find it useful, go ahead.

dirk-thomas · 2019-10-23T18:20:05Z

I don't see the value in an intermediate PR.

Wouldn't that be necessary to build Eloquent using your sub project approach?

rotu · 2019-10-23T18:22:23Z

I'm happy to use this branch as-is with my Eloquent tree, even if it's not slated for Eloquent release

More inline with how other packages do it and avert an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

Avert breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

Prevent anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

dirk-thomas · 2019-11-22T17:30:46Z

With Eloquent being branched master is targeting F-turtle so this can be merged now. Thanks for the patch.

Linux
Linux-aarch64
macOS
Windows (unrelated flaky test)

) Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io>

) Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

…ctory (#206)" This reverts commit 4354d62.

…ctory (#206)" This reverts commit 4354d62. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* Add runner option to ament_add_test (#174) * ament_cmake allow speficiation of a different test runner - By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <pete.baughman@apex.ai> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * improve handling of encoding (#181) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * use deterministic order for updated env vars (#196) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Run tests in current binary directory, not global source directory (#206) Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * remove status attribute from result XML, add skipped tag instead (#218) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Declare AMENT_TEST_RESULTS_DIR as a PATH (#221) Signed-off-by: Dan Rose <dan@digilabs.io> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Generate xunit files valid for the junit10.xsd Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Revert "Run tests in current binary directory, not global source directory (#206)" This reverts commit 4354d62. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> Co-authored-by: Peter Baughman <dblue135@yahoo.com> Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Co-authored-by: Dan Rose <rotu@users.noreply.github.com> Co-authored-by: Jose Luis Rivero <jrivero@osrfoundation.org>

rotu force-pushed the patch-2 branch from 1e72c3a to d0925c1 Compare October 22, 2019 20:33

dirk-thomas reviewed Oct 22, 2019

View reviewed changes

ament_cmake_test/cmake/ament_add_test.cmake Outdated Show resolved Hide resolved

rotu changed the title ~~Run tests in project source directory, not global source directory~~ WIP: Run tests in project source directory, not global source directory Oct 23, 2019

Run tests in current binary directory, not global source directory

561eb58

Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io>

rotu force-pushed the patch-2 branch from baa766b to 561eb58 Compare October 23, 2019 16:56

rotu changed the title ~~WIP: Run tests in project source directory, not global source directory~~ WIP: Run tests in current binary directory, not global source directory Oct 23, 2019

rotu added a commit to RoverRobotics-forks/launch that referenced this pull request Oct 23, 2019

Use ament_cmake_copyright

e1c2374

This removed the dependency on pytest and prevents an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

rotu added a commit to RoverRobotics-forks/launch that referenced this pull request Oct 23, 2019

Use ament_cmake_copyright

a9d07f4

More inline with how other packages do it and avert an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

rotu mentioned this pull request Oct 23, 2019

Use ament_cmake_copyright ros2/launch#349

Merged

rotu added a commit to RoverRobotics-forks/rcutils that referenced this pull request Oct 23, 2019

Specify working directory for filesystem test

338d395

Prevent anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

rotu mentioned this pull request Oct 23, 2019

Specify working directory for filesystem test ros2/rcutils#185

Merged

rotu added a commit to RoverRobotics-forks/rcl that referenced this pull request Oct 23, 2019

Specify test working directory

97c35d1

Avert breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

rotu mentioned this pull request Oct 23, 2019

Specify test working directory ros2/rcl#529

Merged

rotu changed the title ~~WIP: Run tests in current binary directory, not global source directory~~ Run tests in current binary directory, not global source directory Oct 23, 2019

hidmic pushed a commit to ros2/launch that referenced this pull request Oct 23, 2019

Use ament_cmake_copyright (#349)

7ea6cc3

More inline with how other packages do it and avert an anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

dirk-thomas pushed a commit to ros2/rcl that referenced this pull request Oct 23, 2019

Specify test working directory (#529)

c173935

Avert breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

dirk-thomas pushed a commit to ros2/rcutils that referenced this pull request Oct 23, 2019

Specify working directory for filesystem test (#185)

c688c4a

Prevent anticipated breakage from ament/ament_cmake#206 Signed-off-by: Dan Rose <dan@digilabs.io>

wjwwood assigned dirk-thomas Nov 21, 2019

dirk-thomas merged commit 83e1b81 into ament:master Nov 22, 2019

dirk-thomas mentioned this pull request Nov 25, 2019

add note about default working directory for ament_add_test ros2/ros2_documentation#448

Merged

rotu deleted the patch-2 branch November 30, 2019 21:48

nuclearsandwich pushed a commit that referenced this pull request Mar 20, 2020

Run tests in current binary directory, not global source directory (#206

331a756

) Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <dan@digilabs.io>

nuclearsandwich mentioned this pull request Mar 23, 2020

[Dashing] Backport junit xml fixes for updated xunit plugin on ci.ros2.org #230

Merged

nuclearsandwich added a commit that referenced this pull request Apr 9, 2020

Revert "Run tests in current binary directory, not global source dire…

a7ffd1d

…ctory (#206)" This reverts commit 4354d62.

nuclearsandwich added a commit that referenced this pull request Apr 11, 2020

Revert "Run tests in current binary directory, not global source dire…

3ce9599

…ctory (#206)" This reverts commit 4354d62. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

icolwell-as mentioned this pull request Oct 20, 2022

Confusion over WORKING_DIRECTORY #406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run tests in current binary directory, not global source directory#206

Run tests in current binary directory, not global source directory#206
dirk-thomas merged 1 commit intoament:masterfrom
rotu:patch-2

rotu commented Oct 22, 2019 •

edited

Loading

Uh oh!

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019 •

edited

Loading

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rotu commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Oct 23, 2019

Uh oh!

rotu commented Oct 23, 2019

Uh oh!

dirk-thomas commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rotu commented Oct 22, 2019 •

edited

Loading

rotu commented Oct 23, 2019 •

edited

Loading