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 jacoco to latest release v0.8.0 #1993

Merged
merged 1 commit into from Mar 9, 2018

Conversation

shobhitagarwal1612
Copy link
Contributor

@shobhitagarwal1612 shobhitagarwal1612 commented Mar 9, 2018

Some of the unit tests were not getting included in the code coverage report due to some bug in the old jacoco version that we were using. The latest version v0.8.0 is supposed to fix all that issue.

For eg. Validator.java is 100% covered but in codecov it displays 0%.

What has been done to verify that this works as intended?

Tried running the report locally for unit tests and the code coverage increased as expected (Validator file showing 100% coverage and many others)

Why is this the best possible solution? Were any other approaches considered?

Tried approaching the firebase-community and they suggest this as one of the possible solutions

Are there any risks to merging this code? If so, what are they?

Before merging, we need to ensure that the file generated by
firebase is also of the same version so that the report can be properly generated.

Do we need any specific form for testing your changes? If so, please attach one.

no

@lognaturel
Copy link
Member

Heya @shobhitagarwal1612! The reason for the downgrade was to be able to work with Firebase. Do you have any new information that suggests they've upgraded?

@shobhitagarwal1612
Copy link
Contributor Author

@lognaturel I have updated the description for the PR

@shobhitagarwal1612
Copy link
Contributor Author

shobhitagarwal1612 commented Mar 9, 2018

@lognaturel Should I remove the master filter from circle-ci config and try to rebuild the branch just to ensure if everything works fine? I'll revert it back after that

@lognaturel
Copy link
Member

Seems Firebase continues to generate 0x1006 files. If it didn't, the current report would fail, right?

Great idea to reach out to the firebase community. Do you want to link that post here for future reference?

Should I remove the master filter and try to rebuild the branch just to ensure if everything works fine? I'll revert it back after that

Good idea.

@shobhitagarwal1612
Copy link
Contributor Author

Great idea to reach out to the firebase community. Do you want to link that post here for future reference?

I used the slack channel. Can that be linked?

@shobhitagarwal1612
Copy link
Contributor Author

@shobhitagarwal1612
Copy link
Contributor Author

It works !!! Check out this link
The code coverage increased to 28%

@shobhitagarwal1612
Copy link
Contributor Author

@lognaturel I've cleaned the commits and removed unncessary changes

@lognaturel
Copy link
Member

Well isn't that great! Thanks, @shobhitagarwal1612!! Mysterious that 0.7.9 didn't work... but oh well!

@lognaturel lognaturel merged commit 188baa7 into getodk:master Mar 9, 2018
@shobhitagarwal1612 shobhitagarwal1612 deleted the update-jacoco branch March 9, 2018 21:46
@shobhitagarwal1612
Copy link
Contributor Author

In case if you are wondering the source of the solution, it is adopted from here

shobhitagarwal1612 added a commit to shobhitagarwal1612/collect that referenced this pull request May 15, 2018
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.

None yet

3 participants