More quality unit tests for UsersTeamsControllerService service class to be added
muralibasani opened this issue · 13 comments
What is currently missing?
To improve the stability of Klaw, we need to make sure there are good enough tests to cover important code snippets.
Unit tests for UsersTeamsControllerService service class is around 60% which should be improved
How could this be improved?
More quality unit tests for UsersTeamsControllerService service class to be added and also make sure the line coverage is around 85%. If any refactoring is required to achieve the same, please go ahead.
Is this a feature you would work on yourself?
- I plan to open a pull request for this feature
Hello @muralibasani @aindriu-aiven I would like to work on this issue.
Hey @khatibtamal That would be fantastic, I have been trying to find the time to break down this task into smaller sections, and if you would like to work on this it would be fantastic, let me create a couple of related issues in the morning, and I can assign you one if that suits?
Thanks a million!
Hi @khatibtamal I have added #2483 for this epic I will be adding a few more but if you like you could start on this?
or this one has a little more to it
#2485
Hello @aindriu-aiven I can attempt both, I was adding tests for the whole class, I made a lot of progress too.
I will first open a PR for #2483, hoping within a day.
Thanks.
Hello @aindriu-aiven I can attempt both, I was adding tests for the whole class, I made a lot of progress too. I will first open a PR for #2483, hoping within a day. Thanks.
Great I'll review what ever tests you add anyway :) let me assign you the tickets
Hello @aindriu-aiven the line coverage is at 56% with the added tests. There are still plenty of methods whose tests can added like getNewUserRequests, approveNewUserRequests, declineNewUserRequests, getRegistrationInfoFromId and plenty more whose tests can be added. Are you looking to have those added as well? If so you can create the breakdown issues and I can work on them. Thanks.
Hello @aindriu-aiven the line coverage is at 56% with the added tests. There are still plenty of methods whose tests can added like
getNewUserRequests,approveNewUserRequests,declineNewUserRequests,getRegistrationInfoFromIdand plenty more whose tests can be added. Are you looking to have those added as well? If so you can create the breakdown issues and I can work on them. Thanks.
Hey Yes, I can create another issue to cover these tests and I'll put a break down and assign it to you.
@aindriu-aiven the line coverage last I checked was 70%, I see the goal in this issue is 85%. I would like to finish off this epic.
@aindriu-aiven the line coverage last I checked was 70%, I see the goal in this issue is 85%. I would like to finish off this epic.
Hey @khatibtamal absolutely, brilliant work so far I'll write up a couple of tickets this morning to try get the last 10/15%
It gets to diminishing returns as you get above 80% so if we don't quite hit 80% don't worry, but I will @ you on the new tickets I create.
@aindriu-aiven the line coverage last I checked was 70%, I see the goal in this issue is 85%. I would like to finish off this epic.
Hey @khatibtamal absolutely, brilliant work so far I'll write up a couple of tickets this morning to try get the last 10/15%
It gets to diminishing returns as you get above 80% so if we don't quite hit 80% don't worry, but I will @ you on the new tickets I create.
hi @aindriu-aiven just FYI I worked on changePwd and updateTeam as draft that gets the line coverage to 78%.
@aindriu-aiven I have opened a PR directly for this epic. The line coverage is 86%, If you wish to create more tickets for this epic and merge based on that, I am fine, I can close this PR and work on the granular tickets. Thanks.
@aindriu-aiven I have opened a PR directly for this epic. The line coverage is 86%, If you wish to create more tickets for this epic and merge based on that, I am fine, I can close this PR and work on the granular tickets. Thanks.
Hey thanks a million for this, apologies for my slow response I managed to get sick yesterday I'll be back online tomorrow morning, but that sounds fantastic 🚀
@aindriu-aiven I have opened a PR directly for this epic. The line coverage is 86%, If you wish to create more tickets for this epic and merge based on that, I am fine, I can close this PR and work on the granular tickets. Thanks.
Hey thanks a million for this, apologies for my slow response I managed to get sick yesterday I'll be back online tomorrow morning, but that sounds fantastic 🚀
hi no problem at all, please take your time, and if there is anything you prefer differently please let me know. Get well soon!!