Fixed lcov error #771

ahcorde · 2024-04-22T15:03:04Z

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 · 2024-04-22T15:05:08Z

Linux

clalancette

Minor change, but otherwise looks good to me. Please make sure to run CI on both Noble and Jammy to double check this works for both.

ros2_batch_job/__main__.py

Crola1702 · 2024-04-22T16:08:19Z

Could this solve: #770?

ahcorde · 2024-04-22T16:13:01Z

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

ahcorde · 2024-04-22T17:52:55Z

Linux

ahcorde · 2024-04-23T09:52:52Z

Linux

ahcorde · 2024-04-23T12:04:17Z

Linux

ahcorde · 2024-04-23T21:08:17Z

Linux

ahcorde · 2024-04-24T12:53:28Z

Linux

ahcorde · 2024-04-25T07:50:27Z

Noble Linux
Jammy Linux

ahcorde · 2024-04-25T07:53:22Z

@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 · 2024-04-25T20:40:43Z

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 · 2024-04-25T20:55:57Z

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 · 2024-04-25T21:00:09Z

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 · 2024-04-26T21:32:39Z

Linux

ahcorde · 2024-04-29T10:04:47Z

Linux

ahcorde · 2024-04-29T10:13:12Z

Linux

ahcorde · 2024-04-29T11:42:13Z

Coverages are more simular right now

with the fix: https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/2370/cobertura/
last green build https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/2342/cobertura/
iron: https://ci.ros2.org/view/nightly/job/nightly_linux_iron_coverage/408/cobertura/

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 · 2024-04-29T11:54:55Z

related PR eriwen/lcov-to-cobertura-xml#55

ahcorde · 2024-04-30T21:51:25Z

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 · 2024-05-10T00:06:54Z

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

clalancette · 2024-05-17T13:16:59Z

linux_docker_resources/Dockerfile

@@ -144,7 +145,7 @@ RUN if test ${COMPILE_WITH_CLANG} = true; then apt-get update && apt-get install
 # Install coverage build dependencies.
 RUN apt-get update && apt-get install --no-install-recommends -y lcov

-RUN pip3 install -U lcov_cobertura_fix
+RUN pip3 install -U lcov_cobertura && rm /usr/local/lib/python3.12/dist-packages/lcov_cobertura/lcov_cobertura.py && wget https://raw.githubusercontent.com/ahcorde/lcov-to-cobertura-xml/master/lcov_cobertura/lcov_cobertura.py -P /usr/local/lib/python3.12/dist-packages/lcov_cobertura/


I think we can move forward with a fork of this fix for now, but we should do that from an repository. We should also add in a comment here explaining exactly why we are doing that.

Let me discuss today with some people to decide where we should put this fork for now.

OK, I talked to @nuclearsandwich about this, and I got a much better idea of what we should do here. I think the easiest thing is to just use your fork for now, since you already have a fork. In that case, I think this should be:

Suggested change

RUN pip3 install -U lcov_cobertura && rm /usr/local/lib/python3.12/dist-packages/lcov_cobertura/lcov_cobertura.py && wget https://raw.githubusercontent.com/ahcorde/lcov-to-cobertura-xml/master/lcov_cobertura/lcov_cobertura.py -P /usr/local/lib/python3.12/dist-packages/lcov_cobertura/

# There is a bug in upstream lcov-to-cobertura-xml with newer versions of lcov. There is an

# open PR for it in https://github.com/eriwen/lcov-to-cobertura-xml/pull/55, but until that is

# merged use the fork with the fix.

RUN pip3 install -U git+https://github.com/ahcorde/lcov-to-cobertura-xml@master

clalancette · 2024-05-17T13:17:47Z

ros2_batch_job/__main__.py

+        lcov_arguments = ['--ignore-errors', 'inconsistent,inconsistent',
+                          '--ignore-errors', 'mismatch,mismatch',
+                          '--ignore-errors', 'negative,negative',
+                          '--ignore-errors', 'unused,unused',
+                          '--ignore-errors', 'empty,empty']


