[ravindra-RKB]

Summary

This PR introduces a comprehensive unit test suite (service_test.go, +1,241 LOC) for the profile service engine in pkg/profiles/service.go.

Prior to this PR, service.go had 0% unit test coverage (explicitly acknowledged in a codebase comment: "Note that there are no unit tests for these methods in this package..."). This PR implements complete unit test coverage across all core service lifecycle methods (CreateProfile, UpdateProfile, PatchProfile, and DeleteProfile) using gomock stores, selection checkers, and event publishers.

---

Detailed Test Coverage

1. Profile Deletion (TestDeleteProfile)

• Lookup by UUID: Validates successful profile lookup, deletion via qtx.DeleteProfile, and returning the deleted profile record.
• Lookup by Name: Validates fallback to GetProfileByNameAndLock when the identifier argument is not a valid UUID string.
• Not Found (by ID & by Name): Validates codes.NotFound / user-visible errors when DB queries return sql.ErrNoRows.
• Database Query Errors: Ensures proper wrapping and status codes when GetProfileByIDAndLock or DeleteProfile encounters database failure.
• Bundle Subscription Safeguard: Verifies rejection (codes.InvalidArgument) when attempting to delete a profile managed by a bundle subscription (SubscriptionID.Valid == true).

2. Profile Creation (TestCreateProfile)

• Success with Rules & Events: Tests valid profile creation, entity rule instances upserting, CreateProfileForEntity, and reconciliation event publication to TopicQueueReconcileProfileInit.
• Selectors Validation & Creation: Tests profile creation with CEL selectors, entity mapping, and fallback handling when entity is unspecified (dbEnt.Valid = false).
• Conflict & Duplicate Detection:
  • Handles unique constraint violations (23505) and returns user-visible AlreadyExists errors.
  • Validates profile listing conflict checks (ListProfilesByProjectIDAndLabel).
• Input & Rule Validation:
  • Profile name validation failure against namespace rules.
  • Rule validation failures when rule types are missing or invalid in the project hierarchy.
• Failure Boundaries: Tests transaction rollback scenarios when CreateProfile, UpsertRuleInstance, or selector creation DB queries fail.

3. Profile Updates (TestUpdateProfile)

• Full Update Flow: Validates updating existing profiles, updating contextual entity rules, upserting new rule instances, and pruning non-updated rules via DeleteNonUpdatedRules.
• Pruning & Entity Deletion: Verifies that when entity rules are cleared to empty (len(rules) == 0), DeleteProfileForEntity is executed for that entity.
• Immutability & Constraint Enforcement:
  • Rejects attempts to modify profile name (cannot change profile name).
  • Rejects attempts to move profile across projects (cannot change profile project).
  • Returns codes.NotFound if target profile does not exist.
• Selector Updating: Tests updateSelectors workflow including selector syntax checking via SelectionChecker and DeleteSelectorsByProfileID.
• Failure Handling: Tests error propagation when DeleteNonUpdatedRules or updateSelectors encounters database errors.

4. Profile Patching (TestPatchProfile)

• Field Mask Application: Tests patching specific fields (displayName, remediate) via fieldmaskpb.FieldMask, preserving existing rules and properties.
• Lookup by Name & UUID: Tests identifier resolution via both UUID string and profile name.
• Multiple Rows Safety Check: Validates that if GetProfileByProjectAndID returns multiple rows or unexpected records, an error is returned.
• Error Boundaries: Tests database read failures (read replica down), lock acquisition timeouts, and non-existent profile lookups.

---

Verification

• All tests run in parallel (t.Parallel()).
• Formatted and grouped with gci write -s standard -s default -s "prefix(github.com/mindersec/minder)".
• Validated with:

  go test -v -count=1 ./pkg/profiles/...
  go vet ./pkg/profiles/...

  All tests passed cleanly with 0 failures and 0 vet warnings.

[coveralls]

coverage: 67.089% (+0.2%) from 66.924% — ravindra-RKB:test-profiles-service-engine into mindersec:main

[evankanderson]

Thanks for writing (prompting?) this coverage. Unfortunately, the repeated subtest style doesn't really match the pattern we use elsewhere. For example, see pkg/ruletypes/service_test.go -- that one may have gone a bit farther on helpers and DRY (don't repeat yourself) than necessary.

[evankanderson]

There's a lot of common setup and checking in these tests. Can you refactor this to use the "table driven tests" model used elsewhere in the codebase, possibly with a setup(*mockdb.MockStore, ...) and profileArg() fields?

If not, it would be good to explain why the pattern that works elsewhere doesn't work here in a comment. My suspicion (from having seen it myself) is that it's easier for the LLM to generate these types of test cases without refactoring out the common code. Unfortunately, that doesn't make it easier for humans to read the interesting parts of the code, nor does it make it easier to catch unintentional drift between different t.Run() blocks.

[ravindra-RKB]

Refactored the tests across TestDeleteProfile, TestCreateProfile, TestUpdateProfile, and TestPatchProfile to follow the standard table-driven pattern with a shared test environment helper and scenario structs (ProfileArg, DBSetup(store, pub, sel), ExpectedError). This keeps the test cases DRY and matches the conventions used elsewhere in the codebase.