Repository navigation
fix(suite/loader): avoid sharing backing arrays between matrix combinations - #323
Merged
poiana merged 1 commit intoSep 12, 2026
Merged
Conversation
…ations combine() extended each existing combination with newComb := comb followed by an append, which copies the slice header only. generateCombinations calls combine once per value of the current key with the same combs slice, so once a combination has spare capacity the siblings derived from it append into the same backing array and only the last value survives. From the fourth key onwards this silently halves a matrix: the number of generated tests stays correct, so the loss is invisible, but the combinations are duplicated and one value of the last key never appears. Copy the combination before appending, and add a loader test that expands a four-key matrix through Loader.Load and counts distinct combinations. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
|
Welcome @arpitjain099! It looks like this is your first PR to falcosecurity/event-generator 🎉 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arpitjain099, ekoops The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM label has been added. DetailsGit tree hash: 85057e22d9caf0b5ea1db9304da7012b3efd1c70 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area pkg
What this PR does / why we need it:
combinederived each new combination withnewComb := combfollowed by an append. That copies the slice header only, andgenerateCombinationscallscombineonce per value of the current key while passing the samecombseach time, so as soon as a combination carries spare capacity the siblings built from it write to the same backing array and the last value wins.Combinations reach that state after the third key, because the append that grows them from two to three elements leaves room for a fourth. From four keys on, half of the matrix is silently lost: the number of generated tests is still right, so nothing looks wrong, but the tests are duplicates. A four-key spec with two values each expands to sixteen tests covering eight combinations, and one of the values of the last key never appears at all.
Copying the combination before appending keeps each branch independent.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Added
pkg/test/loader/template_test.go, which loads a four-key matrix template throughLoader.Loadand counts distinct combinations. On main it reports 8 of 16 (withd1missing from every case); with the change it reports 16. The package had no tests before, so this is the first file there.The rest of
./pkg/...builds and tests the same before and after; the packages that fail to build here are the Linux-only syscall steps andlibcap, which is expected on darwin.