I'm not against this, but can you explain more why we need to ignore all of these errors? I think it would be best to have a comment here with that, so future readers know why.

I was just launching the CI and fixing the errors.

clalancette · 2024-05-17T18:25:41Z

linux_docker_resources/Dockerfile

@@ -144,7 +145,7 @@ RUN if test ${COMPILE_WITH_CLANG} = true; then apt-get update && apt-get install
 # Install coverage build dependencies.
 RUN apt-get update && apt-get install --no-install-recommends -y lcov

-RUN pip3 install -U lcov_cobertura_fix
+RUN pip3 install -U lcov_cobertura && rm /usr/local/lib/python3.12/dist-packages/lcov_cobertura/lcov_cobertura.py && wget https://raw.githubusercontent.com/ahcorde/lcov-to-cobertura-xml/master/lcov_cobertura/lcov_cobertura.py -P /usr/local/lib/python3.12/dist-packages/lcov_cobertura/


OK, I talked to @nuclearsandwich about this, and I got a much better idea of what we should do here. I think the easiest thing is to just use your fork for now, since you already have a fork. In that case, I think this should be:

Suggested change

RUN pip3 install -U lcov_cobertura && rm /usr/local/lib/python3.12/dist-packages/lcov_cobertura/lcov_cobertura.py && wget https://raw.githubusercontent.com/ahcorde/lcov-to-cobertura-xml/master/lcov_cobertura/lcov_cobertura.py -P /usr/local/lib/python3.12/dist-packages/lcov_cobertura/

# There is a bug in upstream lcov-to-cobertura-xml with newer versions of lcov. There is an

# open PR for it in https://github.com/eriwen/lcov-to-cobertura-xml/pull/55, but until that is

# merged use the fork with the fix.

RUN pip3 install -U git+https://github.com/ahcorde/lcov-to-cobertura-xml@master

ahcorde · 2024-05-21T16:29:38Z

Linux

clalancette

One more change. Once that is in, I'm going to suggest we run the coverage jobs for all of Humble, Iron, Jazzy, and Rolling to ensure that things are working across all of them.

clalancette · 2024-05-21T17:26:34Z

linux_docker_resources/Dockerfile

+  yamllint \
+  wget


I think we don't need this change anymore, since we aren't using wget anymore.

ahcorde · 2024-05-22T08:40:25Z

Linux rolling
Linux jazzy
Linux iron
Linux humble

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

clalancette · 2024-05-24T12:43:22Z

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

Linux rolling
Linux jazzy
Linux iron
Linux humble

clalancette · 2024-05-24T16:53:28Z

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.

ahcorde requested review from clalancette, cottsay and nuclearsandwich as code owners April 22, 2024 15:03

clalancette requested changes Apr 22, 2024

View reviewed changes

ros2_batch_job/__main__.py Outdated Show resolved Hide resolved

ros2_batch_job/__main__.py Outdated Show resolved Hide resolved

ahcorde requested a review from clalancette April 25, 2024 20:31

sloretz assigned clalancette May 10, 2024

clalancette requested changes May 17, 2024

View reviewed changes

clalancette mentioned this pull request May 20, 2024

Buildfarmer log 2024/05/20 - 2024/05/27 osrf/buildfarm-tools#53

Closed

ahcorde requested a review from clalancette May 21, 2024 16:30

clalancette requested changes May 21, 2024

View reviewed changes

ahcorde added 12 commits May 24, 2024 08:40

Fixed lcov error

b70e667

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Added feedback

73dda6e

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Added more ignores

275ad8b

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Include arguments in lcov remove

bc2c95c

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

lcov_cobertura

27bccc9

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

use lcov_cobertura instead of lcov_cobertura_fix

ba3799e

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

remove args on cobertura

c4e8c49

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Try fix

6c473c2

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

installed wget

0978660

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

fix

51064d0

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Used personal fork

d2c70bb

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Removed wget

302b4ff

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

clalancette force-pushed the ahcorde/fix/lcov branch from 59ebc69 to 302b4ff Compare May 24, 2024 12:41

