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

C++ code coverage is broken #39303

Closed
Trott opened this issue Jul 8, 2021 · 6 comments
Closed

C++ code coverage is broken #39303

Trott opened this issue Jul 8, 2021 · 6 comments
Labels
build Issues and PRs related to build files or the CI. coverage Issues and PRs related to native coverage support.

Comments

@Trott
Copy link
Member

Trott commented Jul 8, 2021

Version

master branch

Platform

n/a

Subsystem

build

What steps will reproduce the bug?

View https://coverage.nodejs.org/. See (screenshot below) that the C++ coverage broke some time around June 24 2021.

image

How often does it reproduce? Is there a required condition?

n/a

What is the expected behavior?

Coverage should be generated

What do you see instead?

No coverage generated

Additional information

The last commit shown where the cover worked is e0ebc6b. The commit shown where it stops working is 84d6ce9. If this is a problem in the repository (and not a build infra issue), then that means it is in one of these three commits:

84d6ce9
d65514b
bf9ce95

Of those three commits, the one that could at least plausibly could have somehow caused this is d65514b. /ping @richardlau

@richardlau
Copy link
Member

It's unlikely to be any of those commits. The other, more plausible, difference between

is that we started building with ccache g++-8 (nodejs/build#2684).

@richardlau
Copy link
Member

Logged into the machine and tried building manually without ccache -- problem still exists (C++ coverage is 0%) with g++-8.

@richardlau
Copy link
Member

gcov is

$ gcov --version
gcov (Ubuntu 5.5.0-12ubuntu1~16.04) 5.5.0 20171010
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE.

Testing (manually) with GCOV environment variable set to gcov-8.

@richardlau
Copy link
Member

With gcov-8 I get errors from gcovr:

Finding source file corresponding to a gcov data file
  currdir      /tmp/node/out
  gcov_fname   ^#src#crypto#crypto_context.cc.gcov
               ['        -', '    0', 'Source', '../src/crypto/crypto_context.cc\n']
  source_fname /tmp/node/out/Release/obj.target/libnode/src/crypto/crypto_context.gcda
  root         /tmp/node/out/Release/obj.target
  fname        /tmp/node/out/../src/crypto/crypto_context.cc
Parsing coverage data for file /tmp/node/out/../src/crypto/crypto_context.cc
  Filtering coverage data for file /tmp/node/out/../src/crypto/crypto_context.cc
Traceback (most recent call last):
  File "/tmp/node/out/../gcovr/scripts/gcovr", line 2415, in <module>
    process_datafile(file_, covdata, options)
  File "/tmp/node/out/../gcovr/scripts/gcovr", line 884, in process_datafile
    process_gcov_data(fname, covdata, abs_filename, options)
  File "/tmp/node/out/../gcovr/scripts/gcovr", line 616, in process_gcov_data
    covered[lineno] = int(segments[0].strip())
ValueError: invalid literal for int() with base 10: '885*'
Makefile:241: recipe for target 'coverage-test' failed

I think we'd need a gcovr update (currently pinned at 3.4:

node/Makefile

Lines 223 to 224 in c6d9d8a

if [ ! -d gcovr ]; then git clone -b 3.4 --depth=1 \
--single-branch https://github.com/gcovr/gcovr.git; fi
) because of gcovr/gcovr#226, which was fixed by gcovr/gcovr#228 in gcov 4. There's a slight complication there as gcov 4 and later need to be installed via pip (3.4 was runnable as a script but script/gcovr is gone in the later version). FWIW the GitHub actions coverage workflow bypasses several targets in the Makefile and uses gcovr 4.2 (via pip install):
run: pip install gcovr==4.2

Alternative to using pip is if we update the benchmark machine (where we're running the coverage builds) to Ubuntu 20.04 and install gcovr via apt: https://packages.ubuntu.com/focal/gcovr. (The apt packaged versions of gcovr in Ubuntu 16.04 is 3.2 and in Ubuntu 18.04 3.4.)

@richardlau
Copy link
Member

Should be addressed by changes in the Makefile and on the CI machine.

nodejs-github-bot pushed a commit that referenced this issue Jul 11, 2021
Update the version of `gcovr` used for C++ coverage from 3.4 to 4.2 for
compatibility with gcc/g++ 8.

PR-URL: #39326
Refs: #39303
Refs: gcovr/gcovr#228
Refs: nodejs/build#2705
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau added build Issues and PRs related to build files or the CI. coverage Issues and PRs related to native coverage support. labels Jul 12, 2021
@richardlau
Copy link
Member

C++ coverage is back:
image

targos pushed a commit that referenced this issue Jul 13, 2021
Update the version of `gcovr` used for C++ coverage from 3.4 to 4.2 for
compatibility with gcc/g++ 8.

PR-URL: #39326
Refs: #39303
Refs: gcovr/gcovr#228
Refs: nodejs/build#2705
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Sep 4, 2021
Update the version of `gcovr` used for C++ coverage from 3.4 to 4.2 for
compatibility with gcc/g++ 8.

PR-URL: #39326
Refs: #39303
Refs: gcovr/gcovr#228
Refs: nodejs/build#2705
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. coverage Issues and PRs related to native coverage support.
Projects
None yet
Development

No branches or pull requests

2 participants