search

space-ros / docker

Docker images to facilitate Docker-based development.
21 stars 32 forks source link

Cobra analysis results are empty for all packages #103

Closed xfiderek closed 8 months ago

xfiderek commented 8 months ago

Summary

+build-testing procedure from spaceros Earthfile is invoked in CI to push results of tests and static code analysis. Currently, for all packages cobra-autosar.sarif results are empty. Check the latest artifact saved as part of CI. image

Possible fix

The most probable root cause is that cobra or some dependent executables are not found when colcon test command is invoked. The fix should be as simple adding RUN . install/setup.bash command before colcon test: https://github.com/space-ros/docker/blob/76cd412a7379dd5a5ec338b7b75659afbbf0c9a1/spaceros/Earthfile#L123-L126

That would be a short-term fix, as I think in long-run we should modify ament_cobra package to throw an error / exit code != 0 if cobra executable is not found and stop the tests if possible.

How did I get there

I slightly modified earthfile locally to get to the full build log and found out the following in example package logs (robot_state_publisher) File: /home/spaceros-user/spaceros/build/robot_state_publisher/ament_cobra/cobra-autosar.txt

-- run_test.py: invoking following command in '/home/spaceros-user/spaceros/src/robot_state_publisher':
 - /home/spaceros-user/spaceros/install/ament_cobra/bin/ament_cobra --xunit-file /home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/cobra-autosar.xunit.xml --sarif-file /home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/cobra-autosar.sarif --ruleset C++/autosar --include_dirs /home/spaceros-user/spaceros/src/robot_state_publisher/include  /home/spaceros-user/spaceros/install/tf2_ros/include/tf2_ros /home/spaceros-user/spaceros/install/rclcpp_components/include/rclcpp_components /home/spaceros-user/spaceros/install/rclcpp/include/rclcpp /home/spaceros-user/spaceros/install/sensor_msgs/include/sensor_msgs /home/spaceros-user/spaceros/install/geometry_msgs/include/geometry_msgs /home/spaceros-user/spaceros/install/std_msgs/include/std_msgs /home/spaceros-user/spaceros/install/rcl_interfaces/include/rcl_interfaces /home/spaceros-user/spaceros/install/builtin_interfaces/include/builtin_interfaces /home/spaceros-user/spaceros/install/kdl_parser/include/kdl_parser /home/spaceros-user/spaceros/install/urdf/include/urdf /usr/include/eigen3 /home/spaceros-user/spaceros/install/class_loader/include/class_loader /home/spaceros-user/spaceros/install/gtest_vendor/src/gtest_vendor/include --compile_cmds /home/spaceros-user/spaceros/build/robot_state_publisher/compile_commands.json

error: cannot open ~/.cobra : check tool installation
error: cannot open ~/.cobra : check tool installation
error: cannot open ~/.cobra : check tool installation
error: cannot open ~/.cobra : check tool installation
error: cannot open ~/.cobra : check tool installation
Warning: The include directories from the compile commands file will be used instead of the directories specified using --include_dirs
cobra: cannot find 'C++/autosar'

cobra: cannot find 'C++/autosar'

cobra: cannot find 'C++/autosar'

cobra: cannot find 'C++/autosar'

cobra: cannot find 'C++/autosar'

No problems found

-- run_test.py: return code 0
-- run_test.py: verify result file '/home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/cobra-autosar.xunit.xml'
Invalid XML in result file '/home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/cobra-autosar.xunit.xml' (even after trying to tidy it): not well-formed (invalid token): line 1, column 35

When i re-invoke colcon test from command line it works as expected and outputs correct cobra-autosar.sarif file with violations.

Additional notes

Some background information on how the process of generating SARIF files work https://github.com/space-ros/docker/blob/76cd412a7379dd5a5ec338b7b75659afbbf0c9a1/spaceros/Earthfile#L123-L130 After invoking +build-testing Earthly command:

  1. colcon test should run
  2. As part of colcon test, ament_cobra should output cobra-autosar.sarif file with results of analysis for every package that includes ament_lint_auto and ament_lint_common.
  3. Results for each package should be stored in ~/spaceros/build/PKG_NAME/test_results/PKG_NAME/cobra-autosar.sarif
  4. ros2 run process_sarif make_build_archive node crawls all sarif files in build directory, processes them and saves them to log/build_result_archives/build_results_* zip.

