[client] handle order of check when checking order of files in isChecksEqual #4219
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the isChecksEqual function where the order of checks in the slice was not being considered, only the order of files within each check. This was causing false equality results and unnecessary metadata updates.
- Refactored the
isChecksEqualfunction to normalize check slices by sorting both files within checks and the checks themselves - Added comprehensive test cases to verify the fix handles different check orderings correctly
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| client/internal/engine.go | Rewrote isChecksEqual function to properly handle order independence for both checks and files |
| client/internal/engine_test.go | Added test cases to verify the fix works with different check and file orderings |
| sortedFiles := make([]string, len(check.Files)) | ||
| copy(sortedFiles, check.Files) | ||
| sort.Strings(sortedFiles) | ||
| normalized[i] = strings.Join(sortedFiles, "|") |
There was a problem hiding this comment.
Using "|" as a delimiter could be problematic if filenames contain this character. Consider using a delimiter that's guaranteed not to appear in filenames, such as "\x00" (null byte), or use a more robust serialization approach.
| normalized[i] = strings.Join(sortedFiles, "|") | |
| normalized[i] = strings.Join(sortedFiles, "\x00") |
Copilot uses AI. Check for mistakes.
|
Describe your changes
The order of checks was not considered, only the order of files within a check. This was causing false results and unnecessary meta updates.
Issue ticket number and link
Stack
Checklist