-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[Improvement-16773][Parameter] Create project parameters-Field modify user adds default value #16775
[Improvement-16773][Parameter] Create project parameters-Field modify user adds default value #16775
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Result result = projectParameterController.createProjectParameter(loginUser, 1, "key", "value", | ||
DataType.VARCHAR.name()); | ||
Assertions.assertEquals(Status.SUCCESS.getCode(), result.getCode()); | ||
|
||
ProjectParameter projectParameter = (ProjectParameter) result.getData(); | ||
Assertions.assertEquals(1, projectParameter.getOperator()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assertions.assertEquals(1, projectParameter.getOperator()); | |
Assertions.assertEquals(loginUser.getId(), projectParameter.getOperator()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this UT have any help to this case? You need to add UT to ProjectParameterService
and avoid mocking the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useful because if the creation is successful, it will return projectParameter. By verifying whether the modify user is the expected value, we can determine if the addition was successful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you revert the change of projectParameterService
the case still pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you revert the change of
projectParameterService
the case still pass.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I means since the projectParameterService.createProjectParameter
has been mocked, so the test case is meaningless, it can just test the api call is work but cannot test the logic in projectParameterService
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I means since the
projectParameterService.createProjectParameter
has been mocked, so the test case is meaningless, it can just test the api call is work but cannot test the logic inprojectParameterService
。
The adjustment has been made, this should be suitable now
…value' into project_modify_user_add_defaule_value
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Purpose of the pull request
close #16773
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md