Adds Code Coverage #1076

navinns · 2020-07-24T01:28:04Z

Issue #, if available:

Description of changes:

Adds code to create coverage report
Adds option to upload report to codecov.io

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

tools/ci/ci_agent.py

dan4thewin · 2020-07-24T21:47:21Z

tools/ci/ci_agent.py

+            log("\n----------------------------------------------------------------")
+            log("Upload Code Coverage report to codecov.io")
+            log("----------------------------------------------------------------")
+            cmd = f"curl -s https://codecov.io/bash | bash -s - -t {codecov_token}"


Hey... umm... this is a non-starter.

To clarify, I mean running untrusted code is a non-starter.

codecov-commenter · 2020-07-28T01:53:08Z

Codecov Report

Merging #1076 into development will increase coverage by 3.45%.
The diff coverage is 100.00%.

@@               Coverage Diff                @@
##           development     #1076      +/-   ##
================================================
+ Coverage        96.54%   100.00%   +3.45%     
================================================
  Files                9         4       -5     
  Lines             5643      1180    -4463     
  Branches           641       365     -276     
================================================
- Hits              5448      1180    -4268     
+ Misses               9         0       -9     
+ Partials           186         0     -186

Impacted Files	Coverage Δ
libraries/standard/mqtt/src/mqtt_state.c	`100.00% <ø> (+4.06%)`	⬆️
libraries/standard/http/src/http_client.c	`100.00% <100.00%> (+12.51%)`	⬆️
libraries/standard/mqtt/src/mqtt.c	`100.00% <100.00%> (+5.58%)`	⬆️
libraries/standard/mqtt/src/mqtt_lightweight.c	`100.00% <100.00%> (+5.48%)`	⬆️

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 c2f3405...df9d7c7. Read the comment docs.

nateglims · 2020-07-29T20:38:04Z

tools/ci/ci_agent.py

+        target_build_result = target_result.setdefault("CodeCoverage", {})
+        out = _build_target("coverage", src, build_path, build_flags, c_flags)
+        log(out)
+        out = _run_cmd(f"gcovr -r . -x -o {build_path}/cobertura.xml")


Does this need to be named cobertura?

yes as we can use this file with cobertura as well

nateglims · 2020-07-29T20:39:20Z

tools/ci/ci_agent.py

+        out = _run_cmd(f"gcovr -r . -x -o {build_path}/cobertura.xml")
+        log(out)
+
+        commit_id = os.environ.get("ghprbActualCommit") or ""


We probably want coverage run on the merge head (${sha1} in Jenkins)

nateglims · 2020-07-29T20:41:04Z

tools/ci/ci_agent.py

+        cmd_code_coverage,
+        "--config-file",
+        "--src",
+        "--build-path",
+        "--build-flags",
+        "--c-flags",
+        "--codecov-token",
+    )


At this point I'm inclined to think we should just take the config as an argument and do the rest through the config or environment variables. It's getting difficult to grapple with all the options.

config is one of the argument, we have options to override options in config

I'm not convinced that flexibility is required.

tools/ci/ci_agent.py

nateglims · 2020-07-29T20:45:10Z

tools/ci/ci_agent.py

    return result


+def _build_code_coverage(src, build_path, build_flags, c_flags, codecov_token):


what's the significance of build_flags vs c_flags?

in csdk we have 2 levels of flags one for cmake and other for compiler

build_flags is for cmake and c_flags are for compiler

I don't like the build_flags name. But at the very least we should have a descriptive doc string saying what they are.

tools/ci/ci_agent.py

* Adds code to create coverage report * Adds option to upload report to codecov.io

dan4thewin · 2020-08-04T23:52:13Z

tools/ci/ci_agent.py

+        target_build_result = target_result.setdefault("CodeCoverage", {})
+        out = _build_target("coverage", src, build_path, build_flags, c_flags)
+        log(out)
+        out = _run_cmd(f"gcovr -r . -x -o {build_path}/cobertura.xml")


Looks like this might be passed to a shell - so what happens if build path contains a space?

or worse, a semicolon...

* Adds Code Coverage * Adds code to create coverage report * Adds option to upload report to codecov.io * Use requests

