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 entity unit test coverage #2536

Merged
merged 14 commits into from
Mar 13, 2024

Conversation

mdsage1
Copy link
Contributor

@mdsage1 mdsage1 commented Feb 27, 2024

Description

Organization Service coverage goal of >50% percent addressed with this PR focused on improving overall coverage of model/entity.

Related Issue

Fixes # 2505

Changelog

  • Add unit tests for ChallengeContributionEntity.java that improved coverage connected to OrganizationEntity.java. Coverage (reported by Jacoco) increased from 48% overall to 65%.
  • Add unit tests for OrganizationCategoryEntity.java that improved coverage connected to OrganizationCategoryEntity.java. Coverage (reported by Jacoco) increased from 65% overall to 80%.

@mdsage1 mdsage1 added the sonar-scan-approved-deprecated Ready for Sonar code analysis label Feb 27, 2024
@mdsage1 mdsage1 self-assigned this Feb 27, 2024
@mdsage1
Copy link
Contributor Author

mdsage1 commented Feb 28, 2024

PR is ready for review. Unit tests have been added for OrganizationService model/entity.

@mdsage1 mdsage1 marked this pull request as ready for review February 28, 2024 19:24
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.

Use a more descriptive title for the PR. E.g. be more specific about what part of the project you are modifying.

Let's use this simple use case to further improve how you write tests. The main limitation is that you only use assertEquals or assertNotEqual and this leads to superficial tests. Try to test for more specific general and edge cases, trigger errors, etc.

Consider below the improvement that ChatGPT suggests. Please review each of these suggestions and try to find an application for each of them by the time of our next meeting.

ChatGPT feedback

Prompt: Please review this unit test file written in Java. How could this file be improved?
<code>

Response:

This unit test file for the OrganizationCategoryEntity class in Java is well-structured and covers several aspects, including constructor behavior, getter methods, and the consistency of equals and hashCode methods. However, there are several areas where it can be improved for clarity, maintainability, and effectiveness:

  1. Use Descriptive Test Method Names: The test method names are quite descriptive, but they could be improved to follow a more consistent pattern that explicitly states the expected outcome, the condition under which it's expected, and the specific method or scenario being tested.

  2. Reduce Duplication: There's a noticeable amount of setup duplication, especially in the creation of OrganizationCategoryEntity instances. Consider utilizing a factory method or @ParameterizedTest with different arguments to streamline the creation of test objects and reduce code duplication.

  3. Improve Assertions:

    • Consider using more specific assertions where possible. For instance, instead of just assertEquals, you could use assertThat combined with matchers (e.g., assertThat(contributor1.getCategory(), is(equalTo(category)));). This approach can make the assertions more readable and provide more detailed error messages.
    • It might be beneficial to add messages to assertions to explain the context or reason for the test, especially when the purpose of the test might not be immediately clear from the assertion itself.
  4. Test Naming Convention: Adopt a more consistent naming convention for tests. For example, use a pattern like methodName_StateUnderTest_ExpectedBehavior. This makes it easier to understand what each test is verifying at a glance.

  5. Leverage @DisplayName: Use @DisplayName annotations to give a human-readable description to your tests. This is particularly useful when you want to describe the test scenario in a way that the method name alone cannot convey.

  6. Check for Edge Cases: The current tests cover basic scenarios. Consider adding tests for edge cases, such as null inputs, invalid arguments, or boundary values for the methods being tested.

  7. Separation of Concerns: Each test method currently tests multiple aspects. It's generally a good practice to have each test method focus on a single behavior or aspect of the class under test. This makes it easier to diagnose issues when a test fails because the failing test directly points to the problematic behavior.

  8. Code Comments: While the setup method is somewhat self-explanatory, adding comments to explain the purpose behind certain test cases or the significance of specific test conditions can improve readability and maintainability, especially for more complex tests.

  9. Mocking External Dependencies: If OrganizationEntity or any other external dependencies become more complex and interact with external systems or have side effects, consider using mocks or stubs to isolate the tests better.

Improving these aspects can lead to a more maintainable, readable, and robust suite of unit tests, making it easier to understand the purpose of each test and diagnose issues when tests fail.

@mdsage1
Copy link
Contributor Author

mdsage1 commented Mar 1, 2024

@tschaffter I have used chatGPT to update both java files included in this pr.

@mdsage1 mdsage1 requested a review from tschaffter March 1, 2024 18:58
@mdsage1 mdsage1 marked this pull request as draft March 1, 2024 20:24
@mdsage1 mdsage1 marked this pull request as ready for review March 2, 2024 00:03
Copy link

sonarqubecloud bot commented Mar 2, 2024

Quality Gate Passed Quality Gate passed for 'openchallenges-app'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

sonarqubecloud bot commented Mar 2, 2024

Quality Gate Passed Quality Gate passed for 'openchallenges-challenge-service'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.5% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarqubecloud bot commented Mar 2, 2024

Quality Gate Passed Quality Gate passed for 'openchallenges-organization-service'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarqubecloud bot commented Mar 2, 2024

Quality Gate Passed Quality Gate passed for 'openchallenges-image-service'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@mdsage1
Copy link
Contributor Author

mdsage1 commented Mar 13, 2024

@tschaffter Can you review this pr please?

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.

Tested and approved

@tschaffter tschaffter merged commit cf1c507 into Sage-Bionetworks:main Mar 13, 2024
12 checks passed
@mdsage1 mdsage1 deleted the unittest_modelentity branch March 13, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sonar-scan-approved-deprecated Ready for Sonar code analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants