ros2 / ci

ROS 2 CI Infrastructure
http://ci.ros2.org/
Apache License 2.0
48 stars 30 forks source link

Fixed lcov error #771

Closed ahcorde closed 4 months ago

ahcorde commented 5 months ago

There is an error on CI https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/2348/consoleText related with lcov, this should fix the problem

ahcorde commented 5 months ago
Crola1702 commented 5 months ago

Could this solve: https://github.com/ros2/ci/issues/770?

ahcorde commented 5 months ago

@Crola1702 yes, It's a fix for this issue

ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago

@clalancette I think i fixed this build, coverage is here https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/2361/cobertura/.

Do you mind to take another look to the changes?

clalancette commented 5 months ago

So I'm not totally sure about this, but this may not be producing correct results.

In particular, if you look at:

https://ci.ros2.org/view/nightly/job/nightly_linux_iron_coverage/408/cobertura/src_ros2_rcutils_src/allocator_c/allocator_c/ , you'll see that everything looks like we expect.

But if you look at https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/2361/cobertura/src_ros2_rcutils_src/allocator_c/allocator_c/ (from the CI here), you'll see some functions with numbers in front of them, like 104,rcutils_allocator_is_valid.

I have no idea if that is what lcov_corbertura_fix was fixing, but it may be. Either way, what this is producing right now doesn't seem right. I guess we should be able to pull lcov_corbertura_fix from pypi, and see what it is doing differently to lcov_corbertura.

ahcorde commented 5 months ago

I have no idea if that is what lcov_corbertura_fix was fixing, but it may be. Either way, what this is producing right now doesn't seem right. I guess we should be able to pull lcov_corbertura_fix from pypi, and see what it is doing differently to lcov_corbertura.

The problem with lcov_corbertura_fix is that is not working, ant the Github repository doesn't exist anymore. I will try to find more details about this

clalancette commented 5 months ago

The problem with lcov_corbertura_fix is that is not working, ant the Github repository doesn't exist anymore. I will try to find more details about this

Yeah, understood. We can at least pull the code from https://pypi.org/project/lcov-cobertura-fix/#files , and compare it to a "pristine" lcov_corbertua to see what the differences are.

ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago
ahcorde commented 5 months ago

Coverages are more simular right now

But I can see this error now:

13:30:14 [Cobertura] Publishing Cobertura coverage report...
13:30:14 
13:30:15 [Cobertura] Publishing Cobertura coverage results...
13:30:15 
13:30:16 [Cobertura] Cobertura coverage report found.
13:30:16 
13:30:16 ERROR: ERROR: Failure to paint /home/jenkins-agent/workspace/nightly_linux_coverage/ws/src/ros2/launch/launch/launch/utilities/normalize_to_list_of_substitutions_impl.py to /var/lib/jenkins/jobs/nightly_linux_coverage/cobertura
13:30:16 java.io.IOException: Failed to deserialize response to UserRequest:hudson.FilePath$WritePipe@2dc6ceb6: java.lang.SecurityException: Agent may not access a file path. See the system log for more details about the error ID '62b5d3e7-a5ae-41d2-8a81-d5d411ec39e1' and https://www.jenkins.io/redirect/security-144 for more information.

any thoughts?

ahcorde commented 5 months ago

related PR https://github.com/eriwen/lcov-to-cobertura-xml/pull/55

ahcorde commented 5 months ago
ERROR: ERROR: Failure to paint /home/jenkins-agent/workspace/nightly_linux_coverage/ws/src/ros2/launch/launch/launch/utilities/normalize_to_list_of_substitutions_impl.py to /var/lib/jenkins/jobs/nightly_linux_coverage/cobertura
sloretz commented 5 months ago

@clalancette assigning you as it looks like @ahcorde is awaiting your thoughts 🧇

ahcorde commented 4 months ago
ahcorde commented 4 months ago
clalancette commented 4 months ago

Jazzy CI failed because this needed to be rebased against the latest code (which I've now done). Here is another round:

clalancette commented 4 months ago

All right, this now looks good to me. Since the PR still hasn't merged upstream, let's go with Alex's fork for now, and then when it is eventually merged and released upstream we can use that.