hawkular / hawkular-android-client

Apache License 2.0
10 stars 26 forks source link

Unit tests for models needed to be updated #158

Open KeenWarrior opened 7 years ago

KeenWarrior commented 7 years ago

Recommended to break down in multiple PR

sauravvishal8797 commented 7 years ago

@garg-anuj Would like to work on this.

KeenWarrior commented 7 years ago

@sauravvishal8797 Go on

m-murad commented 7 years ago

@garg-anuj Is this assigned to @sauravvishal8797 or can I work on this?

danielpassos commented 7 years ago

@free4murad Just assigned this to @sauravvishal8797 to avoid confusion

danielpassos commented 7 years ago

@garg-anuj Can you elaborate a little more in the description what do you expect here?

KeenWarrior commented 7 years ago

@danielpassos Yeah sure

Models under org.hawkular.client.android.backend.model need to have updated constructors annotated with @VisibleForTesting and unit tests.

Some models don't have tests at present so new tests needed to be created for them

pg301 commented 7 years ago

@danielpassos @garg-anuj I think this issue requires significant amount of work. Can I start writing tests for some modules.

m-murad commented 7 years ago

@danielpassos @garg-anuj Actually I haven't completed any GSoC issue yet. So if @sauravvishal8797 will not be working on it please assign it to me.

sauravvishal8797 commented 7 years ago

@pg301 @free4murad Still working on this one.

pg301 commented 7 years ago

@sauravvishal8797 Cool! Please let me know if you need some help. I have updated some constructors in a3f0b95 so if you want you can skip that part.

pg301 commented 7 years ago

@sauravvishal8797 The issue description suggests that we break down the issue in multiple PRs. I think we can split some work. Please let me know your progress and we can decide on the remaining modules.

KeenWarrior commented 7 years ago

@free4murad GSoC issue only means that given issue will be probably easy one to tackle and nothing more. So find issues and register them. Thats it :wink:

sauravvishal8797 commented 7 years ago

@pg301 Yeah cool! For the constructors part we also needed to update the constructor for trigger model as the tests were failing for the trigger model, so I have updated it here 55b716267c0230664bfd6d73c0c717cb759d8ed0 and had to set minifyenabled to false to avoid the warnings.

pg301 commented 7 years ago

@sauravvishal8797 @garg-anuj The recently opened pull requests 9e85a15 only fixes 'updating visibleForTesting annotation'. There are a number of models which do not have corresponding tests. I would like to write tests for the remaining models and will send a PR.