I think we should document it somewhere as I could not find the docs that describe this process.

ivanperez-keera commented 8 months ago

I'm a bit confused.

I just opened the spaceros and space_robot images, run colcon test in them, and the sarif files are there.

xfiderek commented 8 months ago

hi @ivanperez-keera. As I said above, cobra-autosar.sarif gets generated correctly after rerun of colcon test. However values for "rules" and "results" in artifact pushed via CI are empty for every package.

chunk of sarif file from artifact:

{
  "version": "2.1.0",
  "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.5",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "Cobra",
          "version": "4.1",
          "informationUri": "https://github.com/nimble-code/Cobra",
          "rules": []
        }
      },
      "artifacts": [
        {
          "location": {
            "uri": "src/robot_state_publisher.cpp",
            "uriBaseId": "/home/spaceros-user/spaceros/src/robot_state_publisher"
          }
        },
        ...
      "results": []
    }
  ]
}

chunk of sarif file after re-running colcon test:

"version": "2.1.0",
  "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.5",
  "runs": [
   {
    "tool": {
      "driver": {
        "name": "Cobra",
        "version": "3.9",
        "informationUri": "https://github.com/nimble-code/Cobra",
        "rules": [
            {
              "id": "R0",
              "fullDescription": {
                "text": "(Required) A project shall not contain unreachable code."
              },
              "messageStrings": {
                "default": {
                  "text": "lines 1080..1084"
                }
              }
            },
            {
              "id": "R1",
              "fullDescription": {
                "text": "(Required) A project shall not contain unused variables."
              },
              "messageStrings": {
                "default": {
                  "text": "lines 49..49"
                }
              }
            },
            {
              "id": "R2",
              "fullDescription": {
                "text": "(Required) A project shall not contain non-volatile POD (plain old data) variables having only one use."
              },
              "messageStrings": {
                "default": {
                  "text": "lines 87..87"
                }
              }
            },
            {
              "id": "R3",
              "fullDescription": {
                "text": "(Required) A project shall not contain unused type declarations."
              },
              "messageStrings": {
                "default": {
                  "text": "lines 331..331"
                }
              }
            },
            {
              "id": "R4",
              "fullDescription": {
                "text": "(Required) The value returned by a function having a non-void return type that is not an overloaded operator shall be used."
              },
              "messageStrings": {
                "default": {
                  "text": "lines 49..49"
                }
...
...
...
ivanperez-keera commented 8 months ago

When i re-invoke colcon test from command line it works as expected and outputs correct cobra-autosar.sarif file with violations.

My bad, I missed this comment the first time.

The fix should be as simple adding RUN . install/setup.bash command before colcon test: That would be a short-term fix, as I think in long-run we should modify ament_cobra package to throw an error / exit code != 0 if cobra executable is not found and stop the tests if possible.

I think both of these solutions are correct and address slightly different concerns. One addresses the issue that cobra analyses are empty when we initially run with +build-testing.

The other addresses the issue of how to detect problems with the execution of the cobra tests. This last one IMO should be a separate issue (filed against the ament_cobra repo).

xfiderek commented 8 months ago

My bad, I missed this comment the first time.

No worries, I think I could have written issue summary in a more concise manner :D.

The other addresses the issue of how to detect problems with the execution of the cobra tests. This last one IMO should be a separate issue (filed against the ament_cobra repo)

Ok, i will file the issue against the cobra repo!

ivanperez-keera commented 8 months ago

No worries, I think I could have written issue summary in a more concise manner :D.

Nah, you're good. You were just trying to give detail to help identify the problem and solution. I wish most people gave even close to that level of info when they file issues :smile:

The fix should be as simple adding RUN . install/setup.bash command before colcon test:

This seems like very low hanging fruit. Would you like to try to address this as part of this release (2024.01.0)?

xfiderek commented 8 months ago

Sure, no problem!

ivanperez-keera commented 8 months ago

Ok, then I'll assign it to you and add it to this milestone.

ivanperez-keera commented 8 months ago

When you are done with this issue, we should also consider whether that closes space-ros/space-ros#48.

xfiderek commented 8 months ago

PR is ready, please take a look