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

Nacho/fix xunit report #8

Merged
merged 2 commits into from Jan 19, 2024
Merged

Nacho/fix xunit report #8

merged 2 commits into from Jan 19, 2024

Conversation

nachovizzo
Copy link
Collaborator

Fixes #6, and Timple#6

Use black Python API to fetch the sources that are being checked

@Ryanf55
Copy link

Ryanf55 commented Dec 16, 2023

This caused test failures for me for previously passing tests.

$ colcon test --packages-select ardupilot_sitl --event-handlers=console_cohesion+


1: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml" "--package-name" "ardupilot_sitl" "--output-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/ament_black/black.txt" "--command" "/opt/ros/humble/bin/ament_black" "--xunit-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml"
1: Test timeout computed to be: 60
1: -- run_test.py: invoking following command in '/home/ryan/Dev/ros2_ws/src/ardupilot/Tools/ros2/ardupilot_sitl':
1:  - /opt/ros/humble/bin/ament_black --xunit-file /home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml
1: Traceback (most recent call last):
1:   File "/opt/ros/humble/bin/ament_black", line 33, in <module>
1:     sys.exit(load_entry_point('ament-black==0.2.3', 'console_scripts', 'ament_black')())
1:   File "/home/ryan/Dev/ros2_ws/install/ament_black/lib/python3.10/site-packages/ament_black/main.py", line 79, in main
1:     sources = get_sources(
1: TypeError: get_sources() missing required keyword-only argument 'root'
1: -- run_test.py: return code 1
1: -- run_test.py: generate result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml' with failed test
1: -- run_test.py: verify result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml'
1/5 Test #1: black ............................***Failed    0.12 sec

@nachovizzo
Copy link
Collaborator Author

This caused test failures for me for previously passing tests.

$ colcon test --packages-select ardupilot_sitl --event-handlers=console_cohesion+


1: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml" "--package-name" "ardupilot_sitl" "--output-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/ament_black/black.txt" "--command" "/opt/ros/humble/bin/ament_black" "--xunit-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml"
1: Test timeout computed to be: 60
1: -- run_test.py: invoking following command in '/home/ryan/Dev/ros2_ws/src/ardupilot/Tools/ros2/ardupilot_sitl':
1:  - /opt/ros/humble/bin/ament_black --xunit-file /home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml
1: Traceback (most recent call last):
1:   File "/opt/ros/humble/bin/ament_black", line 33, in <module>
1:     sys.exit(load_entry_point('ament-black==0.2.3', 'console_scripts', 'ament_black')())
1:   File "/home/ryan/Dev/ros2_ws/install/ament_black/lib/python3.10/site-packages/ament_black/main.py", line 79, in main
1:     sources = get_sources(
1: TypeError: get_sources() missing required keyword-only argument 'root'
1: -- run_test.py: return code 1
1: -- run_test.py: generate result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml' with failed test
1: -- run_test.py: verify result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml'
1/5 Test #1: black ............................***Failed    0.12 sec

This means it's good for you right :D ?

@Ryanf55
Copy link

Ryanf55 commented Jan 12, 2024

This caused test failures for me for previously passing tests.

$ colcon test --packages-select ardupilot_sitl --event-handlers=console_cohesion+


1: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml" "--package-name" "ardupilot_sitl" "--output-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/ament_black/black.txt" "--command" "/opt/ros/humble/bin/ament_black" "--xunit-file" "/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml"
1: Test timeout computed to be: 60
1: -- run_test.py: invoking following command in '/home/ryan/Dev/ros2_ws/src/ardupilot/Tools/ros2/ardupilot_sitl':
1:  - /opt/ros/humble/bin/ament_black --xunit-file /home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml
1: Traceback (most recent call last):
1:   File "/opt/ros/humble/bin/ament_black", line 33, in <module>
1:     sys.exit(load_entry_point('ament-black==0.2.3', 'console_scripts', 'ament_black')())
1:   File "/home/ryan/Dev/ros2_ws/install/ament_black/lib/python3.10/site-packages/ament_black/main.py", line 79, in main
1:     sources = get_sources(
1: TypeError: get_sources() missing required keyword-only argument 'root'
1: -- run_test.py: return code 1
1: -- run_test.py: generate result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml' with failed test
1: -- run_test.py: verify result file '/home/ryan/Dev/ros2_ws/build/ardupilot_sitl/test_results/ardupilot_sitl/black.xunit.xml'
1/5 Test #1: black ............................***Failed    0.12 sec

This means it's good for you right :D ?

Well, it's running, but it's failing to run the test code. So no, I wouldn't recommend merging it in the current state. Let me know if there's anything I can help with.

@nachovizzo
Copy link
Collaborator Author

@Ryanf55 could you confirm the version of black. This also happened to us in our CI at some point. For now, black is tightly coupled to whatever black version is distributed with ubuntu, in humble/iron/rolling this would be:

black/now 21.12b0-1 all 

You can sudo apt install black and try. This is enforced in the setup.py, but you can do pip install black --upgrade later on and override it.

@Ryanf55
Copy link

Ryanf55 commented Jan 15, 2024

@Ryanf55 could you confirm the version of black. This also happened to us in our CI at some point. For now, black is tightly coupled to whatever black version is distributed with ubuntu, in humble/iron/rolling this would be:

black/now 21.12b0-1 all 

You can sudo apt install black and try. This is enforced in the setup.py, but you can do pip install black --upgrade later on and override it.

Yep, I'll let you know by end of day.

@nachovizzo
Copy link
Collaborator Author

@Ryanf55 let me know if it works with that version of black so I can move forward with the releasing process ;)

@Ryanf55
Copy link

Ryanf55 commented Jan 17, 2024

@Ryanf55 let me know if it works with that version of black so I can move forward with the releasing process ;)

Will do. If I don't reply tonight, ship it 🚀

@Ryanf55
Copy link

Ryanf55 commented Jan 18, 2024

sudo apt install black

OS environment

Here's my OS environment without your PR changes:

$  sudo apt install black
[sudo] password for ryan: 
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
black is already the newest version (21.12b0-1).
$ python3 -m pip list | grep black
ament-black                          0.2.3
black                                23.11.0

Creating a test package

$ ros2 pkg create my_black_test --build-type ament_cmake --dependencies ament_cmake_black

Modify package.xml and CMakeLists.txt as originally reported

# find dependencies
find_package(ament_cmake REQUIRED)
find_package(ament_cmake_black REQUIRED)

if(BUILD_TESTING)
  # Add linters.
  find_package(ament_lint_auto REQUIRED)
  ament_lint_auto_find_test_dependencies()
endif()
  <test_depend>ament_lint_auto</test_depend>
  <test_depend>ament_cmake_black</test_depend>
$ git clone --branch nacho/fix_xunit_report git@github.com:botsandus/ament_black.git
touch my_black_test/myfile.py


>>> Add some unformatted content

Running Tests

$ colcon build
$ colcon build
Starting >>> ament_black
--- stderr: ament_black                   
/home/ryan/.local/lib/python3.10/site-packages/setuptools/_distutils/cmd.py:66: SetuptoolsDeprecationWarning: setup.py install is deprecated.
!!

        ********************************************************************************
        Please avoid running ``setup.py`` directly.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
        ********************************************************************************

!!
  self.initialize_options()
---
Finished <<< ament_black [0.84s]
Starting >>> ament_cmake_black
Finished <<< ament_cmake_black [0.16s]                  
Starting >>> my_black_test
Finished <<< my_black_test [0.29s]                

Summary: 3 packages finished [1.91s]
  1 package had stderr output: ament_black
$ source install/setup.bash
$ colcon test --packages-select my_black_test --event-handlers=console_cohesion+
Starting >>> my_black_test
--- output: my_black_test                   
UpdateCTestConfiguration  from :/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/CTestConfiguration.ini
Parse Config file:/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/CTestConfiguration.ini
  Site: B650-970
  Build name: (empty)
Add coverage exclude regular expressions.
Create new tag: 20240118-0505 - Experimental
UpdateCTestConfiguration  from :/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/CTestConfiguration.ini
Parse Config file:/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/CTestConfiguration.ini
Test project /home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 1
   Start 1: black

1: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/test_results/my_black_test/black.xunit.xml" "--package-name" "my_black_test" "--output-file" "/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/ament_black/black.txt" "--command" "/opt/ros/humble/bin/ament_black" "--xunit-file" "/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/test_results/my_black_test/black.xunit.xml"
1: Test timeout computed to be: 60
1: -- run_test.py: invoking following command in '/home/ryan/Dev/ros2_ws/src/black_test/my_black_test':
1:  - /opt/ros/humble/bin/ament_black --xunit-file /home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/test_results/my_black_test/black.xunit.xml
1: Traceback (most recent call last):
1:   File "/opt/ros/humble/bin/ament_black", line 33, in <module>
1:     sys.exit(load_entry_point('ament-black==0.2.3', 'console_scripts', 'ament_black')())
1:   File "/home/ryan/Dev/ros2_ws/src/black_test/install/ament_black/lib/python3.10/site-packages/ament_black/main.py", line 79, in main
1:     sources = get_sources(
1: TypeError: get_sources() missing required keyword-only argument 'root'
1: -- run_test.py: return code 1
1: -- run_test.py: generate result file '/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/test_results/my_black_test/black.xunit.xml' with failed test
1: -- run_test.py: verify result file '/home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/test_results/my_black_test/black.xunit.xml'
1/1 Test #1: black ............................***Failed    0.12 sec

0% tests passed, 1 tests failed out of 1

Label Time Summary:
black     =   0.12 sec*proc (1 test)
linter    =   0.12 sec*proc (1 test)

Total Test time (real) =   0.12 sec

The following tests FAILED:
         1 - black (Failed)
Errors while running CTest
Output from these tests are in: /home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
---
--- stderr: my_black_test
Errors while running CTest
Output from these tests are in: /home/ryan/Dev/ros2_ws/src/black_test/build/my_black_test/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
---
Finished <<< my_black_test [0.20s]      [ with test failures ]

Summary: 1 package finished [0.73s]
 1 package had stderr output: my_black_test
 1 package had test failures: my_black_test

More info

$ black --version
black, 23.11.0 (compiled: yes)
Python (CPython) 3.10.12

Attempting to override the OS black with newer version:

$ python3 -m pip install --upgrade black
Defaulting to user installation because normal site-packages is not writeable
Requirement already satisfied: black in /home/ryan/.local/lib/python3.10/site-packages (23.11.0)
Collecting black
  Downloading black-23.12.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (68 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 69.0/69.0 kB 2.3 MB/s eta 0:00:00
Requirement already satisfied: click>=8.0.0 in /home/ryan/.local/lib/python3.10/site-packages (from black) (8.1.7)
Requirement already satisfied: mypy-extensions>=0.4.3 in /usr/lib/python3/dist-packages (from black) (0.4.3)
Requirement already satisfied: packaging>=22.0 in /home/ryan/.local/lib/python3.10/site-packages (from black) (23.2)
Requirement already satisfied: pathspec>=0.9.0 in /home/ryan/.local/lib/python3.10/site-packages (from black) (0.11.2)
Requirement already satisfied: platformdirs>=2 in /home/ryan/.local/lib/python3.10/site-packages (from black) (3.10.0)
Requirement already satisfied: tomli>=1.1.0 in /usr/lib/python3/dist-packages (from black) (1.2.2)
Requirement already satisfied: typing-extensions>=4.0.1 in /home/ryan/.local/lib/python3.10/site-packages (from black) (4.7.1)
Downloading black-23.12.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.7 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 3.1 MB/s eta 0:00:00
Installing collected packages: black
  Attempting uninstall: black
    Found existing installation: black 23.11.0
    Uninstalling black-23.11.0:
      Successfully uninstalled black-23.11.0
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
ament-black 0.2.3 requires black==21.12b0, but you have black 23.12.1 which is incompatible.
ament-black 0.2.3 requires uvloop==0.17.0, but you have uvloop 0.16.0 which is incompatible.
Successfully installed black-23.12.1

And, retest, same failure.

$ python3 -m pip show black
Name: black
Version: 23.12.1
Summary: The uncompromising code formatter.
Home-page: 
Author: 
Author-email: Łukasz Langa <lukasz@langa.pl>
License: MIT
Location: /home/ryan/.local/lib/python3.10/site-packages
Requires: click, mypy-extensions, packaging, pathspec, platformdirs, tomli, typing-extensions
Required-by: ament-black

Sorry, I honestly don't know how to get your PR to work. If you need it reproduced in a dockerfile with the above, I can do that.

@Ryanf55
Copy link

Ryanf55 commented Jan 18, 2024

Hey look! AN ABI break that's affecting my OS.
psf/black@6310a40#diff-d23a6917d7262ca9815093bffb7148299e516575251959ed9b405b648e76aa29L618

Sadly, I broke my OS: https://stackoverflow.com/questions/71673404/importerror-cannot-import-name-unicodefun-from-click

$ black --version
Traceback (most recent call last):
  File "/usr/bin/black", line 33, in <module>
    sys.exit(load_entry_point('black==21.10b0', 'console_scripts', 'black')())
  File "/usr/lib/python3/dist-packages/black/__init__.py", line 1372, in patched_main
    patch_click()
  File "/usr/lib/python3/dist-packages/black/__init__.py", line 1358, in patch_click
    from click import _unicodefun
ImportError: cannot import name '_unicodefun' from 'click' (/home/ryan/.local/lib/python3.10/site-packages/click/__init__.py)

@Ryanf55
Copy link

Ryanf55 commented Jan 18, 2024

If I'm understanding this correctly, Ubuntu 22.04 installs black which relies on click, any version.

$ apt-cache depends black
black
  Depends: python3-pkg-resources
  Depends: python3-click
  Depends: python3-mypy-extensions
  Depends: python3-pathspec
  Depends: python3-platformdirs
  Depends: python3-tomli
  Depends: python3-typing-extensions
  Depends: <python3:any>
    python3:i386
    python3
  Suggests: python-black-doc

black was importing a "private" function from the click library, which broke, because it wasn't pinned the click authors wanted to change their implementation.
psf/black#2964

Now, no one has filed a bug on Ubuntu, so I did:
https://bugs.launchpad.net/ubuntu/+source/black/+bug/2049717

And, It's still broken.

Typically, we rely on rosdep to installl our dependencies. rosdep doesn't have support for version pinning, so honestly I'm a bit stumped on how to solve this.

@nachovizzo
Copy link
Collaborator Author

@Ryanf55 I'm sorry you struggled this much! And happy with all the detailed information. I already fought with such monsters when dealing with Python packages and it's horrible
image

What I would probably do in your case to uninstall all black and click versions. Believe it or not, just run (a few times) pip uninstall black to do so. pip will uninstall the first one first, and the second invocation the 2nd one, and so on.

We DO NOT have/allow pip for this same reason. One can install a newer version of a package and break some other components, this is why we have been using happily black with no issues so far.

Let me know if you can solve the black issue otherwise, I might move forward with the release. Once more, sorry for all the trouble!

@nachovizzo
Copy link
Collaborator Author

Mergin this as it's working with the ubuntu-supported black version

@nachovizzo nachovizzo merged commit 90f0c3e into main Jan 19, 2024
8 checks passed
@nachovizzo nachovizzo deleted the nacho/fix_xunit_report branch January 19, 2024 15:41
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.

ament_cmake_black not listing checked files in XUNIT report during successful run
2 participants