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.

Sign up for GitHub

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

Jump to bottom

Tests for rest calls #125

Merged
merged 40 commits into from
Mar 15, 2024
Merged

Tests for rest calls #125

merged 40 commits into from
Mar 15, 2024

Conversation

Weltraumschaf
Copy link
Member

@Weltraumschaf Weltraumschaf commented Mar 7, 2024 •
edited
Loading

This PR adds capture and replay integration tests for all generic services.

@Weltraumschaf Weltraumschaf marked this pull request as draft March 7, 2024 20:50
@Weltraumschaf Weltraumschaf self-assigned this Mar 7, 2024
@Weltraumschaf Weltraumschaf added the enhancement New feature or request label Mar 7, 2024
@Weltraumschaf
Capture & Replay Integration Test for EndpointService
ab94dd8 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Simplify Fixture Loading
5577076 
Since package is always same, so we can resolve the whole path in
the base class.

Also put the context information in file name at the end, so that
fixtures show up near the test class in the IDE.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for EngagementService
db808e5 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
No Integration Test for Base Service Implementation
57f11ab 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for GroupMemberService
13f2635 
Also fix wrong JSON property mapping.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for GroupService
fb6d745 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for ProductGroupService
79ef531 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for ProductService
c2a2991 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for TestService
d511791 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for TestTypeService
5d89d5b 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for ToolConfigService
2dcae1d 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Capture & Replay Integration Test for ToolTypeService
3083557 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Remove Example WireMock Test Case
1d759f8 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Port FindingServiceTest To WireMock
1738067 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Add Missing SPDX Headers
c8fa2b1 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Port UserProfileServiceTest To WireMock
9eec68f 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Contains Only Tests for Interface
6b38ec9 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
This Is Not a Typical Service
ae22d59 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf Weltraumschaf force-pushed the tests_for_rest_calls branch 3 times, most recently from 66515d3 to 4f4e3c9 Compare March 14, 2024 14:22
@Weltraumschaf
Port Test Cases from EndpointService to Other Services
eccab28 
Also introduce some defaults to models which came from DefectDojo:
We had some fields null by default which will be empty string/array
by default in DefectDojo responses.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Add Missing TEsts and Fixtures for ProductGroupService
155345f 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Add Missing Fixture
2af85fa 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Fix Assertions for Special Implementation of UserProfileService
9bdce96 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Move Public API to top of Class
f793401 
The structure of code should be ordered, so that the more abstract
API is at the top and the more nitty gritty details are below. This
has the advantage that one interested in an quick overview need not
to scroll up and down. You only need to scroll down for details.

Also add some null-checks and missing API doc.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf Weltraumschaf marked this pull request as ready for review March 14, 2024 20:20
@Weltraumschaf
Update dependencies
37931fc 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Update plugins
b9835b8 
Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf
Add Missing Interface Doc and Null Checks
e4b6e3f 
Signed-off-by: Sven Strittmatter <[email protected]>
Ilyesbdlala
Ilyesbdlala approved these changes Mar 15, 2024
View reviewed changes
Copy link
Member

@Ilyesbdlala Ilyesbdlala left a comment

Choose a reason for hiding this comment

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

Have a couple of questions, but LGTM.

src/main/java/io/securecodebox/persistence/defectdojo/service/ProductTypeService.java
@@ -10,7 +10,7 @@
import io.securecodebox.persistence.defectdojo.model.PaginatedResult;
import io.securecodebox.persistence.defectdojo.model.ProductType;

public class ProductTypeService extends GenericDefectDojoService<ProductType> {
public final class ProductTypeService extends GenericDefectDojoService<ProductType> {
Copy link
Member

Choose a reason for hiding this comment

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

What is the reasoning behind making the class final ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a general advice to make API as private and immutable as possible to avoid accidental missuse. I make classes in general package private and final. Only, and only if, I later encounter that it is necessary to open such contraints, I do this explicitly. This practice helps me to avoid accidental complexity (e.g. inheritance hell).

Another example – not relevant for this particular example because it is a service and not a value object – is equalTo() and hashCode(). If your class is not final then you must handle Liskov Substitution Principle to avoid possible memory leaks. That's quite complicated, so it makes sense to forbid deriving a model class to avoid this.

A very good book about this is Effecitve Java from Joshua Bloch.

src/test/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanServiceTest.java
*/
class DefaultImportScanServiceTest {
final class DefaultImportScanServiceTest {
Copy link
Member

Choose a reason for hiding this comment

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

what do you mean in the commit by "This is not a Typical Service"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not "typical" means "does not inherit from GenericDefectDojoService". Thisis not very good documented nor well architected. Hope to make implicit stuff like this more explicit during my refacotrings.

@Weltraumschaf Weltraumschaf merged commit 4c532fa into secureCodeBox:main Mar 15, 2024
2 checks passed
@Weltraumschaf Weltraumschaf deleted the tests_for_rest_calls branch March 15, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ilyesbdlala Ilyesbdlala Ilyesbdlala approved these changes

Assignees

@Weltraumschaf Weltraumschaf

Labels
enhancement New feature or request
Projects
secureCodeBox v4
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Weltraumschaf @Ilyesbdlala