[TEST] Adds the codecoverage test #32

Irallia · 2020-11-16T17:28:06Z

Resolves #30

Using the example https://github.com/codecov, we created a codecoverage for the app-template.
The documentation can be found here https://docs.codecov.io/docs/about-the-codecov-bash-uploader

.travis.yml

eseiler · 2020-11-20T09:54:49Z

You can also try to run cmake and make locally. The only thing that won't happen when you build locally is the upload to codecov :)

codecov-commenter · 2021-04-30T10:17:49Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d54a41d). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             master       #32   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?         1           
  Lines             ?        19           
  Branches          ?         0           
==========================================
  Hits              ?        19           
  Misses            ?         0           
  Partials          ?         0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d54a41d...93aa8ad. Read the comment docs.

eseiler

Cool!

At some point, we probably want to use seqan3's cmake stuff, but it does not seem that trivial, because we do the api/cli stuff differently.

I'll ask Marcel to have a glance at this and then we can merge this.
I would then maybe adapt the CI to better reflect our current seqan3 CI and clean up the workflow file (maybe even split it like in seqan3).

Co-authored-by: Lydia Buntrock <[email protected]> Co-authored-by: David Heller <[email protected]>

.github/workflows/ci.yml

Signed-off-by: Lydia Buntrock <[email protected]>

.github/workflows/ci.yml

Irallia · 2021-04-30T13:54:06Z

We still have the warning in Submit coverage build

->  Issue detecting commit SHA. Please run actions/checkout with fetch-depth > 1 or set to 0

I try the solution of codecov/codecov-action#190

eseiler · 2021-04-30T14:03:16Z

We still have the warning in Submit coverage build
->  Issue detecting commit SHA. Please run actions/checkout with fetch-depth > 1 or set to 0
I try the solution of codecov/codecov-action#190

Just revert it, please don't use a depth of 0 because it will pull the complete history.
When this PR is done, I can pull the changes from seqan3 which will also resolve this issue

Irallia · 2021-04-30T14:07:02Z

We still have the warning in Submit coverage build
->  Issue detecting commit SHA. Please run actions/checkout with fetch-depth > 1 or set to 0
I try the solution of codecov/codecov-action#190
Just revert it, please don't use a depth of 0 because it will pull the complete history.
When this PR is done, I can pull the changes from seqan3 which will also resolve this issue

Okay

marehr

A couple of small things. Great work, thank you!

marehr · 2021-04-30T15:43:33Z

CMakeLists.txt

+# Code Coverage Configuration
+add_library(coverage_config INTERFACE)
+
+option(CODE_COVERAGE "Enable coverage reporting" OFF)


Suggested change

option(CODE_COVERAGE "Enable coverage reporting" OFF)

option(SEQAN_APP_CODE_COVERAGE "Enable coverage reporting" OFF)

This should have a proper unique name

marehr · 2021-04-30T15:43:40Z

CMakeLists.txt

+
+option(CODE_COVERAGE "Enable coverage reporting" OFF)
+if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+  message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")


Suggested change

message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")

message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")

?

marehr · 2021-04-30T15:46:49Z

CMakeLists.txt

