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

Include Codecov reports to Github PR checks #3663

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Conversation

Guardiola31337
Copy link
Contributor

@Guardiola31337 Guardiola31337 commented Oct 19, 2020

Description

Includes Codecov reports to Github PR checks

Fixes #2655

  • I have added any issue links
  • I have added all related labels (bug, feature, new API(s), SEMVER, etc.)
  • I have added the appropriate milestone and project boards

Goal

Include Codecov reports to Github PR checks so we can see how we're doing on every PR.

Implementation

Update core-unit-tests and ui-unit-tests Makefile so that Jacoco test reports are generete and update circle.yml so they're post to Codecov.io

Testing

  • New and existing unit tests pass locally with my changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have updated the CHANGELOG including this PR
  • We might need to update / push api/current.txt files after running $> make core-update-api (Core) / $> make ui-update-api (UI) if there are changes / errors we're 🆗 with (e.g. AddedMethod changes are marked as errors but don't break SemVer) 🚀 If there are SemVer breaking changes add the SEMVER label. See Metalava docs

@Guardiola31337 Guardiola31337 added this to the v1.2.0 milestone Oct 19, 2020
@Guardiola31337 Guardiola31337 requested a review from a team October 19, 2020 17:43
@Guardiola31337 Guardiola31337 self-assigned this Oct 19, 2020
@Guardiola31337 Guardiola31337 marked this pull request as ready for review October 19, 2020 18:45
@@ -73,7 +73,7 @@ assemble-core-release:

.PHONY: core-unit-tests
core-unit-tests:
$(call run-gradle-tasks,$(CORE_MODULES),test)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jacoco is slow and heavy, could you add a separate makefile command for tests with coverage so that it doesn't impact the duration of tests that we run during local development?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, checked the CI building times and sometimes they even take longer

This build -> 9m 17s / 2m 26s https://app.circleci.com/pipelines/github/mapbox/mapbox-navigation-android/9084/workflows/e494e304-e6c5-4d10-bbd7-9a8f4a339e1d/jobs/33801

#3661 build -> 12m 45s / 3m 4s https://app.circleci.com/pipelines/github/mapbox/mapbox-navigation-android/9090/workflows/1b3e8626-1735-4867-a43b-30d123b7a072/jobs/33832 (not sure if we added a bunch of low tests as part of #3661)

#3664 build -> 9m 12s / 2m 15s https://app.circleci.com/pipelines/github/mapbox/mapbox-navigation-android/9085/workflows/38332f8f-217f-4988-bedb-6a55816a2f70/jobs/33808

Will add a separate command to scratch a couple of seconds 👍

@codecov-io
Copy link

Codecov Report

Merging #3663 into master will increase coverage by 8.77%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #3663      +/-   ##
============================================
+ Coverage     39.30%   48.08%   +8.77%     
+ Complexity     2322     1568     -754     
============================================
  Files           550      293     -257     
  Lines         20088    12218    -7870     
  Branches       1919     1298     -621     
============================================
- Hits           7896     5875    -2021     
+ Misses        11254     5755    -5499     
+ Partials        938      588     -350     

circle.yml Outdated
@@ -16,6 +16,7 @@ workflows:
- unit-tests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to run these if we're already running jacoco ones? Looks like duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was your concern 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern was not to change the makefile commands to jacoco because we run them locally as well. On CI, we can run only the jacoco ones 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3663 (comment) Ohh you meant only exposing it in the Makefile to use it locally 👍

@Guardiola31337 Guardiola31337 merged commit c2e0546 into master Oct 20, 2020
@Guardiola31337 Guardiola31337 deleted the pg-2655-codecov branch October 20, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include 1.0 Codecov reports to Github PR checks
3 participants