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

Added test methods for submodelrepository and submodelservice #335

Merged
merged 7 commits into from
Aug 21, 2024

Conversation

VivekHub97
Copy link
Contributor

This PR adds missing test methods in the submodelrepository and submodelservice test suite.


Submodel retrievedSubmodelMetadata = repo.getSubmodelByIdMetadata(ID);

assertEquals(expectedSubmodel.getId(), retrievedSubmodelMetadata.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole content of the Submodel should be the same and not just ID.
Please correct this.

public void getSubmodelByIdMetadata() throws JsonProcessingException {
SubmodelRepository repo = getSubmodelRepository();
Submodel expectedSubmodel = buildDummySubmodelWithNoSmElement(ID);
repo.createSubmodel(expectedSubmodel);
Copy link
Contributor

Choose a reason for hiding this comment

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

A normal Submodel with SM Elements should be created but you can create an expected SM same as per the expected SM returned by the API. So that you can compare the expected and the actual.

SubmodelValueOnly expectedSmValueOnly = new SubmodelValueOnly(submodelElements);
SubmodelValueOnly retrievedSmValueOnly = repo.getSubmodelByIdValueOnly(ID);

assertSameSubmodelValueOnly(expectedSmValueOnly,retrievedSmValueOnly);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we just use assertEquals? bothe are of type SubmodelValueOnly.
Please also correct formatting, use BaSyx formatter if not using.

assertSameSubmodelValueOnly(expectedSmValueOnly, retrievedSmValueOnly);
}

private void assertSameSubmodelValueOnly(SubmodelValueOnly expectedSmValueOnly,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this to the end of this class.
The hierarchy of method should always be:
Public
Protected
Private

SubmodelValueOnly expectedSmValueOnly = new SubmodelValueOnly(submodelElements);
SubmodelValueOnly retrievedSmValueOnly = repo.getSubmodelByIdValueOnly(ID);

assertSameSubmodelValueOnly(expectedSmValueOnly, retrievedSmValueOnly);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already a helper function for what you are doing in this method.
Please use this helper function.

And then remove this assertSameSubmodelValueOnly method.


Submodel patchedSubmodel = submodelService.getSubmodel();

assertEquals(submodel.getSubmodelElements().size(),patchedSubmodel.getSubmodelElements().size());
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still formatting needed here (there is no space after the comma).
Please use the BaSyx formatter.


Submodel patchedSubmodel = repo.getSubmodel(ID);

assertEquals(submodel.getSubmodelElements().size(),patchedSubmodel.getSubmodelElements().size());
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still formatting needed here (there is no space after the comma).
Please use the BaSyx formatter.

Copy link
Member

Choose a reason for hiding this comment

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

Please update the License header to 2024

Copy link
Member

Choose a reason for hiding this comment

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

Please update the license header to 2024

@aaronzi aaronzi merged commit 7ea6337 into eclipse-basyx:main Aug 21, 2024
2 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