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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ament_cmake_pytest package #116

Merged
merged 3 commits into from
Nov 14, 2017
Merged

add ament_cmake_pytest package #116

merged 3 commits into from
Nov 14, 2017

Conversation

dirk-thomas
Copy link
Contributor

Connect to #115.

See the "nice" test grouping 馃槈

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

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Nov 14, 2017
@dirk-thomas dirk-thomas self-assigned this Nov 14, 2017
@mikaelarguedas
Copy link
Contributor

Regarding the test grouping I'm a bit surprised to see (e.g. for rclpy) that the tests in rclpy are not grouped under rclpy but independent:
rclpy.src.ros2.rclpy.rclpy.test.<TEST_NAME>. Is that expected?

Also I have the same issue as before of not being able to display the dontent of these folders: http://ci.ros2.org/job/ci_linux/3515/testReport/rclpy.src.ros2.rclpy.rclpy.test/. Did anybody succeed to display them ?

Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm, I took for granted the logic that was copied for other packages and focused on the new code (has_pytest...).

If I read this correctly, coverage will not be ran by default compared to the pure python packages (which is the right behavior IMO), maybe coverage should be made optional so that we can run it if needed?

# :type QUIET: option
# :param PYTHON_EXECUTABLE: absolute path to the Python interpreter to be used,
# default to the CMake variable with the same name returned by
# FindPythonInterp
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be wrapped ? seems more readable if it was on the line above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't fit within 80 characters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, in that case, is it possible to wrap the parameter description just above (QUIET) that is much longer than this one ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in dcfedfb.

@dirk-thomas dirk-thomas merged commit f8469fd into master Nov 14, 2017
@dirk-thomas dirk-thomas deleted the ament_cmake_pytest branch November 14, 2017 01:34
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Nov 14, 2017
@mikaelarguedas
Copy link
Contributor

@dirk-thomas can you comment on the questions from #116 (comment) please? I'm not sure if this is expected behavior and how we can mitigate it if it's not. It's pretty inconvenient to not be able to display the test results and need to be resolved otherwise we'll have to revert these PR until we figure it out.

@dirk-thomas
Copy link
Contributor Author

dirk-thomas commented Nov 14, 2017

Regarding the test grouping I'm a bit surprised to see (e.g. for rclpy) that the tests in rclpy are not grouped under rclpy but independent:
rclpy.src.ros2.rclpy.rclpy.test.<TEST_NAME>. Is that expected?

I would guess that it is related to the custom working directory (https://github.com/ros2/rclpy/blob/309942f48c6543d52ae22b61b98b6bc34759238d/rclpy/CMakeLists.txt#L112). That results in a long relative path for each test. I assume the working directory is set to allow loading the Python extension library (in the build space) from a different location than the Python modules (in the source space). I don't know if there is a way to get rid of the working directory while keeping it working using a different approach.

Also I have the same issue as before of not being able to display the dontent of these folders: http://ci.ros2.org/job/ci_linux/3515/testReport/rclpy.src.ros2.rclpy.rclpy.test/. Did anybody succeed to display them ?

I would assume that the problem is related to the previous question. Jenkins might not be able to resolve the weird relative path and "hang" itself during that process.

"-m" "pytest"
"${path}"
# store last failed tests
"-o" "cache_dir=${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_pytest/${testname}/.cache"
Copy link
Contributor

Choose a reason for hiding this comment

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

On Xenial I'm seeing an error with this option when testing rclpy. I installed pytest from apt python3-pytest.

1: usage: pytest.py [options] [file_or_dir] [file_or_dir] [...]
1: pytest.py: error: unrecognized arguments: -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache

Full output

test 1
    Start 1: rclpytests

1: Test command: /usr/bin/python3 "-u" "/workspace/ros2_ws/install_isolated/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/
workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml" "--output-file" "/workspace/ros2_ws/build_isolated/rclpy/
ament_cmake_pytest/rclpytests.txt" "--append-env" "AMENT_PREFIX_PATH=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_index" "LD_LIBRA
RY_PATH=/workspace/ros2_ws/build_isolated/rclpy" "--command" "/usr/bin/python3" "-u" "-m" "pytest" "/workspace/ros2_ws/src/ros2/rclpy/rcl
py/test" "-o" "cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache" "--junit-xml=/workspace/ros2_ws/bu
ild_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml" "--junit-prefix=rclpy"
1: Test timeout computed to be: 90
1: -- run_test.py: extra environment variables to append:
1:  - AMENT_PREFIX_PATH+=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_index
1:  - LD_LIBRARY_PATH+=/workspace/ros2_ws/build_isolated/rclpy
1: -- run_test.py: invoking following command in '/workspace/ros2_ws/build_isolated/rclpy':
1:  - /usr/bin/python3 -u -m pytest /workspace/ros2_ws/src/ros2/rclpy/rclpy/test -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ame
nt_cmake_pytest/rclpytests/.cache --junit-xml=/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml --junit-pre
fix=rclpy
1: usage: pytest.py [options] [file_or_dir] [file_or_dir] [...]
1: pytest.py: error: unrecognized arguments: -o cache_dir=/workspace/ros2_ws/build_isolated/rclpy/ament_cmake_pytest/rclpytests/.cache
1:   inifile: None
1:   rootdir: /workspace/ros2_ws
1: -- run_test.py: return code 2
1: -- run_test.py: generate result file '/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml' with failed te$
t
1: -- run_test.py: verify result file '/workspace/ros2_ws/build_isolated/rclpy/test_results/rclpy/rclpytests.xunit.xml'
1/8 Test #1: rclpytests .......................***Failed    0.51 sec

Where is -o cache_dir= supposed be handled? I haven't found any references to cache_dir in cpython, unittest, or nose. I see some references to cache_dir in an ini in pytest here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like cache_dir was added in version 3.2, but the version in apt is 2.8.7-4. Installing via pip fixed the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

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