Unit tests coverage for properties helpers

[andrii-bodnar]

Some classes in the com.crowdin.cli.properties namespace have low coverage by Unit tests. It would be great to add Unit tests for these classes.

• [ ] PropertiesBuilders.java
• [ ] BasePropertiesBuilder.java
• [ ] PropertiesWithTargets.java
• [ ] ProjectPropertiesBuilder.java
• [ ] PropertiesBuilderChecker.java
• [ ] PropertiesWithTargetsBuilder.java

Similar tests for reference can be found here

[manish-singh-bisht]

Hey @andrii-bodnar in order to complete this task, what knowledge should one have? Thank You

[andrii-bodnar]

@manish-singh-bisht you need to have a basic knowledge of writing Unit tests in Java using Jupiter and Mockito libraries

[DukeRino7]

Hello @andrii-bodnar. Is it possible to assign this issue to write unit test and assing it to me? Thanks for reply.

[andrii-bodnar]

Hi @DukeRino7! It sounds great! I would suggest you to check the code coverage report, find the part of code you like and cover this code by unit tests. What do you think about this idea? Or you need a specific issue for that?

[andrii-bodnar]

@DukeRino7 also, this issue is unassigned yet. I could assign it to you

[DukeRino7]

@andrii-bodnar im not sure if Iam able to do tests for all classes.can i take a look at the code and write you tomorrow? Until then can you assign me to this issue?

[andrii-bodnar]

@DukeRino7 you can split this issue into several PRs and begin from just one or two classes

[DukeRino7]

@DukeRino7 you can split this issue into several PRs and begin from just one or two classes

@andrii-bodnar ok i think it can work. Can you assign it to me?

[andrii-bodnar]

Hi @DukeRino7, do you need any assistance here? Just wondering what the progress on this issue

[DukeRino7]

Hello @andrii-bodnar. I had some problems but now im working on it. If i will be in trouble i will write you. Do you have specific deadline or something like that?

[andrii-bodnar]

@DukeRino7 thanks for the update! There is no specific deadline 🙂

[DukeRino7]

@andrii-bodnar i will do it as fast as possible

[calfaand]

Hello, can I contribute to this project with writing tests ?

[andrii-bodnar]

Hi @calfaand, sure, thank you!

Feel free to take some classes from the issue description.

Also, you are welcome to check other parts of the project for classes or methods where it's possible to write some tests.

[andrii-bodnar]

@DukeRino7 which classes you are currently working on?

[calfaand]

@andrii-bodnar so I will take the first 3 classes okay?

[andrii-bodnar]

@calfaand sure, please let me know if you need any assistance

[calfaand]

@andrii-bodnar In PropertiesWithTargetBuilder.java you will always get exception because Targets are not able to be initialized. Problem lies in ParamsWithTargets as it does not have any variable Targets that can be populated if you are not using config file. Is this by design or it's only possible way of doing that?

[andrii-bodnar]

@calfaand what exact method do you mean? As I see from the PropertiesWithTargetsBuilder class, we call the getTargets getter only once:

https://github.com/crowdin/crowdin-cli/blob/cli3/src/main/java/com/crowdin/cli/properties/PropertiesWithTargetsBuilder.java#L68

It is called on the props object which is the PropertiesWithTargets class instance

[calfaand]

I mean this class. https://github.com/crowdin/crowdin-cli/blob/cli3/src/main/java/com/crowdin/cli/properties/ParamsWithTargets.java

If I try to initialize targets with this class I am not able to do this.

[andrii-bodnar]

@calfaand I'm not sure if I understand you correctly. Could you please provide the related code and the failed test error message?

[DukeRino7]

@andrii-bodnar i get to point where i want to test some methods but they are "protected". How can i continue? Because from my understanding i cant reach protected methods when test are in different folder. Should i chAnge them to public? Thanks for advice.

[andrii-bodnar]

@DukeRino7 it's not recommended to make some methods public just for tests. I'm not familiar with Java Unit testing so I'm not sure how to test such methods. Maybe there is some workaround.

[DukeRino7]

@andrii-bodnar okay, today i will look into it and hopefully i will be able to make it work somehow. If i will find way to make it work i will probably finish writing test this week. I will keep you updated If i will have some difficulties

[andrii-bodnar]

@DukeRino7 great, thanks for the update!

[DukeRino7]

@andrii-bodnar i find way how to test methods and im writing test for them right now. Sorry for lack of communication up to now, i had some problems that i need to solve first. Now im working on it and hopefully i will be able to finish writing test this week. I will also keep you updated about my progress.

[andrii-bodnar]

@DukeRino7 no worries, thanks for the update!

[DukeRino7]

@andrii-bodnar hello, i request pull, coverage is increased by 2,5%

[andrii-bodnar]

Hi @DukeRino7, awesome, thanks a lot for the contribution!

Briefly looked over the PR and it looks good to me. I'll review it in more detail tomorrow and then will merge it. Thanks!