navinns requested review from aggarw13, cobusve and pavanmr94 July 24, 2020 01:28

navinns force-pushed the development branch from 4831ed7 to b59ce3a Compare July 24, 2020 03:03

aggarw13 reviewed Jul 24, 2020

View reviewed changes

tools/ci/ci_agent.py Show resolved Hide resolved

navinns force-pushed the development branch from b59ce3a to d60a9ec Compare July 24, 2020 19:59

navinns requested a review from aggarw13 July 24, 2020 20:00

navinns force-pushed the development branch 2 times, most recently from 59cfdd8 to 97629a4 Compare July 24, 2020 20:27

pavanmr94 previously approved these changes Jul 24, 2020

View reviewed changes

aggarw13 previously approved these changes Jul 24, 2020

View reviewed changes

navinns requested a review from dan4thewin July 24, 2020 21:46

dan4thewin suggested changes Jul 24, 2020

View reviewed changes

navinns dismissed stale reviews from aggarw13 and pavanmr94 via df9d7c7 July 29, 2020 20:29

navinns force-pushed the development branch from 6289eaa to df9d7c7 Compare July 29, 2020 20:29

navinns requested review from aggarw13, dan4thewin, nateglims and pavanmr94 July 29, 2020 20:30

nateglims reviewed Jul 29, 2020

View reviewed changes

pavanmr94 reviewed Jul 29, 2020

View reviewed changes

tools/ci/ci_agent.py Outdated Show resolved Hide resolved

navinns added 3 commits August 4, 2020 20:49

Adds Code Coverage

8cd7cba

* Adds code to create coverage report * Adds option to upload report to codecov.io

Use requests

e9ed3c6

Add docstring

4711b52

navinns force-pushed the development branch from df9d7c7 to 4711b52 Compare August 4, 2020 23:23

nateglims previously approved these changes Aug 4, 2020

View reviewed changes

dan4thewin reviewed Aug 4, 2020

View reviewed changes

navinns dismissed nateglims’s stale review via d980d51 August 5, 2020 00:52

navinns force-pushed the development branch 2 times, most recently from 4be5512 to f55fbf7 Compare August 5, 2020 01:02

nateglims previously approved these changes Aug 5, 2020

View reviewed changes

Add quote

45f9c9c

navinns dismissed nateglims’s stale review via 45f9c9c August 5, 2020 01:12

navinns force-pushed the development branch from f55fbf7 to 45f9c9c Compare August 5, 2020 01:12

dan4thewin approved these changes Aug 5, 2020

View reviewed changes

nateglims approved these changes Aug 5, 2020

View reviewed changes

navinns merged commit b5f1ed6 into aws:development Aug 5, 2020

navinns deleted the development branch August 5, 2020 01:17

navinns restored the development branch August 5, 2020 19:26

leegeth pushed a commit to leegeth/aws-iot-device-sdk-embedded-C that referenced this pull request Aug 27, 2020

Adds Code Coverage (aws#1076)

c092610

* Adds Code Coverage * Adds code to create coverage report * Adds option to upload report to codecov.io * Use requests

leegeth pushed a commit to leegeth/aws-iot-device-sdk-embedded-C that referenced this pull request Aug 28, 2020

Adds Code Coverage (aws#1076)

ca91cf8

* Adds Code Coverage * Adds code to create coverage report * Adds option to upload report to codecov.io * Use requests

leegeth pushed a commit to leegeth/aws-iot-device-sdk-embedded-C that referenced this pull request Aug 31, 2020

Adds Code Coverage (aws#1076)

685790c

* Adds Code Coverage * Adds code to create coverage report * Adds option to upload report to codecov.io * Use requests

leegeth pushed a commit to leegeth/aws-iot-device-sdk-embedded-C that referenced this pull request Sep 1, 2020

Adds Code Coverage (aws#1076)

809fe55

* Adds Code Coverage * Adds code to create coverage report * Adds option to upload report to codecov.io * Use requests

		return result


		def _build_code_coverage(src, build_path, build_flags, c_flags, codecov_token):

Uh oh!

Adds Code Coverage #1076

Adds Code Coverage #1076

Uh oh!

Conversation

navinns commented Jul 24, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Jul 28, 2020 •

edited

Loading