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

test(openchallenges): improve organization service model ImageResponse and OrganizationMapper unit test coverage #2388

Merged
merged 25 commits into from
Dec 8, 2023

Conversation

mdsage1
Copy link
Contributor

@mdsage1 mdsage1 commented Dec 5, 2023

Description

Changes include packages w/a single class.
Sonar unit test coverage has been increased to 12% for model/mapper package. This package has a single class, OrganizationMapper.
Sonar unit test coverage has been increased to 100% for service/model/rest/response package. This package has a single class, ImageResponse.
The OrganizationServiceApplication class in the organization/service package is the only class in that package but the only uncovered code would be categorized as an integration test while the focus here is on unit testing.

A comment was also added to OrganizationServiceApplication for the existing integration test. This confused me and I had to deep dive into what was happening to find out that it was an integration test rather than a unit test.

Related Issue

#2063
#1528
#2348

Changelog

See files Changed section

These changes have not increased the Sonar coverage above the 15%,9.5% according to the Code Quality Dashboard, achieved with the exception unit test PR #2348 but, different components, methods etc., of these packages' classes are being tested by the unit tests added. These tests were written last week and debugged this week. The PR has been released after the approval of the exception unit test PR on 2023-12-04.

Preview

The Sonar coverage increase for model/mapper:
image

The Sonar coverage increase for service/model/rest/response:
image

@mdsage1 mdsage1 self-assigned this Dec 5, 2023
@mdsage1 mdsage1 changed the title test(openchallenges): improve organization service model mapper OrganizationMapper unit test coverage test(openchallenges): improve organization service model ImageResponse and OrganizationMapper unit test coverage Dec 5, 2023
@mdsage1 mdsage1 marked this pull request as ready for review December 5, 2023 20:23
Copy link
Member

@tschaffter tschaffter left a comment

Choose a reason for hiding this comment

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

Looks good overall! See comments below

@mdsage1
Copy link
Contributor Author

mdsage1 commented Dec 5, 2023

  1. Removed the comment regarding the integration test in service/OrganizationServiceApplicationTests
  2. Renamed organizationEntity to entity in model/mapper/OrganizationMapperTest.java
  3. Renamed user variable to dTo in service/model/mapper/OrganizationMapperTest.java
  4. Changed the variable names that used retrieved to actual throughout model/mapper/OrganizationMapperTest.java

@tschaffter
Copy link
Member

  1. Removed the comment regarding the integration test in service/OrganizationServiceApplicationTests
    ...

Let's keep the discussion in their respective thread. When you address a comment from a reviewer, you can either close it or comment + close it if you have addressed it. If work is in progress or clarification is needed, keep the comment thread open. Thanks!

@tschaffter tschaffter merged commit ca0f467 into Sage-Bionetworks:main Dec 8, 2023
6 checks passed
@mdsage1 mdsage1 deleted the org_srvc_model_mapper branch December 8, 2023 19:45
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.

2 participants