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

Update to AndroidX, gradle version and plugin updates #357

Merged
merged 20 commits into from
Aug 23, 2022

Conversation

ekigamba
Copy link
Contributor

@ekigamba ekigamba commented Jul 26, 2022

  • Update to Mapbox v9
  • Update to AndroidX
  • Update AGP to 7.1.3

Reduced coverage by 10%

  • Jacoco has a known bug that results in no coverage generation when buildType.debug.testCoverageEnabled is set to true. With this option set to false, gradle does not find the task 'createDebugCoverageReport which is used to run instrumentation tests with coverage reporting. These tests contribute to a fraction of the test coverage.

Below is the difference especially in the sample module where the tests are mostly instrumentation tests
Screenshot from 2022-08-22 18-11-48
Before

Screenshot from 2022-08-22 18-11-38
After

This is being tracked here

@ekigamba ekigamba force-pushed the update-dependencies branch 5 times, most recently from 5559d3d to 0845b02 Compare July 27, 2022 14:11
…a 11

- Fix getMasterCommitCount() groovy function not working by fetching origin and changing branch to origin/master
@ekigamba ekigamba force-pushed the update-dependencies branch 2 times, most recently from 0406944 to 89c8389 Compare August 4, 2022 09:28
- A bug as reported on a relative repository vanniktech/gradle-android-junit-jacoco-plugin#183
  prevents jacoco exec coverage file from being generated when android.buildTypes.debug.testCoverageEnabled is true
- Disabled the createDebugCoverageReport from the jacocoTestReport therefore disabling running of instrumentation tests
@coveralls
Copy link

coveralls commented Aug 4, 2022

Pull Request Test Coverage Report for Build #584

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-9.4%) to 44.187%

Totals Coverage Status
Change from base Build #582: -9.4%
Covered Lines: 2611
Relevant Lines: 5909

💛 - Coveralls

@ekigamba ekigamba force-pushed the update-dependencies branch 2 times, most recently from db184bb to 6d69618 Compare August 21, 2022 06:28
- Downgrade Robolectric to 4.3.1 which is the only working version currently
- Update Mapbox annotations to com.mapbox.mapboxsdk:mapbox-android-plugin-annotation-v9:0.9.0
- Move dependency names and versions to configs.gradle
- Switch CI OS to Macos for better AVD stability
- Enable download of Robolectric jars manually
@ekigamba ekigamba marked this pull request as ready for review August 22, 2022 14:44
@ndegwamartin
Copy link
Collaborator

The PR looks good for merge, however I think we should create a ticket around reporting the Instrumentation coverage.
cc @ekigamba

@@ -29,6 +29,11 @@ POM_SETTING_LICENCE_URL=http://www.apache.org/licenses/LICENSE-2.0.txt
POM_SETTING_LICENCE_DIST=repo
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we need to bump up the artefact version for tagging on git and publishing the snapshot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do that here but I prefer to have this on separate PRs for better tracking since releases were not always done after every PR https://github.com/onaio/kujaku/pulls?q=is%3Apr+sort%3Aupdated-desc+is%3Aclosed+release

Comment on lines +66 to +67
//libraryModuleRelease(configuration)
libraryModuleDevelopment(this, configuration)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need some documentation on the usage of these i.e. release vs dev mode e.g. on the READ ME

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An issue to update the documentation here #359

@ekigamba
Copy link
Contributor Author

The PR looks good for merge, however I think we should create a ticket around reporting the Instrumentation coverage. cc @ekigamba

I agree. Here is the ticket #358

@ekigamba ekigamba merged commit ef13dbe into master Aug 23, 2022
@ekigamba ekigamba deleted the update-dependencies branch August 23, 2022 13:34
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.

3 participants