+if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+  message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")
+  # Add required flags (GCC & LLVM/Clang)
+  target_compile_options(coverage_config INTERFACE


Suggested change

target_compile_options(coverage_config INTERFACE

target_compile_options(seqan_app_coverage_config INTERFACE

this needs some prefix

marehr · 2021-04-30T15:48:39Z

CMakeLists.txt

+  else()
+    target_link_libraries(coverage_config INTERFACE --coverage)
+  endif()
+endif(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")


Suggested change

endif(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")

endif()

marehr · 2021-04-30T15:50:43Z

CMakeLists.txt

@@ -27,6 +27,25 @@ set (FontReset "${Esc}[m")
 # Dependency: SeqAn3.
 find_package (SeqAn3 QUIET REQUIRED HINTS lib/seqan3/build_system)

+# Code Coverage Configuration
+add_library(coverage_config INTERFACE)


Suggested change

add_library(coverage_config INTERFACE)

add_library (coverage_config INTERFACE)

We put spaces between name and (

marehr · 2021-04-30T15:58:35Z

CMakeLists.txt

+    -O0        # no optimization
+    -g         # generate debug info


These are not guaranteed to overwrite the build type.

I would just warn that CMAKE_BUILD_TYPE isn't set to debug and remove the above lines

Something like

if (CMAKE_BUILD_TYPE AND NOT CMAKE_BUILD_TYPE MATCHES "DEBUG") message (WARNING "Please use build type debug, e.g. cmake -DCMAKE_BUILD_TYPE=DEBUG ...") endif ()

marehr · 2021-04-30T16:00:20Z

src/CMakeLists.txt

+# Include code-coverage settings:
+target_link_libraries("${PROJECT_NAME}" PUBLIC coverage_config)


I would make this optional more clear

Suggested change

# Include code-coverage settings:

target_link_libraries("${PROJECT_NAME}" PUBLIC coverage_config)

# Include code-coverage settings if requested:

if (CODE_COVERAGE)

target_link_libraries ("${PROJECT_NAME}" PUBLIC coverage_config)

endif ()

marehr · 2021-04-30T16:01:31Z

.github/workflows/ci.yml

+            cmake ../app -DCODE_COVERAGE=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}"
+          else
+            cmake ../app -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}"


@eseiler The only difference is -DCODE_COVERAGE=ON, can't we just add a ${{ matrix.additional_cmake_flags }} ?

-DCODE_COVERAGE=${{ matrix.build_type == 'Coverage' && 'ON' || 'OFF' }} should work (maybe setting it as env variable for this step is cleaner).

I would do this in my follow-up PR, because I probably change something anyways

marehr · 2021-04-30T16:03:12Z

.github/workflows/ci.yml

+          lcov --directory ./app-build/ --capture --output-file ./app-build/app_coverage --gcov-tool gcov-10
+          lcov --remove ./app-build/app_coverage '/usr/*' '${{ github.workspace }}/app/lib/*' '${{ github.workspace }}/app-build/vendor/*' --output-file ./app-build/app_coverage
+          lcov --list ./app-build/app_coverage


seqan3 we integrated that into cmake in "test/coverage", but I guess it is fine for now to just have this in CI

Yes, but it doesn't seem trivial to reuse it. As far as I can see it relies on a certain way the tests are configured...

Signed-off-by: Lydia Buntrock <[email protected]>

Irallia · 2021-05-03T10:14:18Z

If you're okay with the changes, you can just sqash merge.

eseiler reviewed Nov 19, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

Irallia force-pushed the TEST/add_codecov_test branch 2 times, most recently from e4c868b to d85c316 Compare November 19, 2020 14:58

Irallia force-pushed the TEST/add_codecov_test branch 2 times, most recently from 8564338 to 5be818b Compare November 28, 2020 16:00

Irallia mentioned this pull request Apr 27, 2021

iGenVar - Write API & CLI tests for all functions seqan/iGenVar#30

Closed

13 tasks

Irallia force-pushed the TEST/add_codecov_test branch 3 times, most recently from 93f8b50 to 4c040f0 Compare April 30, 2021 10:13

eseiler approved these changes Apr 30, 2021

View reviewed changes

eseiler requested a review from marehr April 30, 2021 10:34

eseiler marked this pull request as ready for review April 30, 2021 10:43

[TEST] Add code coverage

b62ca48

Co-authored-by: Lydia Buntrock <[email protected]> Co-authored-by: David Heller <[email protected]>

Irallia force-pushed the TEST/add_codecov_test branch from 4c040f0 to b62ca48 Compare April 30, 2021 10:52

eseiler reviewed Apr 30, 2021

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

[TEST] Update removed paths

c8a4dac

Signed-off-by: Lydia Buntrock <[email protected]>

eseiler reviewed Apr 30, 2021

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Update .github/workflows/ci.yml

44714ce

Irallia force-pushed the TEST/add_codecov_test branch from 93aa8ad to 669783c Compare April 30, 2021 13:59

Irallia force-pushed the TEST/add_codecov_test branch 2 times, most recently from c534d80 to 44714ce Compare April 30, 2021 14:06

marehr reviewed Apr 30, 2021

View reviewed changes

[MISC] Apply Review

b861b8e

Signed-off-by: Lydia Buntrock <[email protected]>

Irallia force-pushed the TEST/add_codecov_test branch from de611b2 to b861b8e Compare May 3, 2021 10:02

Irallia requested a review from marehr May 3, 2021 10:14

marehr approved these changes May 6, 2021

View reviewed changes

marehr merged commit 484b661 into seqan:master May 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEST] Adds the codecoverage test #32

[TEST] Adds the codecoverage test #32

Irallia commented Nov 16, 2020 •

edited

Loading

eseiler commented Nov 20, 2020

codecov-commenter commented Apr 30, 2021 •

edited

Loading

eseiler left a comment •

edited

Loading

Irallia commented Apr 30, 2021 •

edited

Loading

eseiler commented Apr 30, 2021

Irallia commented Apr 30, 2021

marehr left a comment

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

marehr Apr 30, 2021

eseiler Apr 30, 2021 •

edited

Loading

marehr Apr 30, 2021

eseiler Apr 30, 2021

Irallia commented May 3, 2021

	option(CODE_COVERAGE "Enable coverage reporting" OFF)
	option(SEQAN_APP_CODE_COVERAGE "Enable coverage reporting" OFF)

	message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")
	message (STATUS "Hallo CMAKE_CXX_COMPILER_ID")

	target_compile_options(coverage_config INTERFACE
	target_compile_options(seqan_app_coverage_config INTERFACE

	endif(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
	endif()

	add_library(coverage_config INTERFACE)
	add_library (coverage_config INTERFACE)

		# Include code-coverage settings:
		target_link_libraries("${PROJECT_NAME}" PUBLIC coverage_config)

-# Include code-coverage settings:
-target_link_libraries("${PROJECT_NAME}" PUBLIC coverage_config)
+# Include code-coverage settings if requested:
+if (CODE_COVERAGE)
+    target_link_libraries ("${PROJECT_NAME}" PUBLIC coverage_config)
+endif ()

[TEST] Adds the codecoverage test #32

[TEST] Adds the codecoverage test #32

Conversation

Irallia commented Nov 16, 2020 • edited Loading

eseiler commented Nov 20, 2020

codecov-commenter commented Apr 30, 2021 • edited Loading

Codecov Report

eseiler left a comment • edited Loading

Choose a reason for hiding this comment

Irallia commented Apr 30, 2021 • edited Loading

eseiler commented Apr 30, 2021

Irallia commented Apr 30, 2021

marehr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eseiler Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Irallia commented May 3, 2021

Irallia commented Nov 16, 2020 •

edited

Loading

codecov-commenter commented Apr 30, 2021 •

edited

Loading

eseiler left a comment •

edited

Loading

Irallia commented Apr 30, 2021 •

edited

Loading

eseiler Apr 30, 2021 •

edited

Loading