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

maintenance: Add getAllTeamsSUFromRegisterUser, getAllTeamsSUOnly, ge… #2530

Conversation

khatibtamal
Copy link
Contributor

@khatibtamal khatibtamal commented Jul 10, 2024

…tAllTeamsSU, deleteTeam, updateTeam and changePwd for UserTeamsControllerService

Linked issue

Resolves: #2427

What kind of change does this PR introduce?

  • Bug fix
  • New feature
  • Refactor
  • Docs update
  • CI update

What is the current behavior?

UsersTeamsControllerService.java has unit test line coverage of apprx 70%.

What is the new behavior?

New unit tests added for getAllTeamsSUFromRegisterUsers, getAllTeamsSUOnly, updateTeam, changePwd. The following tests were further refined getAllTeamsSU, deleteTeam .

Other information

There was a test for getAllTeamsSUOnly but it was in fact testing getAllTeamsSU, the name was changed and getAllTeamsSU was refined. In UsersTeamsControllerService.java there was some unused code, which was removed. Now the line coverage of tests for UsersTeamsControllerService is 86%.

Requirements (all must be checked before review)

  • The pull request title follows our guidelines
  • Tests for the changes have been added (if relevant)
  • The latest changes from the main branch have been pulled
  • pnpm lint has been run successfully

…tAllTeamsSU, deleteTeam, updateTeam and changePwd for UserTeamsControllerService

Signed-off-by: Khatib Tamal <[email protected]>
…tAllTeamsSU, deleteTeam, updateTeam and changePwd for UserTeamsControllerService

Signed-off-by: Khatib Tamal <[email protected]>
@muralibasani muralibasani force-pushed the 2427-finalize-unit-tests-for-usersteamscontrollerservice branch from e724260 to 11438e2 Compare July 22, 2024 05:49
@muralibasani muralibasani self-assigned this Jul 22, 2024
@muralibasani
Copy link
Contributor

@khatibtamal I will take a look today


@Test
public void getAllTeamsSU() {}
public void getAllTeamsSUFromRegisterUsersEnvInvalidForTenant() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good one indeed

@muralibasani
Copy link
Contributor

@khatibtamal thank you for such beautiful tests with high coverage. Looks good to me.
Will wait for @aindriu-aiven if any comments.

@aindriu-aiven
Copy link
Contributor

@khatibtamal thank you for such beautiful tests with high coverage. Looks good to me. Will wait for @aindriu-aiven if any comments.

Yeah I am sorry, between getting sick and then trying to catch up at work, I haven't had a chance to look at this yet, but I am going to check it out this morning.

Thanks a million for being so patient!

@khatibtamal
Copy link
Contributor Author

@khatibtamal thank you for such beautiful tests with high coverage. Looks good to me. Will wait for @aindriu-aiven if any comments.

Thanks a lot! appreciate

…rsteamscontrollerservice' into 2427-finalize-unit-tests-for-usersteamscontrollerservice
@khatibtamal
Copy link
Contributor Author

@khatibtamal thank you for such beautiful tests with high coverage. Looks good to me. Will wait for @aindriu-aiven if any comments.

Yeah I am sorry, between getting sick and then trying to catch up at work, I haven't had a chance to look at this yet, but I am going to check it out this morning.

Thanks a million for being so patient!

Thanks @aindriu-aiven hope you are well now. I have updated the branch.

Copy link
Contributor

@aindriu-aiven aindriu-aiven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM This is a great addition to improving the quality in Klaw Thank you very much!

@aindriu-aiven aindriu-aiven merged commit 0b742e6 into Aiven-Open:main Jul 24, 2024
24 checks passed
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.

More quality unit tests for UsersTeamsControllerService service class to be added
3 participants