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

more tests for better coverage #509

Merged
merged 9 commits into from
Nov 25, 2022
Merged

more tests for better coverage #509

merged 9 commits into from
Nov 25, 2022

Conversation

calfaand
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #509 (b491143) into cli3 (6001732) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               cli3     #509   +/-   ##
=========================================
  Coverage     59.47%   59.47%           
  Complexity     1036     1036           
=========================================
  Files           153      153           
  Lines          4771     4771           
  Branches        737      737           
=========================================
  Hits           2837     2837           
  Misses         1538     1538           
  Partials        396      396           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@andrii-bodnar
Copy link
Member

Hi @calfaand, thanks a lot for your contribution! 🚀

It's strange but it seems like the new tests are not affecting the coverage (according to the codecov comment)

Could you please take a look?

@calfaand
Copy link
Contributor Author

Yes it seems strange to me because I verified it in Intelijj idea and it shows different coverage, but I take look at it later afternoon

@andrii-bodnar
Copy link
Member

@calfaand thank you! I'm going to merge this PR and then will see if the coverage will be increased.

@andrii-bodnar andrii-bodnar merged commit 0d8e3d8 into crowdin:cli3 Nov 25, 2022
@andrii-bodnar andrii-bodnar linked an issue Nov 25, 2022 that may be closed by this pull request
6 tasks
@andrii-bodnar
Copy link
Member

@calfaand the coverage wasn't changed after the merge.

But I found another interesting thing:

The PropertiesBuilderTest.java class is located in the java/com/crowdin/cli/commands/functionality namespace,

but the PropertiesBuilders.java class itself is located in the java/com/crowdin/cli/properties namespace.

It could be the reason for not accounting coverage. Could you please move the test class to the right namespace?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit tests coverage for properties helpers
2 participants