clalancette approved these changes May 24, 2024

View reviewed changes

clalancette merged commit 5d3807e into master May 24, 2024
1 check passed

delete-merged-branch bot deleted the ahcorde/fix/lcov branch May 24, 2024 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed lcov error #771

Fixed lcov error #771

ahcorde commented Apr 22, 2024

ahcorde commented Apr 22, 2024 •

edited

clalancette left a comment

Crola1702 commented Apr 22, 2024

ahcorde commented Apr 22, 2024

ahcorde commented Apr 22, 2024 •

edited by clalancette

ahcorde commented Apr 23, 2024

ahcorde commented Apr 23, 2024 •

edited

ahcorde commented Apr 23, 2024

ahcorde commented Apr 24, 2024

ahcorde commented Apr 25, 2024 •

edited

ahcorde commented Apr 25, 2024

clalancette commented Apr 25, 2024

ahcorde commented Apr 25, 2024

clalancette commented Apr 25, 2024

ahcorde commented Apr 26, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 30, 2024

sloretz commented May 10, 2024 •

edited

clalancette May 17, 2024

clalancette May 17, 2024

clalancette May 17, 2024

ahcorde May 21, 2024

clalancette May 17, 2024

ahcorde commented May 21, 2024

clalancette left a comment

clalancette May 21, 2024

ahcorde commented May 22, 2024 •

edited

clalancette commented May 24, 2024

clalancette commented May 24, 2024

-RUN pip3 install -U lcov_cobertura && rm /usr/local/lib/python3.12/dist-packages/lcov_cobertura/lcov_cobertura.py && wget https://raw.githubusercontent.com/ahcorde/lcov-to-cobertura-xml/master/lcov_cobertura/lcov_cobertura.py -P /usr/local/lib/python3.12/dist-packages/lcov_cobertura/
+# There is a bug in upstream lcov-to-cobertura-xml with newer versions of lcov.  There is an
+# open PR for it in https://github.com/eriwen/lcov-to-cobertura-xml/pull/55, but until that is
+# merged use the fork with the fix.
+RUN pip3 install -U git+https://github.com/ahcorde/lcov-to-cobertura-xml@master

Fixed lcov error #771

Fixed lcov error #771

Conversation

ahcorde commented Apr 22, 2024

ahcorde commented Apr 22, 2024 • edited

clalancette left a comment

Choose a reason for hiding this comment

Crola1702 commented Apr 22, 2024

ahcorde commented Apr 22, 2024

ahcorde commented Apr 22, 2024 • edited by clalancette

ahcorde commented Apr 23, 2024

ahcorde commented Apr 23, 2024 • edited

ahcorde commented Apr 23, 2024

ahcorde commented Apr 24, 2024

ahcorde commented Apr 25, 2024 • edited

ahcorde commented Apr 25, 2024

clalancette commented Apr 25, 2024

ahcorde commented Apr 25, 2024

clalancette commented Apr 25, 2024

ahcorde commented Apr 26, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 29, 2024

ahcorde commented Apr 30, 2024

sloretz commented May 10, 2024 • edited

clalancette May 17, 2024

Choose a reason for hiding this comment

clalancette May 17, 2024

Choose a reason for hiding this comment

clalancette May 17, 2024

Choose a reason for hiding this comment

ahcorde May 21, 2024

Choose a reason for hiding this comment

clalancette May 17, 2024

Choose a reason for hiding this comment

ahcorde commented May 21, 2024

clalancette left a comment

Choose a reason for hiding this comment

clalancette May 21, 2024

Choose a reason for hiding this comment

ahcorde commented May 22, 2024 • edited

clalancette commented May 24, 2024

clalancette commented May 24, 2024

ahcorde commented Apr 22, 2024 •

edited

ahcorde commented Apr 22, 2024 •

edited by clalancette

ahcorde commented Apr 23, 2024 •

edited

ahcorde commented Apr 25, 2024 •

edited

sloretz commented May 10, 2024 •

edited

ahcorde commented May 22, 2024 •

edited