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

feat: add tests for submodel methods #360

Conversation

ShehriyarShariq-Fraunhofer
Copy link
Contributor

@ShehriyarShariq-Fraunhofer ShehriyarShariq-Fraunhofer commented Jul 26, 2024

Pull Request Template

Description of Changes

Added test cases for SubmodelRepository(getSubmodelByIdValueOnly and getSubmodelByIdMetadata) and SubmodelService(patchSubmodelElements).

Copy link
Contributor

@mateusmolina-iese mateusmolina-iese left a comment

Choose a reason for hiding this comment

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

@@ -625,10 +625,9 @@ public void deleteNonExistingFile() throws IOException {

submodelService.deleteFileValue(SubmodelServiceHelper.SUBMODEL_TECHNICAL_DATA_FILE_ID_SHORT);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these changes

return repoApi.getSubmodelById(submodelId, null, null);
} catch (ApiException e) {
if (e.getCode() == HttpStatus.NOT_FOUND.value()) {
throw new ElementDoesNotExistException(submodelId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use mapExceptionSubmodelAccess(...) instead of the conditional

@@ -47,4 +47,12 @@ public PropertyValue(String value) {
public String getValue() {
return value;
}

@Override
public boolean equals(Object obj) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to override equals here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In getSubmodelByIdValueOnlyExistingSubmodel, I need to compare the objects of type SubmodelValueOnly which have a Map of items where the value is of type SubmodelElementValue.

PropertyValue implements the interface SubmodelElementValue, hence, when i compare the SubmodelValueOnly objects, I'm actually comparing their map of items to check if they are equal.

Given that PropertyValue isn't a primitive data type, I had to add the equals method so that the comparison can be made between the items. The test fails otherwise.

@ShehriyarShariq-Fraunhofer
Copy link
Contributor Author

Yes, it is being tested. SubmodelRepositoryApiHTTPController uses SubmodelRepository and basyx.submodelrepository-core contains the test for getSubmodelByIdValueOnly in SubmodelRepositorySuite.java.

@@ -207,7 +207,11 @@ public SubmodelValueOnly getSubmodelByIdValueOnly(String submodelId) throws Elem

@Override
public Submodel getSubmodelByIdMetadata(String submodelId) throws ElementDoesNotExistException {
return repoApi.getSubmodelById(submodelId, null, null);
try {
return repoApi.getSubmodelById(submodelId, null, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

should be getSubmodelByIdMetadata

@ShehriyarShariq-Fraunhofer ShehriyarShariq-Fraunhofer marked this pull request as ready for review November 5, 2024 06:42
@aaronzi aaronzi merged commit 065c2ec into eclipse-basyx:main Nov 5, 2024
27 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.

3 participants