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

Pep 257 test fails with internal error on rolling #795

Closed
Flova opened this issue Feb 13, 2023 · 4 comments
Closed

Pep 257 test fails with internal error on rolling #795

Flova opened this issue Feb 13, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@Flova
Copy link

Flova commented Feb 13, 2023

Description

I have a standard ros2 CI setup for this repo. All tests passed until recently. But the rolling CI failed after a README.md change. The nearly identical humble one is fine. All checks pass locally.

I get the following error in my actions log:


  test/test_copyright.py .                                                 [  6%]
  test/test_flake8.py .                                                    [ 12%]
  test/test_ipm.py ...........                                             [ 81%]
  test/test_pep257.py F                                                    [ 87%]
  test/test_utils.py ..                                                    [100%]
  
  =================================== FAILURES ===================================
  _________________________________ test_pep257 __________________________________
  test/test_pep257.py:22: in test_pep257
      rc = main(argv=['.', 'test'])
  /opt/ros/rolling/lib/python3.10/site-packages/ament_pep257/main.py:125: in main
      report = generate_pep257_report(args.paths, excludes, args.ignore, args.select,
  /opt/ros/rolling/lib/python3.10/site-packages/ament_pep257/main.py:190: in generate_pep257_report
      for filename, checked_codes, ignore_decorators in files_to_check:
  E   ValueError: too many values to unpack (expected 3)
  ----- generated xml file: /__w/ipm/ipm/ros_ws/build/ipm_library/pytest.xml -----
  =========================== short test summary info ============================
  FAILED test/test_pep257.py::test_pep257 - ValueError: too many values to unpack (expected 3)

Relevant part of the workflow configurations (same for both humble and rolling):

  build:
    # The type of runner that the job will run on
    runs-on: ubuntu-latest
    container:
      image: ubuntu:jammy

    # Steps represent a sequence of tasks that will be executed as part of the job
    steps:
      - uses: ros-tooling/setup-ros@v0.5
        with:
          use-ros2-testing: true
      - uses: ros-tooling/action-ros-ci@v0.2
        with:
          target-ros2-distro: rolling

Expected Behavior

The pep257 check should pass under rolling as it does under humble.

Actual Behavior

It fails with an internal ValueError

To Reproduce

  1. Create a repo with a default configuration like e.g. the one linked above
  2. Run the workflows for humble and rolling

System (please complete the following information)

  • OS: Ubuntu 22.04
  • ROS 2 Distro: rolling
@Flova Flova added the bug Something isn't working label Feb 13, 2023
@christophebedard
Copy link
Member

christophebedard commented Feb 13, 2023

The issue is in ament_pep257 itself. It was fixed: ament/ament_lint#428. However, they haven't created a new release yet, so the only way to get the fix would be to build ament_pep257 from source. This happens from time to time, unfortunately.

This works on Humble because the fix was backported to humble and then released: https://github.com/ament/ament_lint/commits/humble

@christophebedard
Copy link
Member

I personally have jobs that both:

  1. Build my package from source using ROS 2 binaries
  2. Build my package + ROS 2 from source

(1) can fail in this situation, but (2) usually doesn't. You can build other packages from source using the vcs-repo-file-url option.

@christophebedard
Copy link
Member

christophebedard commented Feb 14, 2023

they haven't created a new release yet

They have now: ros/rosdistro#36153. The fix should therefore be available in the next Rolling sync, possibly in a few weeks. In the meantime, it should soon be available in the prerelease/testing APT repo (which you can use with a setup-ros option).

@christophebedard
Copy link
Member

Since there's no actual issue with action-ros-ci, I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants