Additional test coverage and refactoring needed in ProfileService
Please describe the enhancement
Profile Create/Update was moved to a separate interface as part of this PR: https://github.com/stacklok/minder/pull/2653
There is a significant amount of logic in these two methods, and I decided to defer writing new tests for these methods.
Solution Proposal
Write a test suite for these two methods. Further refactoring may be necessary.
Describe alternatives you've considered
No response
Additional context
No response
Acceptance Criteria
No response
Hey @gajananan, you mentioned you were interested in this issue, shall I assign it to you?
hi @eleftherias yes, please. (I am sorry for late response)
hi @dmjb
Could you share your suggestions on additional test cases. I thought of the followings.
Additional Test cases for CreateProfile Method:
- Test invalid profiles to trigger ErrValidationFailed - e.g. consider example profiles from here: https://github.com/stacklok/minder-rules-and-profiles/tree/main/profiles/github
- Test creating profile with valid and invalid labels
- Test with a profile containing edge case values (e.g., maximum length strings).
Hey @gajananan -- when you finish #4651, do you want to do this, or should we unassign?
@evankanderson yes, I will have a crack at.
@evankanderson
Could you clarify this issue is already addressed here: https://github.com/mindersec/minder/blob/main/internal/controlplane/handlers_profile_test.go ?
- TestCreateProfile
- TestDeleteProfile
It looks like the issue attempting to cover the code here:
https://github.com/mindersec/minder/blob/main/pkg/profiles/service.go
That said, coverage in the pkg/profiles directory appears to be around 80%, which meets our expectations. In particular, here is the coverage for pkg/profiles/service.go:
https://coveralls.io/builds/72057881/source?filename=pkg%2Fprofiles%2Fservice.go
Thanks for flagging that this was already done!